hyper: root cause of curl test 565 failure

[pmk21]

I did this

Ran curl test 565, analyzed the debug trace and generated logs, concluded that the first POST request seems to be missing the terminating chunk which has to be inputted during the read callback. This is possibly causing some issues in hyper.

Initial request from server.input -

POST /565 HTTP/1.1
Host: 127.0.0.1:37051
Accept: */*
Transfer-Encoding: chunked
Content-Type: application/x-www-form-urlencoded

3
one
3
two
5
three
1D
and a final longer crap: four
0

Requests after fix -

POST /565 HTTP/1.1
Host: 127.0.0.1:42461
Accept: */*
Transfer-Encoding: chunked
Content-Type: application/x-www-form-urlencoded

0

POST /565 HTTP/1.1
Host: 127.0.0.1:42461
Authorization: Digest username="foo", realm="testrealm", nonce="1053604144", uri="/565", response="877424f750af047634dbd94f9933217b"
Accept: */*
Transfer-Encoding: chunked
Content-Type: application/x-www-form-urlencoded
Expect: 100-continue

3
one
3
two
5
three
1D
and a final longer crap: four
0

Patched the lib510.c file to allow the test to pass -

diff --git a/tests/libtest/lib510.c b/tests/libtest/lib510.c
index 2b8da207d..a5bd06a9c 100644
--- a/tests/libtest/lib510.c
+++ b/tests/libtest/lib510.c
@@ -36,6 +36,7 @@ struct WriteThis {
   int counter;
 };
 
+#if defined(USE_HYPER) && defined(LIB565)
 static size_t read_callback(char *ptr, size_t size, size_t nmemb, void *userp)
 {
   struct WriteThis *pooh = (struct WriteThis *)userp;
@@ -44,6 +45,11 @@ static size_t read_callback(char *ptr, size_t size, size_t nmemb, void *userp)
   if(size*nmemb < 1)
     return 0;
 
+  if(pooh->counter == -1) {
+    pooh->counter++;
+    return 0;
+  }
+
   data = post[pooh->counter];
 
   if(data) {
@@ -58,6 +64,30 @@ static size_t read_callback(char *ptr, size_t size, size_t nmemb, void *userp)
   }
   return 0;                         /* no more data left to deliver */
 }
+#else
+static size_t read_callback(char *ptr, size_t size, size_t nmemb, void *userp)
+{
+  struct WriteThis *pooh = (struct WriteThis *)userp;
+  const char *data;
+
+  if(size*nmemb < 1)
+    return 0;
+
+  data = post[pooh->counter];
+
+  if(data) {
+    size_t len = strlen(data);
+    if(size*nmemb < len) {
+      fprintf(stderr, "read buffer is too small to run test\n");
+      return 0;
+    }
+    memcpy(ptr, data, len);
+    pooh->counter++; /* advance pointer */
+    return len;
+  }
+  return 0;                         /* no more data left to deliver */
+}
+#endif
 
 int test(char *URL)
 {
@@ -65,7 +95,11 @@ int test(char *URL)
   CURLcode res = CURLE_OK;
   struct curl_slist *slist = NULL;
   struct WriteThis pooh;
+#if defined(USE_HYPER) && defined(LIB565)
+  pooh.counter = -1;
+#else
   pooh.counter = 0;
+#endif
 
   if(curl_global_init(CURL_GLOBAL_ALL) != CURLE_OK) {
     fprintf(stderr, "curl_global_init() failed\n");

curl/libcurl version

curl 7.83.1-DEV (x86_64-pc-linux-gnu) libcurl/7.83.1-DEV OpenSSL/1.1.1o zlib/1.2.12 brotli/1.0.9 zstd/1.5.2 libidn2/2.3.2 libpsl/0.21.1 (+libidn2/2.3.0) librtmp/2.3 Hyper/0.14.18 OpenLDAP/2.6.1
Release-Date: [unreleased]
Protocols: dict file ftp ftps gopher gophers http https imap imaps ldap ldaps mqtt pop3 pop3s rtmp smb smbs smtp smtps telnet tftp 
Features: alt-svc AsynchDNS brotli Debug HSTS HTTP2 HTTPS-proxy IDN Largefile libz NTLM NTLM_WB PSL SSL TLS-SRP TrackMemory UnixSockets zstd

operating system

Linux msi 5.15.38-1-MANJARO #1 SMP PREEMPT Mon May 9 07:52:21 UTC 2022 x86_64 GNU/Linux

[bagder]

Test 565 uses (explicit) Digest authentication. That's a multi-request mechanism and curl will then not issue the full request in the first request but just sends a zero length one to get the auth done, then send the full body in the second request.

When hyper is not used, libcurl does not call the callback until it wants to read (and send) actual data. Your research here shows that with hyper enabled, the read callback wrongly gets called already in the first request and then things go wrong.

[pmk21]

Thanks for the explanation!

I did some more digging and found this check in libcurl -

curl/lib/http.c

Lines 2663 to 2670 in 10cd696

	if(data->req.upload_chunky && conn->bits.authneg) {
	/* Chunky upload is selected and we're negotiating auth still, send
	end-of-data only */
	result = Curl_dyn_addn(r, (char *)STRCONST("\x30\x0d\x0a\x0d\x0a"));
	/* 0 CR LF CR LF */
	if(result)
	return result;
	}

I guess there has to be some similar logic in c-hyper.c as well. I found that data->req.upload_chunky is being set to false always.

curl/lib/c-hyper.c

Line 1132 in 10cd696

data->req.upload_chunky = FALSE;

Removing that line causes failures in other tests, so I added a messy fix -

--- a/lib/c-hyper.c
+++ b/lib/c-hyper.c
@@ -671,6 +671,7 @@ static int uploadstreamed(void *userdata, hyper_context *ctx,
 {
   size_t fillcount;
   struct Curl_easy *data = (struct Curl_easy *)userdata;
+  struct connectdata *conn = (struct connectdata *)data->conn;
   CURLcode result;
   (void)ctx;
 
@@ -685,7 +686,15 @@ static int uploadstreamed(void *userdata, hyper_context *ctx,
     return HYPER_POLL_PENDING;
   }
 
-  result = Curl_fillreadbuffer(data, data->set.upload_buffer_size, &fillcount);
+  if(data->req.upload_chunky && conn->bits.authneg) {
+    fillcount = 0;
+    data->req.upload_chunky = FALSE;
+    result = CURLE_OK;
+  }
+  else {
+    result = Curl_fillreadbuffer(data, data->set.upload_buffer_size,
+                                 &fillcount);
+  }
   if(result) {
     data->state.hresult = result;
     return HYPER_POLL_ERROR;
@@ -1129,7 +1138,12 @@ CURLcode Curl_http(struct Curl_easy *data, bool *done)
   if(result)
     goto error;
 
-  data->req.upload_chunky = FALSE;
+  if(data->req.upload_chunky && conn->bits.authneg) {
+    data->req.upload_chunky = TRUE;
+  }
+  else {
+    data->req.upload_chunky = FALSE;
+  }
   sendtask = hyper_clientconn_send(client, req);
   if(!sendtask) {
     failf(data, "hyper_clientconn_send");

Is there a better way to handle this?

[bagder]

I'm not sure there's a better way, but I think that if this works I think it is better to move ahead with this and enable the test rather than waiting and trying to find a nicer solution. Can you turn this into a PR?

[pmk21]

Yes, I'll open a PR.