Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CURLOPT_POST issue after CURLOPT_UPLOAD #9507

Closed
RobBotic1 opened this issue Sep 14, 2022 · 7 comments
Closed

CURLOPT_POST issue after CURLOPT_UPLOAD #9507

RobBotic1 opened this issue Sep 14, 2022 · 7 comments
Assignees
Labels

Comments

@RobBotic1
Copy link

RobBotic1 commented Sep 14, 2022

I did this

The following snippet recreates the behavior with minimal error handling for readability:

#include <stdio.h>
#include <string.h>
#include <curl/curl.h>

typedef struct
{
    char *buf;
    size_t len;
} put_buffer;

static size_t put_callback(char *ptr, size_t size, size_t nmemb, void *stream)
{
    put_buffer *putdata = (put_buffer *)stream;
    size_t totalsize = size * nmemb;
    size_t tocopy = (putdata->len < totalsize) ? putdata->len : totalsize;
    memcpy(ptr, putdata->buf, tocopy);
    putdata->len -= tocopy;
    putdata->buf += tocopy;
    return tocopy;
}

int main()
{
    CURL *curl = NULL;
    char *testput = "This is test PUT data";
    put_buffer pbuf = {};

    curl_global_init(CURL_GLOBAL_DEFAULT);

    curl = curl_easy_init();

    // PUT
    curl_easy_setopt(curl, CURLOPT_UPLOAD, 1L);
    curl_easy_setopt(curl, CURLOPT_READFUNCTION, put_callback);
    pbuf.buf = testput;
    pbuf.len = strlen(testput);
    curl_easy_setopt(curl, CURLOPT_READDATA, &pbuf);
    curl_easy_setopt(curl, CURLOPT_INFILESIZE, strlen(testput));
    curl_easy_setopt(curl, CURLOPT_URL, "http://localhost:5000/puttest");
    curl_easy_perform(curl);

    // Without this line, I sent a PUT instead of a POST below
    //curl_easy_setopt(curl, CURLOPT_UPLOAD, 0L);

    // POST (will be a PUT without the line just above)
    curl_easy_setopt(curl, CURLOPT_POST, 1L);
    curl_easy_setopt(curl, CURLOPT_POSTFIELDS, testput);
    curl_easy_setopt(curl, CURLOPT_POSTFIELDSIZE, strlen(testput));
    curl_easy_setopt(curl, CURLOPT_URL, "http://localhost:5000/posttest");
    curl_easy_perform(curl);

    curl_easy_cleanup(curl);

    curl_global_cleanup();

    return 0;
}

I expected the following

I expected using CURLOPT_POST to cause an HTTP POST. However, it appears that the previously set CURLOPT_UPLOAD overrides this behavior and causes an HTTP PUT to occur. I would expect CURLOPT_POST to unset HTTP PUT as well as HTTP GET and HTTP HEAD (much like setting CURLOPT_HTTPGET overrides previously set CURLOPT_NOBODY and CURLOPT_UPLOAD).

curl/libcurl version

curl 7.85.0 (aarch64-apple-darwin21.6.0) libcurl/7.85.0 (SecureTransport) OpenSSL/1.1.1q zlib/1.2.11 brotli/1.0.9 zstd/1.5.2 libidn2/2.3.3 libssh2/1.10.0 nghttp2/1.49.0 librtmp/2.3
Release-Date: 2022-08-31
Protocols: dict file ftp ftps gopher gophers http https imap imaps ldap ldaps mqtt pop3 pop3s rtmp rtsp scp sftp smb smbs smtp smtps telnet tftp
Features: alt-svc AsynchDNS brotli GSS-API HSTS HTTP2 HTTPS-proxy IDN IPv6 Kerberos Largefile libz MultiSSL NTLM NTLM_WB SPNEGO SSL threadsafe TLS-SRP UnixSockets zstd

operating system

macOS Monterey

@RobBotic1
Copy link
Author

I believe this may be the issue.

CURLOPT_POST does not touch the upload flag:

curl/lib/setopt.c

Lines 693 to 703 in ddda4fd

case CURLOPT_POST:
/* Does this option serve a purpose anymore? Yes it does, when
CURLOPT_POSTFIELDS isn't used and the POST data is read off the
callback! */
if(va_arg(param, long)) {
data->set.method = HTTPREQ_POST;
data->set.opt_no_body = FALSE; /* this is implied */
}
else
data->set.method = HTTPREQ_GET;
break;

