do_async_method_url ignoring content-type

[Arne Goedeke]

do_async_method_url overrides the content-type given in extra headers.
Its possible to work around it by using "Content-Type" instead of
"content-type". This is relevant at least in the XMLRPC module, not
sure if anybody noticed. here is a patch

arne

diff --git a/lib/modules/Protocols.pmod/HTTP.pmod/Session.pike b/lib/modules/Protocols.pmod/HTTP.pmod/Session.pike
index 5b82fc9..b7e2e74 100644
--- a/lib/modules/Protocols.pmod/HTTP.pmod/Session.pike
+++ b/lib/modules/Protocols.pmod/HTTP.pmod/Session.pike
@@ -874,8 +874,8 @@ Request async_do_method_url(string method,

     if (method=="POST")
        extra_headers=
- (extra_headers||([]))+
- (["content-type":"application/x-www-form-urlencoded"]);
+ (["content-type":"application/x-www-form-urlencoded"])+
+ (extra_headers||([]));

     p->do_async(p->prepare_method(method,url,query_variables,
  extra_headers,data));

Looks reasonable, since (afaics) that method doesn't do anything with
the data. It'd be a different matter if it took the variables and
encoded them into the body, for instance.

Considering that it doesn't do anything with the data at all, I'd say
it really has no business setting a content type, but I guess it's
better left in place for compatibility.

Patch applied in 7.8. Thanks.

[Arne Goedeke]

I also pondered moving that into do_async_post.. but maybe someone is
relying on that somewhere.

thanks

arne

On Fri, 30 Oct 2009, Martin Stjernholm, Roxen IS @ Pike  developers forum wrote:

> Looks reasonable, since (afaics) that method doesn't do anything with
> the data. It'd be a different matter if it took the variables and
> encoded them into the body, for instance.
>
> Considering that it doesn't do anything with the data at all, I'd say
> it really has no business setting a content type, but I guess it's
> better left in place for compatibility.
>
> Patch applied in 7.8. Thanks.
>

[Arne Goedeke]

Arne Goedeke wrote:

> I also pondered moving that into do_async_post.. but maybe someone is
> relying on that somewhere.
>
> thanks
>
> arne
>
> On Fri, 30 Oct 2009, Martin Stjernholm,    Roxen IS @ Pike  developers
> forum wrote:
>
>> Looks reasonable, since (afaics) that method doesn't do anything with
>> the data. It'd be a different matter if it took the variables and
>> encoded them into the body, for instance.
>>
>> Considering that it doesn't do anything with the data at all, I'd say
>> it really has no business setting a content type, but I guess it's
>> better left in place for compatibility.
>>
>> Patch applied in 7.8. Thanks.
>>

the same seems to be happening in the blocking variant do_method_url.
There is also a bug in Tools.Hilfe.GenericAsyncHilfe, where the write
callback is triggered permanently because an empty string is written in
case the outbuffer is empty. I attached a patch for hilfe.

arne

diff --git a/lib/modules/Tools.pmod/Hilfe.pmod b/lib/modules/Tools.pmod/Hilfe.pmod
index aafa465..64a746c 100644
--- a/lib/modules/Tools.pmod/Hilfe.pmod
+++ b/lib/modules/Tools.pmod/Hilfe.pmod
@@ -2973,6 +2973,7 @@ class GenericAsyncHilfe
 
   void write_callback()
   {
+    if (!sizeof(outbuffer)) return;
     int i=outfile->write(outbuffer);
     outbuffer=outbuffer[i..];
   }

> the same seems to be happening in the blocking variant
> do_method_url.

Right, thanks.

> There is also a bug in Tools.Hilfe.GenericAsyncHilfe, where the write
> callback is triggered permanently because an empty string is written in
> case the outbuffer is empty. I attached a patch for hilfe.

I'm not familiar with that code, but for write callbacks in general,
they shouldn't be installed in the first place unless they're going to
actually write something. I guess your patch covers the problem since
the write callback gets blocked internally until the next write on the
fd, but then it could remain blocked even when something is put into
the output buffer.

[Arne Goedeke]

Martin Stjernholm, Roxen IS @ Pike developers forum wrote:

>> the same seems to be happening in the blocking variant
>> do_method_url.
>
> Right, thanks.
>
>> There is also a bug in Tools.Hilfe.GenericAsyncHilfe, where the write
>> callback is triggered permanently because an empty string is written in
>> case the outbuffer is empty. I attached a patch for hilfe.
>
> I'm not familiar with that code, but for write callbacks in general,
> they shouldn't be installed in the first place unless they're going to
> actually write something. I guess your patch covers the problem since
> the write callback gets blocked internally until the next write on the
> fd, but then it could remain blocked even when something is put into
> the output buffer.

The send_output method which writes to the buffer triggers a write
attempt. So that should not be a problem. But I cannot say how it was
intended to work in the first place.

> The send_output method which writes to the buffer triggers a write
> attempt. So that should not be a problem.

I see. Then I guess it works, although personally I think it's a bit
obscure to rely on the internal write cb blocker. Anyway, I've applied
your patch. Thanks.

[Arne Goedeke]

Martin Stjernholm, Roxen IS @ Pike developers forum wrote:
>> The send_output method which writes to the buffer triggers a write
>> attempt. So that should not be a problem.
>
> I see. Then I guess it works, although personally I think it's a bit
> obscure to rely on the internal write cb blocker. Anyway, I've applied
> your patch. Thanks.

Its what I usually do. So i guess it would be better to do a
set_write_callback(0) after write if !sizeof(out_buffer) and to
reinstall the callback when more data is available?

> So i guess it would be better to do a set_write_callback(0) after
> write if !sizeof(out_buffer) and to reinstall the callback when more
> data is available?

..and remove the initial callback install in create(). Yes, that is
what I'd do too.