Le 12 janv. 2014 à 00:19, Daniel Stenberg <daniel_at_haxx.se> a écrit :

>> The question is: should we prevent libcurl to send the request by failing early?
>
> Yes I think so. Sending -1 is plain wrong.

After a quick code review I have several questions I would like to discuss:

I compared what happens with an HTTP PUT request if the caller forgot to set the expected size via `CURLOPT_INFILESIZE`: in that case, the Content-Length header is omitted (and thus libcurl does not send any negative Content-Length).

In other words, this means the HTTP PUT[1] and HTTP POST[2] behaviors are different.

=> I would say they should be the same, and this is what should be fixed instead of failing early as I first suggested. Do you agree?

According to RFC2616 I have the feeling that what is done for PUT is a better behavior since:
* a negative Content-Length is clearly invalid - which is what HTTP POST does so far,
* not sending the Content-Length is permitted - which is what HTTP PUT does so far.

The section 4.4 describes what it implies in the latter case, in particular:

    If a request contains a message-body and a Content-Length is not given, the
    server SHOULD respond with 400 (bad request) if it cannot determine the
    length of the message, or with 411 (length required) if it wishes to insist
    on receiving a valid Content-Length.

(so if the caller forgot to pass an explicit size when appropriate, we let the server reply with an ad hoc error)

=> So I would say we should fix the POST check[2] by replacing it with the PUT one (that does NOT send the Content-Length header in case of negative post size). Do you agree?

If yes, then there is a last subtlety for auth requests:

The Content-Length is explicitly set to 0 by libcurl during the auth negotiation (i.e before the final request). Here again the behavior between PUT and POST are different:
* for HTTP POST, libcurl force a Content-Length: 0 even though the caller has set its own Content-Length header,
* this is not the case for HTTP PUT (i.e. if a user-defined Content-Length header exists, it is set even at auth negotiation time).

=> I assume the HTTP POST behavior is what should be done. Do you agree?

> It'd also be great to have a test case or two for this. See tests/libtest/ for inspiration!

Sure! The tests/README documentation is helpful. Also, I've bookmarked a recent commit that acts as a perfect example[3].

Cedric

[1]: https://github.com/bagder/curl/blob/a33e7ed/lib/http.c#L2365-L2373
[2]: https://github.com/bagder/curl/blob/a33e7ed/lib/http.c#L2411-L2425
[3]: https://github.com/bagder/curl/commit/62da1e7

-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette: http://curl.haxx.se/mail/etiquette.html
Received on 2014-01-12