But when the upload flag is set the method is changed to PUT:

curl/lib/http.c

Lines 2101 to 2108 in c616259

void Curl_http_method(struct Curl_easy *data, struct connectdata *conn,
const char **method, Curl_HttpReq *reqp)
{
Curl_HttpReq httpreq = (Curl_HttpReq)data->state.httpreq;
const char *request;
if((conn->handler->protocol&(PROTO_FAMILY_HTTP|CURLPROTO_FTP)) &&
data->set.upload)
httpreq = HTTPREQ_PUT;

@bagder bagder added the HTTP label Sep 15, 2022
@bagder bagder self-assigned this Sep 15, 2022
bagder added a commit that referenced this issue Sep 15, 2022
Reported-by: RobBotic1 on github
Fixes #9507
bagder added a commit that referenced this issue Sep 15, 2022
@bagder bagder linked a pull request Sep 15, 2022 that will close this issue
@bagder
Copy link
Member

bagder commented Sep 15, 2022

Thanks for the excellent reproducer. I successfully converted it into a curl test case and could reproduce the problem and then verify my fix, all based on your work here.

bagder added a commit that referenced this issue Sep 15, 2022
Reported-by: RobBotic1 on github
Fixes #9507
Closes #9511
bagder added a commit that referenced this issue Sep 15, 2022
@RobBotic1
Copy link
Author

Thanks! I'm thrilled that I could help!

As I see (for HTTP) CURLOPT_HTTPGET, CURLOPT_NOBODY, CURLOPT_UPLOAD, and CURLOPT_POST as mutually exclusive (I'm not certain about CURLOPT_CUSTOMREQUEST), I do have a question and two suggestions:

  • Should CURLOPT_NOBODY also be updated to set upload to false?

    curl/lib/setopt.c

    Lines 342 to 354 in b98d45b

    case CURLOPT_NOBODY:
    /*
    * Do not include the body part in the output data stream.
    */
    data->set.opt_no_body = (0 != va_arg(param, long)) ? TRUE : FALSE;
    #ifndef CURL_DISABLE_HTTP
    if(data->set.opt_no_body)
    /* in HTTP lingo, no body means using the HEAD request... */
    data->set.method = HTTPREQ_HEAD;
    else if(data->set.method == HTTPREQ_HEAD)
    data->set.method = HTTPREQ_GET;
    #endif
    break;
  • I would suggest updating the man page for CURLOPT_HTTPGET highlighting that it overrides CURLOPT_POST
    When setting \fICURLOPT_HTTPGET(3)\fP to 1, it will automatically set
    \fICURLOPT_NOBODY(3)\fP to 0 and \fICURLOPT_UPLOAD(3)\fP to 0.
  • I would suggest updating the man page for CURLOPT_POST (highlighting that it sets CURLOPT_UPLOAD to 0 and noting this issue for previous versions where CURLOPT_UPLOAD must be explicitly set to 0).
    When setting \fICURLOPT_POST(3)\fP to 1, libcurl will automatically set
    \fICURLOPT_NOBODY(3)\fP and \fICURLOPT_HTTPGET(3)\fP to 0.

@dfandrich
Copy link
Contributor

dfandrich commented Sep 15, 2022 via email

@RobBotic1
Copy link
Author

I was looking at this as CURLOPT_NOBODY essentially maps to HTTP HEAD, per

output when doing what would otherwise be a download. For HTTP(S), this makes
libcurl do a HEAD request. For most other protocols it means just not asking
to transfer the body data.

and
If you do a transfer with HTTP that involves a method other than HEAD, you
will get a body (unless the resource and server sends a zero byte body for the
specific URL you request).

I'd also note that CURLOPT_POST already disables CURLOPT_NOBODY, so this would be consistent for a PUT (CURLOPT_UPLOAD).

That said, a mechanism to request no response body would be useful.

@dfandrich
Copy link
Contributor

dfandrich commented Sep 15, 2022 via email

@bagder bagder closed this as completed in a64e3e5 Sep 15, 2022
bagder added a commit that referenced this issue Sep 15, 2022
@bagder
Copy link
Member

bagder commented Sep 15, 2022

UPLOAD over HTTP means PUT, there is no way to ask for a PUT without a body in HTTP so curl cannot do that on itself.

If you want to stop reading at some point, you do that by returning an error from the write callback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging a pull request may close this issue.

3 participants