{"thread":{"id":"26509","subject":"[PATCH] smart-http: Don't use Expect: 100-Continue","startedAt":"2011-02-15T16:57:24Z","lastAt":"2011-02-16T18:54:34Z","messageCount":5,"participants":["Shawn O. Pearce","Junio C Hamano","Shawn Pearce","Daniel Stenberg"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"161193","messageId":"1297789044-17978-1-git-send-email-spearce@spearce.org","threadId":"26509","inReplyTo":null,"subject":"[PATCH] smart-http: Don't use Expect: 100-Continue","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2011-02-15T16:57:24Z","receivedAt":"2011-02-15T16:57:24Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Some HTTP/1.1 servers or proxies don't correctly implement the\n100-Continue feature of HTTP/1.1.  Its a difficult feature to\nimplement right, and isn't commonly used by browsers, so many\ndevelopers may not even be aware that their server (or proxy)\ndoesn't honor it.\n\nWithin the smart HTTP protocol for Git we only use this newer\n\"Expect: 100-Continue\" feature to probe for missing authentication\nbefore uploading a large payload like a pack file during push.\nIf authentication is necessary, we expect the server to send the\n401 Not Authorized response before the bulk data transfer starts,\nthus saving the client bandwidth during the retry.\n\nA different method to probe for working authentication is to send an\nempty command list (that is just \"0000\") to $URL/git-receive-pack.\nor $URL/git-upload-pack.  All versions of both receive-pack and\nupload-pack since the introduction of smart HTTP in Git 1.6.6\ncleanly accept just a flush-pkt under --stateless-rpc mode, and\nexit with success.\n\nIf HTTP level authentication is successful, the backend will return\nan empty response, but with HTTP status code 200.  This enables\nthe client to continue with the transfer.\n\nSigned-off-by: Shawn O. Pearce <spearce@spearce.org>\n---\n remote-curl.c |   66 +++++++++++++++++++++++++++++++++++++++++++++++---------\n 1 files changed, 55 insertions(+), 11 deletions(-)\n\ndiff --git a/remote-curl.c b/remote-curl.c\nindex 04d4813..3d82dc2 100644\n--- a/remote-curl.c\n+++ b/remote-curl.c\n@@ -356,14 +356,59 @@ static size_t rpc_in(const void *ptr, size_t eltsize,\n \treturn size;\n }\n \n+static int run_slot(struct active_request_slot *slot)\n+{\n+\tint err = 0;\n+\tstruct slot_results results;\n+\n+\tslot->results = &results;\n+\tslot->curl_result = curl_easy_perform(slot->curl);\n+\tfinish_active_slot(slot);\n+\n+\tif (results.curl_result != CURLE_OK) {\n+\t\terr |= error(\"RPC failed; result=%d, HTTP code = %ld\",\n+\t\t\tresults.curl_result, results.http_code);\n+\t}\n+\n+\treturn err;\n+}\n+\n+static int probe_rpc(struct rpc_state *rpc)\n+{\n+\tstruct active_request_slot *slot;\n+\tstruct curl_slist *headers = NULL;\n+\tstruct strbuf buf = STRBUF_INIT;\n+\tint err;\n+\n+\tslot = get_active_slot();\n+\n+\theaders = curl_slist_append(headers, rpc->hdr_content_type);\n+\theaders = curl_slist_append(headers, rpc->hdr_accept);\n+\n+\tcurl_easy_setopt(slot->curl, CURLOPT_NOBODY, 0);\n+\tcurl_easy_setopt(slot->curl, CURLOPT_POST, 1);\n+\tcurl_easy_setopt(slot->curl, CURLOPT_URL, rpc->service_url);\n+\tcurl_easy_setopt(slot->curl, CURLOPT_ENCODING, \"\");\n+\tcurl_easy_setopt(slot->curl, CURLOPT_POSTFIELDS, \"0000\");\n+\tcurl_easy_setopt(slot->curl, CURLOPT_POSTFIELDSIZE, 4);\n+\tcurl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, headers);\n+\tcurl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite_buffer);\n+\tcurl_easy_setopt(slot->curl, CURLOPT_FILE, buf);\n+\n+\terr = run_slot(slot);\n+\n+\tcurl_slist_free_all(headers);\n+\tstrbuf_release(&buf);\n+\treturn err;\n+}\n+\n static int post_rpc(struct rpc_state *rpc)\n {\n \tstruct active_request_slot *slot;\n-\tstruct slot_results results;\n \tstruct curl_slist *headers = NULL;\n \tint use_gzip = rpc->gzip_request;\n \tchar *gzip_body = NULL;\n-\tint err = 0, large_request = 0;\n+\tint err, large_request = 0;\n \n \t/* Try to load the entire request, if we can fit it into the\n \t * allocated buffer space we can use HTTP/1.0 and avoid the\n@@ -386,8 +431,13 @@ static int post_rpc(struct rpc_state *rpc)\n \t\trpc->len += n;\n \t}\n \n+\tif (large_request) {\n+\t\terr = probe_rpc(rpc);\n+\t\tif (err)\n+\t\t\treturn err;\n+\t}\n+\n \tslot = get_active_slot();\n-\tslot->results = &results;\n \n \tcurl_easy_setopt(slot->curl, CURLOPT_NOBODY, 0);\n \tcurl_easy_setopt(slot->curl, CURLOPT_POST, 1);\n@@ -401,7 +451,7 @@ static int post_rpc(struct rpc_state *rpc)\n \t\t/* The request body is large and the size cannot be predicted.\n \t\t * We must use chunked encoding to send it.\n \t\t */\n-\t\theaders = curl_slist_append(headers, \"Expect: 100-continue\");\n+\t\theaders = curl_slist_append(headers, \"Expect:\");\n \t\theaders = curl_slist_append(headers, \"Transfer-Encoding: chunked\");\n \t\trpc->initial_buffer = 1;\n \t\tcurl_easy_setopt(slot->curl, CURLOPT_READFUNCTION, rpc_out);\n@@ -475,13 +525,7 @@ static int post_rpc(struct rpc_state *rpc)\n \tcurl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, rpc_in);\n \tcurl_easy_setopt(slot->curl, CURLOPT_FILE, rpc);\n \n-\tslot->curl_result = curl_easy_perform(slot->curl);\n-\tfinish_active_slot(slot);\n-\n-\tif (results.curl_result != CURLE_OK) {\n-\t\terr |= error(\"RPC failed; result=%d, HTTP code = %ld\",\n-\t\t\tresults.curl_result, results.http_code);\n-\t}\n+\terr = run_slot(slot);\n \n \tcurl_slist_free_all(headers);\n \tfree(gzip_body);\n-- \n1.7.4.rc3.268.g2af8b\n"},{"id":"161205","messageId":"7vr5b9nkzb.fsf@alter.siamese.dyndns.org","threadId":"26509","inReplyTo":"1297789044-17978-1-git-send-email-spearce@spearce.org","subject":"Re: [PATCH] smart-http: Don't use Expect: 100-Continue","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-02-15T19:42:00Z","receivedAt":"2011-02-15T19:42:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Shawn O. Pearce\" <spearce@spearce.org> writes:\n\n> diff --git a/remote-curl.c b/remote-curl.c\n> index 04d4813..3d82dc2 100644\n> --- a/remote-curl.c\n> +++ b/remote-curl.c\n> @@ -356,14 +356,59 @@ static size_t rpc_in(const void *ptr, size_t eltsize,\n> ...\n> +static int probe_rpc(struct rpc_state *rpc)\n> +{\n> +...\n> +\tcurl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite_buffer);\n> +\tcurl_easy_setopt(slot->curl, CURLOPT_FILE, buf);\n> +\n> +\terr = run_slot(slot);\n> +\n> +\tcurl_slist_free_all(headers);\n> +\tstrbuf_release(&buf);\n> +\treturn err;\n> +}\n\nHmm, I am getting\n\n    remote-curl.c:403: error: call to '_curl_easy_setopt_err_cb_data' declared\n    with attribute warning: curl_easy_setopt expects a private data pointer as\n    argument for this option\n\nShouldn't the above be giving a pointer to buf anyway?\n\n remote-curl.c |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/remote-curl.c b/remote-curl.c\nindex 297ecf7..256326a 100644\n--- a/remote-curl.c\n+++ b/remote-curl.c\n@@ -400,7 +400,7 @@ static int probe_rpc(struct rpc_state *rpc)\n \tcurl_easy_setopt(slot->curl, CURLOPT_POSTFIELDSIZE, 4);\n \tcurl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, headers);\n \tcurl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite_buffer);\n-\tcurl_easy_setopt(slot->curl, CURLOPT_FILE, buf);\n+\tcurl_easy_setopt(slot->curl, CURLOPT_FILE, &buf);\n \n \terr = run_slot(slot);\n \n"},{"id":"161241","messageId":"AANLkTikVp0xC3OciJ7eN=P4+5_Pu=KPeO5X_+b_Nv30N@mail.gmail.com","threadId":"26509","inReplyTo":"7vr5b9nkzb.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] smart-http: Don't use Expect: 100-Continue","fromName":"Shawn Pearce","fromEmail":"spearce@spearce.org","sentAt":"2011-02-15T23:54:28Z","receivedAt":"2011-02-15T23:54:28Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"On Tue, Feb 15, 2011 at 11:42, Junio C Hamano <gitster@pobox.com> wrote:\n> \"Shawn O. Pearce\" <spearce@spearce.org> writes:\n>\n>> diff --git a/remote-curl.c b/remote-curl.c\n>> index 04d4813..3d82dc2 100644\n>> --- a/remote-curl.c\n>> +++ b/remote-curl.c\n>> @@ -356,14 +356,59 @@ static size_t rpc_in(const void *ptr, size_t eltsize,\n>> ...\n>> +static int probe_rpc(struct rpc_state *rpc)\n>> +{\n>> +...\n>> +     curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite_buffer);\n>> +     curl_easy_setopt(slot->curl, CURLOPT_FILE, buf);\n>> +\n>> +     err = run_slot(slot);\n>> +\n>> +     curl_slist_free_all(headers);\n>> +     strbuf_release(&buf);\n>> +     return err;\n>> +}\n>\n> Hmm, I am getting\n>\n>    remote-curl.c:403: error: call to '_curl_easy_setopt_err_cb_data' declared\n>    with attribute warning: curl_easy_setopt expects a private data pointer as\n>    argument for this option\n>\n> Shouldn't the above be giving a pointer to buf anyway?\n\nYes.  Please squash your patch into mine.  I'm surprised my build\ndoesn't have sufficient warning flags enabled when I built this. :-(\n\n-- \nShawn.\n"},{"id":"161263","messageId":"alpine.DEB.2.00.1102160836360.20870@tvnag.unkk.fr","threadId":"26509","inReplyTo":"AANLkTikVp0xC3OciJ7eN=P4+5_Pu=KPeO5X_+b_Nv30N@mail.gmail.com","subject":"Re: [PATCH] smart-http: Don't use Expect: 100-Continue","fromName":"Daniel Stenberg","fromEmail":"daniel@haxx.se","sentAt":"2011-02-16T07:38:50Z","receivedAt":"2011-02-16T07:38:50Z","isPatch":true,"sender":{"key":"daniel@haxx.se","avatar":"https://gravatar.com/avatar/69fdca87edd17cee21ca2e79fc2ff671d644603c3dc27167430f3cd3dbab7ba8?d=mp&s=160"},"body":"On Tue, 15 Feb 2011, Shawn Pearce wrote:\n\n>>    remote-curl.c:403: error: call to '_curl_easy_setopt_err_cb_data' declared\n>>    with attribute warning: curl_easy_setopt expects a private data pointer as\n>>    argument for this option\n>\n> I'm surprised my build doesn't have sufficient warning flags enabled when I \n> built this. :-(\n\nThose are warnings generated by the macro magic in curl's typecheck-gcc.h \nheader file, and they require gcc 4.3 or later so perhaps you used an older \ncompiler?\n\n-- \n\n  / daniel.haxx.se"},{"id":"161323","messageId":"7vlj1fkdxx.fsf@alter.siamese.dyndns.org","threadId":"26509","inReplyTo":"alpine.DEB.2.00.1102160836360.20870@tvnag.unkk.fr","subject":"Re: [PATCH] smart-http: Don't use Expect: 100-Continue","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-02-16T18:54:34Z","receivedAt":"2011-02-16T18:54:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Daniel Stenberg <daniel@haxx.se> writes:\n\n> On Tue, 15 Feb 2011, Shawn Pearce wrote:\n>> I'm surprised my build doesn't have sufficient warning flags enabled\n>> when I built this. :-(\n>\n> Those are warnings generated by the macro magic in curl's\n> typecheck-gcc.h header file, and they require gcc 4.3 or later...\n\nYes, and thanks for putting them in.  I wouldn't have caught this myself\nby a mere code inspection, as I tend to skim over patches from people with\nskills known to be above a certain threshold.\n"}]}