Bugs item #2727981, was opened at 2009-04-03 11:15
Message generated for change (Comment added) made by bagder
You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=100976&aid=2727981&group_id=976

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: libcurl
Group: wrong behaviour
Status: Open
Resolution: None
Priority: 6
Private: No
Submitted By: Martin Storsjö (mstorsjo)
Assigned to: Daniel Stenberg (bagder)
Summary: Setting CURLOPT_NOBODY=0 resets httpreq to HTTPREQ_GET

Initial Comment:
If setting CURLOPT_NOBODY = 0, the httpreq internal state is reset to HTTPREQ_GET, even though it actually should be something else.

One possible scenario is if the user sets options in this order (it's perfectly reasonable to reset the _NOBODY setting if the curl handle has been used for other things earlier):

curl_easy_setopt(slot->curl, CURLOPT_PUT, 1);
curl_easy_setopt(slot->curl, CURLOPT_NOBODY, 0);

The first setopt call sets httpreq to HTTPREQ_PUT in lib/url.c:914 (in 7.19.4), but the second call resets httpreq to HTTPREQ_GET in lib/url.c:896.

One workaround is simply to do all reset/disable settings before setting the new options.

Is this a bug, or should the documentation be updated to reflect this?

A sample patch is attached, which solves this particular case but doesn't handle all possible cases. One case it doesn't handle is setting CURLOPT_POST=1, then CURLOPT_NOBODY=0, which would require even more state to be saved or better handling in url.c. Or what about setting e.g. CURLOPT_POST=1, CUROPT_NOBODY=1, CURLOPT_NOBODY=0 - should such scenarios be supported?

The API makes it look like all these options modify distinct separate flags, even though they all modify one shared variable (and only some of them are stored as flags).

----------------------------------------------------------------------

>Comment By: Daniel Stenberg (bagder)
Date: 2009-04-06 23:53

Message:
Right, so attached here is now my smaller take at basically not letting
NOBODY set the http request at all until perform() is about to run. This
allows for POST=1 NOBODY=0 without the latter ruining the post.

I didn't actually try the code (only compiled/built) so there might be a
blatant error in there.

----------------------------------------------------------------------

Comment By: Martin Storsjö (mstorsjo)
Date: 2009-04-06 17:07

Message:
Yeah, a proper fix probably is a bit larger change - that's why I didn't
send any readymade patch for that ;-)

Documentation of the current behaviour, and perhaps minor fixes along the
lines of my original patch is of course a good solution meanwhile.

----------------------------------------------------------------------

Comment By: Daniel Stenberg (bagder)
Date: 2009-04-05 23:44

Message:
Actually, doing it "the big change" will risk altering behavior as well and
thus it might hurt existing applications.

I think we should perhaps take this to the mailing list to see what people
think, but the most conservative thing to do is possibly to just clearly
and more accurately document the current behavior...

----------------------------------------------------------------------

Comment By: Daniel Stenberg (bagder)
Date: 2009-04-03 23:30

Message:
Thank you for your clear thoughts. You're of course right.

I'll work on a somewhat larger fix and submit it here for further
comments. It might just take me a day or two.

----------------------------------------------------------------------

Comment By: Martin Storsjö (mstorsjo)
Date: 2009-04-03 22:41

Message:
Yes, that's also a solution that would solve this particular case.

It would still not solve the generic API issue; the way the API is
written, one may believe that PUT=1, NOBODY=1, NOBODY=0 would yield the
same result as just PUT=1, even though NOBODY=1 sets httpreq to HEAD, and
NOBODY=0 sets it to GET. (I think the attached patch would get this one
right, but probably not all corner cases.) Nothing in the API docs hints
that the order of setting the options would matter (except for setting the
same option to different values, where the last value should be the one
that is used, of course).

The way the API is designed, the best way would probably be to store all
these settings as simple flags, without touching httpreq while they're set.
When the request actually is started, the flags would be evaluated and the
best matching httpreq would be selected. (This allows the user to set
conflicting sets of flags, though; what should happen if the user has set
HTTPGET=1, PUT=1, NOBODY=1?) In this way, the order different options are
set doesn't matter at all, the only thing that matters is the last value
that has been set to each option.

This would, unfortunately, require a bit larger rewrite than just simply
fixing this particular case I stumbled upon.

----------------------------------------------------------------------

Comment By: Daniel Stenberg (bagder)
Date: 2009-04-03 22:29

Message:
Hm, yes I agree that the current behavior is a bit annoying and disturbing
and we should do something about it.

The main reason NOBODY=0 sets the request type is that because previously
when someone did NOBODY=1 and then did NOBODY=0 on the same handle, it
would do mostly a GET but with a HEAD method which was completely wrong...

I'm now thinking that perhaps the fix is rather to if NOBODY is set to 0,
it should only change the method (to GET) if it is at that time set to
HEAD. If another method has been selected, it should just let that remain.
That would make your little snippet work and it "feel right" to me.

Or what to you think?

----------------------------------------------------------------------

You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=100976&aid=2727981&group_id=976
Received on 2009-04-06