{"thread":{"id":"64648","subject":"[PATCH 0/3] test-suite fixes for upcoming curl 8.18.0","startedAt":"2025-12-18T12:11:29Z","lastAt":"2025-12-20T02:14:26Z","messageCount":14,"participants":["Jeff King","Daniel Stenberg","Matthew John Cheetham","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"532462","messageId":"20251218121120.GA3252258@coredump.intra.peff.net","threadId":"64648","inReplyTo":null,"subject":"[PATCH 0/3] test-suite fixes for upcoming curl 8.18.0","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2025-12-18T12:11:20Z","receivedAt":"2025-12-18T12:11:29Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"After upgrading my Debian unstable box to libcurl 8.18.0~rc2-1, I\nnoticed a few new test failures. They seem to mostly be caused by\nbrittle expectations in the tests. These patches fix the tests to handle\nboth old and new versions (I tested against curl's 8_17_0 and\nrc-8_18_0-2 tags).\n\nDaniel: I'm cc-ing you in case you want to double-check that curl's\nbehavior changes are all OK before the release. I think it's mostly\nfine, though the handling of tab versus space in the third patch is\nperhaps questionable.\n\nMatthew: I had to make some educated guesses about one of the tests in\npatch 2. You might remember the original intent.\n\n  [1/3]: t5551: handle trailing slashes in expected cookies output\n  [2/3]: t5563: add missing end-of-line in HTTP header\n  [3/3]: t5563: relax whitespace assumptions for unfolded headers\n\n t/t5551-http-fetch-smart.sh | 15 +++++++++------\n t/t5563-simple-http-auth.sh | 15 +++++++++++----\n 2 files changed, 20 insertions(+), 10 deletions(-)\n\n-Peff\n"},{"id":"532463","messageId":"20251218121347.GA3758205@coredump.intra.peff.net","threadId":"64648","inReplyTo":"20251218121120.GA3252258@coredump.intra.peff.net","subject":"[PATCH 1/3] t5551: handle trailing slashes in expected cookies output","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2025-12-18T12:13:47Z","receivedAt":"2025-12-18T12:13:48Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"We check in t5551 that curl updates the expected list of cookies after\nmaking a request. We do this by telling it to read and write cookies\nfrom a particular text file, and then checking that after curl runs, the\nfile has the expected content.\n\nHowever, in the upcoming curl 8.18.0, the output file has changed\nslightly: curl will canonicalize the paths it writes, due to commit\na093c93994 (cookie: only keep and use the canonical cleaned up path,\n2025-12-07). In particular, it strips trailing slashes from the paths we\nsee in the cookies.txt file.\n\nThis doesn't matter to Git, as the cookie handling is all internal to\ncurl. But our test is overly brittle and breaks as a result.\n\nWe can fix it by matching either format. We'll expect the new format\n(without trailing slashes) and strip the slashes from curl's output\nbefore comparing. That lets us pass with both old and new versions (I\ntested against curl's 8_17_0 and rc-8_18_0-2 tags, which are\nrespectively before and after the curl change).\n\nIn theory it might be nice to try to future-proof this test more by\nlooking only for the bits we care about, rather than a byte-wise\ncomparison of the whole file. But after removing comments and blank\nlines (which we already do), we care about most of what's there. So it's\nnot clear to me what a more liberal test would look like. Given that the\nformat doesn't change all that often, it's probably OK to stop here and\nsee if it ever breaks again.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n t/t5551-http-fetch-smart.sh | 15 +++++++++------\n 1 file changed, 9 insertions(+), 6 deletions(-)\n\ndiff --git a/t/t5551-http-fetch-smart.sh b/t/t5551-http-fetch-smart.sh\nindex b0d4ea7801..73cf531580 100755\n--- a/t/t5551-http-fetch-smart.sh\n+++ b/t/t5551-http-fetch-smart.sh\n@@ -333,12 +333,12 @@ test_expect_success 'dumb clone via http-backend respects namespace' '\n \n test_expect_success 'cookies stored in http.cookiefile when http.savecookies set' '\n \tcat >cookies.txt <<-\\EOF &&\n-\t127.0.0.1\tFALSE\t/smart_cookies/\tFALSE\t0\tothername\tothervalue\n+\t127.0.0.1\tFALSE\t/smart_cookies\tFALSE\t0\tothername\tothervalue\n \tEOF\n \tsort >expect_cookies.txt <<-\\EOF &&\n-\t127.0.0.1\tFALSE\t/smart_cookies/\tFALSE\t0\tothername\tothervalue\n-\t127.0.0.1\tFALSE\t/smart_cookies/repo.git/\tFALSE\t0\tname\tvalue\n-\t127.0.0.1\tFALSE\t/smart_cookies/repo.git/info/\tFALSE\t0\tname\tvalue\n+\t127.0.0.1\tFALSE\t/smart_cookies\tFALSE\t0\tothername\tothervalue\n+\t127.0.0.1\tFALSE\t/smart_cookies/repo.git\tFALSE\t0\tname\tvalue\n+\t127.0.0.1\tFALSE\t/smart_cookies/repo.git/info\tFALSE\t0\tname\tvalue\n \tEOF\n \tgit config http.cookiefile cookies.txt &&\n \tgit config http.savecookies true &&\n@@ -351,8 +351,11 @@ test_expect_success 'cookies stored in http.cookiefile when http.savecookies set\n \t\ttag -m \"foo\" cookie-tag &&\n \tgit fetch $HTTPD_URL/smart_cookies/repo.git cookie-tag &&\n \n-\tgrep \"^[^#]\" cookies.txt | sort >cookies_stripped.txt &&\n-\ttest_cmp expect_cookies.txt cookies_stripped.txt\n+\t# Strip trailing slashes from cookie paths to handle output from both\n+\t# old curl (\"/smart_cookies/\") and new (\"/smart_cookies\").\n+\tHT=\"\t\" &&\n+\tgrep \"^[^#]\" cookies.txt | sed \"s,/$HT,$HT,\" | sort >cookies_clean.txt &&\n+\ttest_cmp expect_cookies.txt cookies_clean.txt\n '\n \n test_expect_success 'transfer.hiderefs works over smart-http' '\n-- \n2.52.0.595.gac9d83db54\n\n"},{"id":"532464","messageId":"20251218121819.GB3758205@coredump.intra.peff.net","threadId":"64648","inReplyTo":"20251218121120.GA3252258@coredump.intra.peff.net","subject":"[PATCH 2/3] t5563: add missing end-of-line in HTTP header","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2025-12-18T12:18:19Z","receivedAt":"2025-12-18T12:18:21Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"In t5563, we test how various oddly-formatted WWW-Authenticate headers\nare passed through curl to git's credential subsystem (and ultimately\nout to credential helpers). One test, \"access using basic auth with\nwwwauth header mixed line-endings\" does something odd. It does not mix\nline endings at all (which must be CRLF according to the RFC anyway),\nbut omits the line ending entirely for the final header!\n\nThis means that the server produces an incomplete response. We send our\nfinal header, and then the newline which is meant to mark the end of\nheaders (and the start of the body) becomes the line ending for that\nheader. And there is no header/body separator in the output at all.\n\nLooking at strace, this is what the client reads:\n\n  recvfrom(9, \"WWW-Authenticate: FooBar param1=\\\"value1\\\"\\r\\n \\r\\n\\tparam2=\\\"value2\\\"\\r\\nWWW-Authenticate: Basic realm=\\\"example.com\\\"\", 16384, 0, NULL, NULL) = 106\n  recvfrom(9, \"\\n\", 16384, 0, NULL, NULL) = 1\n  recvfrom(9, \"\", 16384, 0, NULL, NULL) = 0\n\nThe headers themselves are produced from the custom-auth.challenge file\nwe write in the test (which is missing the final CRLF), and then the\nheader/body separator comes from our lib-httpd/nph-custom-auth.sh CGI.\n(Ignore for a moment that it is producing a bare newline, which I think\nis a bug; it should be a CRLF but curl is happy with either).\n\nOlder versions of curl seemed to be OK with the truncated output, but\nthe upcoming 8.18.0 release seems to get confused. Specifically, since\n67ae101666 (http: unfold response headers earlier, 2025-12-12) our\nrequest to the server fails with insufficient credentials. I traced far\nenough to see that curl does relay the header back to us, which we then\npass to a credential helper, which gives us the correct\nusername/password combination. But on our followup request, curl refuses\nto send the Authorization header (and so gets an HTTP 401 again).\n\nThe change in curl's behavior is a bit unexpected, but since we are\nsending it garbage, it is hard to complain too much. Let's add the\nmissing CRLF to the header. I _think_ this was just an oversight and not\nthe intent of the test. And that the \"mixed line-endings\" really meant\n\"mixed continuations\", since we differ from the previous test in\ncontinuing with both space and tab. So I've likewise updated the test\ntitle to match that assumption.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nI do find it puzzling that we hand curl the credential, but it doesn't\nget used in the follow-up request. So I may have mis-analyzed something,\nbut I really think that's what is happening. I can share the\nhacky instrumentation I added if anybody wants to dig further. But since\nthe original was garbage AFAICT, I didn't think it was worth spending\na lot of time on it.\n\n t/t5563-simple-http-auth.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t5563-simple-http-auth.sh b/t/t5563-simple-http-auth.sh\nindex 317f33af5a..c1febbae9d 100755\n--- a/t/t5563-simple-http-auth.sh\n+++ b/t/t5563-simple-http-auth.sh\n@@ -469,7 +469,7 @@ test_expect_success 'access using basic auth with wwwauth header empty continuat\n \tEOF\n '\n \n-test_expect_success 'access using basic auth with wwwauth header mixed line-endings' '\n+test_expect_success 'access using basic auth with wwwauth header mixed continuations' '\n \ttest_when_finished \"per_test_cleanup\" &&\n \n \tset_credential_reply get <<-EOF &&\n@@ -490,7 +490,7 @@ test_expect_success 'access using basic auth with wwwauth header mixed line-endi\n \tprintf \"id=default response=WWW-Authenticate: FooBar param1=\\\"value1\\\"\\r\\n\" >>\"$CHALLENGE\" &&\n \tprintf \"id=default response= \\r\\n\" >>\"$CHALLENGE\" &&\n \tprintf \"id=default response=\\tparam2=\\\"value2\\\"\\r\\n\" >>\"$CHALLENGE\" &&\n-\tprintf \"id=default response=WWW-Authenticate: Basic realm=\\\"example.com\\\"\" >>\"$CHALLENGE\" &&\n+\tprintf \"id=default response=WWW-Authenticate: Basic realm=\\\"example.com\\\"\\r\\n\" >>\"$CHALLENGE\" &&\n \n \ttest_config_global credential.helper test-helper &&\n \tgit ls-remote \"$HTTPD_URL/custom_auth/repo.git\" &&\n-- \n2.52.0.595.gac9d83db54\n\n"},{"id":"532465","messageId":"20251218122204.GC3758205@coredump.intra.peff.net","threadId":"64648","inReplyTo":"20251218121120.GA3252258@coredump.intra.peff.net","subject":"[PATCH 3/3] t5563: relax whitespace assumptions for unfolded headers","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2025-12-18T12:22:04Z","receivedAt":"2025-12-18T12:22:05Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"In t5563 we check the handling of WWW-Authenticate headers that have\nbeen folded (i.e., where a continuation line starts with extra\nwhitespace). Traditionally curl handed each line to us individually, but\nin the upcoming v8.18.0, it hands us full lines that have been unfolded.\nBut it doesn't produce exactly the same unfolding that we did!\n\nIn particular, two of the tests send an extra blank continuation line.\nSomething like this:\n\n  printf 'WWW-Authenticate: foo param1=\"value1\"\\r\\n'\n  printf ' \\r\\n'\n  printf ' param2=\"value2\"\\r\\n\"\n\nWe unfold that into:\n\n  WWW-Authenticate: foo param1=\"value1\" param2=\"value2\"\n\nBut curl will give us a string with an extra space:\n\n  WWW-Authenticate: foo param1=\"value1\"  param2=\"value2\"\n\nI think curl is actually correct here. RFC 7230 says:\n\n   A user agent that receives an obs-fold in a response message that is\n   not within a message/http container MUST replace each received\n   obs-fold with one or more SP octets prior to interpreting the field\n   value.\n\nSo each folded instance turns the initial whitespace into \"one or more\"\nspaces, and the \"blank\" line becomes a single space. Whereas Git's\nunfolding code explicitly avoids this, with the comment \"Do not bother\nappending the new value if this continuation header is itself empty.\" in\nfwrite_wwwauth().\n\nI think it's mostly academic at this point. These folded continuations\nhave been deprecated entirely since RFC 7230 came out in 2014, and\nthere's very little reason for a server to add a blank continuation line\nat all. And anybody parsing the unfolded header contents should skip\npast the extra whitespace (which is allowed to be present according to\nthe RFC).\n\nBut our tests do a byte-wise comparison, so they care about the\ndifference between the two outputs. We have two options here:\n\n  1. We can modify Git's unfolding code to behave like modern curl.\n\n  2. We can relax the tests to be happy with either output.\n\nI picked (2) here, just because it seemed less risky to touch only the\ntests and not the code (though if any real-world systems _do_ care about\nthe distinction, they will eventually run into problems when libcurl is\nupgraded).\n\nThere is one further curiosity here. There's a second test which mixes\ntabs and spaces for continuation, like this:\n\n  printf 'WWW-Authenticate: foo param1=\"value1\"\\r\\n'\n  printf '\\t\\r\\n'\n  printf ' param2=\"value2\"\\r\\n\"\n\nFrom the snippet of RFC quoted above, I believe this should produce the\nexact same output (the continuation whitespace is replaced with one or\nmore spaces, even though it is a tab here). But curl retains the tab\ninstead!\n\nSo to implement the \"relaxed whitespace\" mode in the test, we just\nconvert any run of multiple whitespace characters to a single space.\nThis is a bit hacky and over-zealous, but it's easy to do and good\nenough for our purposes here. We only enable the relaxed mode for the\ntwo tests which trigger this issue.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nNote that when built against this new version of curl, Git's unfolding\ncode should never trigger at all. In the long run we should be able to\nrip it out, but we probably need to wait a decade or so before we can\nbump the minimum libcurl version to 8.18.0.\n\nI guess we could make it a conditional in the code (which would help us\nremember to eventually rip it out), but it felt weird to start adding\nversion conditionals for a version that isn't even released yet. ;)\n\n t/t5563-simple-http-auth.sh | 11 +++++++++--\n 1 file changed, 9 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t5563-simple-http-auth.sh b/t/t5563-simple-http-auth.sh\nindex c1febbae9d..0967cd501c 100755\n--- a/t/t5563-simple-http-auth.sh\n+++ b/t/t5563-simple-http-auth.sh\n@@ -47,6 +47,13 @@ set_credential_reply () {\n expect_credential_query () {\n \tlocal suffix=\"$(test -n \"$2\" && echo \"-$2\")\"\n \tcat >\"$TRASH_DIRECTORY/$1-expect$suffix.cred\" &&\n+\tif $(test \"$3\" = \"--relax-whitespace\")\n+\tthen\n+\t\tHT='\t' &&\n+\t\tsed \"s/[ $HT][ $HT]*/ /g\" \\\n+\t\t\t<\"$TRASH_DIRECTORY/$1-query$suffix.cred\" >tmp &&\n+\t\tmv tmp \"$TRASH_DIRECTORY/$1-query$suffix.cred\"\n+\tfi &&\n \ttest_cmp \"$TRASH_DIRECTORY/$1-expect$suffix.cred\" \\\n \t\t \"$TRASH_DIRECTORY/$1-query$suffix.cred\"\n }\n@@ -451,7 +458,7 @@ test_expect_success 'access using basic auth with wwwauth header empty continuat\n \ttest_config_global credential.helper test-helper &&\n \tgit ls-remote \"$HTTPD_URL/custom_auth/repo.git\" &&\n \n-\texpect_credential_query get <<-EOF &&\n+\texpect_credential_query get \"\" --relax-whitespace <<-EOF &&\n \tcapability[]=authtype\n \tcapability[]=state\n \tprotocol=http\n@@ -495,7 +502,7 @@ test_expect_success 'access using basic auth with wwwauth header mixed continuat\n \ttest_config_global credential.helper test-helper &&\n \tgit ls-remote \"$HTTPD_URL/custom_auth/repo.git\" &&\n \n-\texpect_credential_query get <<-EOF &&\n+\texpect_credential_query get \"\" --relax-whitespace <<-EOF &&\n \tcapability[]=authtype\n \tcapability[]=state\n \tprotocol=http\n-- \n2.52.0.595.gac9d83db54\n"},{"id":"532466","messageId":"613s97no-7021-pp15-79s4-302o39p7n5r8@unkk.fr","threadId":"64648","inReplyTo":"20251218121120.GA3252258@coredump.intra.peff.net","subject":"Re: [PATCH 0/3] test-suite fixes for upcoming curl 8.18.0","fromName":"Daniel Stenberg","fromEmail":"daniel@haxx.se","sentAt":"2025-12-18T12:37:11Z","receivedAt":"2025-12-18T12:46:33Z","isPatch":true,"sender":{"key":"daniel@haxx.se","avatar":"https://gravatar.com/avatar/69fdca87edd17cee21ca2e79fc2ff671d644603c3dc27167430f3cd3dbab7ba8?d=mp&s=160"},"body":"On Thu, 18 Dec 2025, Jeff King wrote:\n\n> Daniel: I'm cc-ing you in case you want to double-check that curl's behavior \n> changes are all OK before the release. I think it's mostly fine, though the \n> handling of tab versus space in the third patch is perhaps questionable.\n\nThanks Jeff, allow me to add my comments here on the three patches:\n\n>  [1/3]: t5551: handle trailing slashes in expected cookies output\n\nThis is all benign. As you correctly observed, we no longer keep the \n\"original\" cookie path around and only work with the sanitized version - so \nthat's the one stored now. It was already the one used for actual comparisons \nso apart from the change in storage, it *should* not cause any problems.\n\n>  [2/3]: t5563: add missing end-of-line in HTTP header\n\nI believe I made some code checks a little stricter: header lines MUST end \nwith at CR or LF (or both) to be treated as a valid one. Your fix for this \nshould be good also for older libcurl versions.\n\n>  [3/3]: t5563: relax whitespace assumptions for unfolded headers\n\nThis one is material for me to rethink.\n\nI had to completely change our header unfolding logic because we learned that \nwe did not apply it early enough, so some header parsing was wrongly done on \npre-unfolded data. In this process, I also changed the logic that appends the \nfollowing line on the previous line. To avoid having to keep a state, I \ndecided to just append the second line onto the first one without trying to \nreduce the whitespace characters to a single one.\n\nI did not fully consider the impact this might have on users such as you. \nAllow me to rework that a little bit further and get the former white-space \nbehavior back. Thanks!\n\n-- \n\n  / daniel.haxx.se\n"},{"id":"532467","messageId":"FRWPR03MB110658677899817CC49A50DE7C0A8A@FRWPR03MB11065.eurprd03.prod.outlook.com","threadId":"64648","inReplyTo":"20251218121819.GB3758205@coredump.intra.peff.net","subject":"Re: [PATCH 2/3] t5563: add missing end-of-line in HTTP header","fromName":"Matthew John Cheetham","fromEmail":"mjcheetham@outlook.com","sentAt":"2025-12-18T13:41:54Z","receivedAt":"2025-12-18T13:41:53Z","isPatch":true,"sender":{"key":"mjcheetham@outlook.com","avatar":"https://avatars.githubusercontent.com/u/5658207?v=4"},"body":"On 2025-12-18 12:18, Jeff King wrote:\n\n > In t5563, we test how various oddly-formatted WWW-Authenticate headers\n > are passed through curl to git's credential subsystem (and ultimately\n > out to credential helpers). One test, \"access using basic auth with\n > wwwauth header mixed line-endings\" does something odd. It does not mix\n > line endings at all (which must be CRLF according to the RFC anyway),\n > but omits the line ending entirely for the final header!\n\nAha! Yes, the test should be using *all CRLF line endings*, and is\npoorly named. I believe the intent here is to test mixed *continuation\nline* characters.\n\nE.g, when a continuation line starts with a space, or a tab character,\nfor the same logical header:\n\nWWW-Authenticate: FooBar param1=\"value1\"\\r\\n\n  \\r\\n\n\\tparam2=\"value2\"\\r\\n\n\n\n > This means that the server produces an incomplete response. We send our\n > final header, and then the newline which is meant to mark the end of\n > headers (and the start of the body) becomes the line ending for that\n > header. And there is no header/body separator in the output at all.\n >\n > Looking at strace, this is what the client reads:\n >\n >    recvfrom(9, \"WWW-Authenticate: FooBar param1=\\\"value1\\\"\\r\\n \n\\r\\n\\tparam2=\\\"value2\\\"\\r\\nWWW-Authenticate: Basic \nrealm=\\\"example.com\\\"\", 16384, 0, NULL, NULL) = 106\n >    recvfrom(9, \"\\n\", 16384, 0, NULL, NULL) = 1\n >    recvfrom(9, \"\", 16384, 0, NULL, NULL) = 0\n >\n > The headers themselves are produced from the custom-auth.challenge file\n > we write in the test (which is missing the final CRLF), and then the\n > header/body separator comes from our lib-httpd/nph-custom-auth.sh CGI.\n > (Ignore for a moment that it is producing a bare newline, which I think\n > is a bug; it should be a CRLF but curl is happy with either).\n >\n > Older versions of curl seemed to be OK with the truncated output, but\n > the upcoming 8.18.0 release seems to get confused. Specifically, since\n > 67ae101666 (http: unfold response headers earlier, 2025-12-12) our\n > request to the server fails with insufficient credentials. I traced far\n > enough to see that curl does relay the header back to us, which we then\n > pass to a credential helper, which gives us the correct\n > username/password combination. But on our followup request, curl refuses\n > to send the Authorization header (and so gets an HTTP 401 again).\n >\n > The change in curl's behavior is a bit unexpected, but since we are\n > sending it garbage, it is hard to complain too much. Let's add the\n > missing CRLF to the header. I _think_ this was just an oversight and not\n > the intent of the test. And that the \"mixed line-endings\" really meant\n > \"mixed continuations\", since we differ from the previous test in\n > continuing with both space and tab. So I've likewise updated the test\n > title to match that assumption.\n >\n > Signed-off-by: Jeff King <peff@peff.net>\n > ---\n > I do find it puzzling that we hand curl the credential, but it doesn't\n > get used in the follow-up request. So I may have mis-analyzed something,\n > but I really think that's what is happening. I can share the\n > hacky instrumentation I added if anybody wants to dig further. But since\n > the original was garbage AFAICT, I didn't think it was worth spending\n > a lot of time on it.\n >\n >   t/t5563-simple-http-auth.sh | 4 ++--\n >   1 file changed, 2 insertions(+), 2 deletions(-)\n >\n > diff --git a/t/t5563-simple-http-auth.sh b/t/t5563-simple-http-auth.sh\n > index 317f33af5a..c1febbae9d 100755\n > --- a/t/t5563-simple-http-auth.sh\n > +++ b/t/t5563-simple-http-auth.sh\n > @@ -469,7 +469,7 @@ test_expect_success 'access using basic auth with \nwwwauth header empty continuat\n >   \tEOF\n >   '\n >\n > -test_expect_success 'access using basic auth with wwwauth header \nmixed line-endings' '\n > +test_expect_success 'access using basic auth with wwwauth header \nmixed continuations' '\n\nPerfect! Thanks for fixing my poor naming :)\n\n >   \ttest_when_finished \"per_test_cleanup\" &&\n >\n >   \tset_credential_reply get <<-EOF &&\n > @@ -490,7 +490,7 @@ test_expect_success 'access using basic auth with \nwwwauth header mixed line-endi\n >   \tprintf \"id=default response=WWW-Authenticate: FooBar \nparam1=\\\"value1\\\"\\r\\n\" >>\"$CHALLENGE\" &&\n >   \tprintf \"id=default response= \\r\\n\" >>\"$CHALLENGE\" &&\n >   \tprintf \"id=default response=\\tparam2=\\\"value2\\\"\\r\\n\" >>\"$CHALLENGE\" &&\n > -\tprintf \"id=default response=WWW-Authenticate: Basic \nrealm=\\\"example.com\\\"\" >>\"$CHALLENGE\" &&\n > +\tprintf \"id=default response=WWW-Authenticate: Basic \nrealm=\\\"example.com\\\"\\r\\n\" >>\"$CHALLENGE\" &&\n >\n >   \ttest_config_global credential.helper test-helper &&\n >   \tgit ls-remote \"$HTTPD_URL/custom_auth/repo.git\" &&\n\nThanks,\nMatthew\n\n"},{"id":"532468","messageId":"FRWPR03MB11065628883E63378EF8EB329C0A8A@FRWPR03MB11065.eurprd03.prod.outlook.com","threadId":"64648","inReplyTo":"20251218122204.GC3758205@coredump.intra.peff.net","subject":"Re: [PATCH 3/3] t5563: relax whitespace assumptions for unfolded headers","fromName":"Matthew John Cheetham","fromEmail":"mjcheetham@outlook.com","sentAt":"2025-12-18T13:45:54Z","receivedAt":"2025-12-18T13:45:56Z","isPatch":true,"sender":{"key":"mjcheetham@outlook.com","avatar":"https://avatars.githubusercontent.com/u/5658207?v=4"},"body":"On 2025-12-18 12:22, Jeff King wrote:\n\n > In t5563 we check the handling of WWW-Authenticate headers that have\n > been folded (i.e., where a continuation line starts with extra\n > whitespace). Traditionally curl handed each line to us individually, but\n > in the upcoming v8.18.0, it hands us full lines that have been unfolded.\n > But it doesn't produce exactly the same unfolding that we did!\n >\n > In particular, two of the tests send an extra blank continuation line.\n > Something like this:\n >\n >    printf 'WWW-Authenticate: foo param1=\"value1\"\\r\\n'\n >    printf ' \\r\\n'\n >    printf ' param2=\"value2\"\\r\\n\"\n >\n > We unfold that into:\n >\n >    WWW-Authenticate: foo param1=\"value1\" param2=\"value2\"\n >\n > But curl will give us a string with an extra space:\n >\n >    WWW-Authenticate: foo param1=\"value1\"  param2=\"value2\"\n >\n > I think curl is actually correct here. RFC 7230 says:\n >\n >     A user agent that receives an obs-fold in a response message that is\n >     not within a message/http container MUST replace each received\n >     obs-fold with one or more SP octets prior to interpreting the field\n >     value.\n >\n > So each folded instance turns the initial whitespace into \"one or more\"\n > spaces, and the \"blank\" line becomes a single space. Whereas Git's\n > unfolding code explicitly avoids this, with the comment \"Do not bother\n > appending the new value if this continuation header is itself empty.\" in\n > fwrite_wwwauth().\n >\n > I think it's mostly academic at this point. These folded continuations\n > have been deprecated entirely since RFC 7230 came out in 2014, and\n > there's very little reason for a server to add a blank continuation line\n > at all. And anybody parsing the unfolded header contents should skip\n > past the extra whitespace (which is allowed to be present according to\n > the RFC).\n >\n > But our tests do a byte-wise comparison, so they care about the\n > difference between the two outputs. We have two options here:\n >\n >    1. We can modify Git's unfolding code to behave like modern curl.\n >\n >    2. We can relax the tests to be happy with either output.\n >\n > I picked (2) here, just because it seemed less risky to touch only the\n > tests and not the code (though if any real-world systems _do_ care about\n > the distinction, they will eventually run into problems when libcurl is\n > upgraded).\n\nI think that is a fair choice.\n\n > There is one further curiosity here. There's a second test which mixes\n > tabs and spaces for continuation, like this:\n >\n >    printf 'WWW-Authenticate: foo param1=\"value1\"\\r\\n'\n >    printf '\\t\\r\\n'\n >    printf ' param2=\"value2\"\\r\\n\"\n >\n >  From the snippet of RFC quoted above, I believe this should produce the\n > exact same output (the continuation whitespace is replaced with one or\n > more spaces, even though it is a tab here). But curl retains the tab\n > instead!\n\nHeader continuations are just a nightmare :-(\n\n > So to implement the \"relaxed whitespace\" mode in the test, we just\n > convert any run of multiple whitespace characters to a single space.\n > This is a bit hacky and over-zealous, but it's easy to do and good\n > enough for our purposes here. We only enable the relaxed mode for the\n > two tests which trigger this issue.\n >\n > Signed-off-by: Jeff King <peff@peff.net>\n > ---\n > Note that when built against this new version of curl, Git's unfolding\n > code should never trigger at all. In the long run we should be able to\n > rip it out, but we probably need to wait a decade or so before we can\n > bump the minimum libcurl version to 8.18.0.\n >\n > I guess we could make it a conditional in the code (which would help us\n > remember to eventually rip it out), but it felt weird to start adding\n > version conditionals for a version that isn't even released yet. ;)\n >\n >   t/t5563-simple-http-auth.sh | 11 +++++++++--\n >   1 file changed, 9 insertions(+), 2 deletions(-)\n >\n > diff --git a/t/t5563-simple-http-auth.sh b/t/t5563-simple-http-auth.sh\n > index c1febbae9d..0967cd501c 100755\n > --- a/t/t5563-simple-http-auth.sh\n > +++ b/t/t5563-simple-http-auth.sh\n > @@ -47,6 +47,13 @@ set_credential_reply () {\n >   expect_credential_query () {\n >   \tlocal suffix=\"$(test -n \"$2\" && echo \"-$2\")\"\n >   \tcat >\"$TRASH_DIRECTORY/$1-expect$suffix.cred\" &&\n > +\tif $(test \"$3\" = \"--relax-whitespace\")\n > +\tthen\n > +\t\tHT='\t' &&\n > +\t\tsed \"s/[ $HT][ $HT]*/ /g\" \\\n > +\t\t\t<\"$TRASH_DIRECTORY/$1-query$suffix.cred\" >tmp &&\n > +\t\tmv tmp \"$TRASH_DIRECTORY/$1-query$suffix.cred\"\n > +\tfi &&\n >   \ttest_cmp \"$TRASH_DIRECTORY/$1-expect$suffix.cred\" \\\n >   \t\t \"$TRASH_DIRECTORY/$1-query$suffix.cred\"\n >   }\n > @@ -451,7 +458,7 @@ test_expect_success 'access using basic auth with \nwwwauth header empty continuat\n >   \ttest_config_global credential.helper test-helper &&\n >   \tgit ls-remote \"$HTTPD_URL/custom_auth/repo.git\" &&\n >\n > -\texpect_credential_query get <<-EOF &&\n > +\texpect_credential_query get \"\" --relax-whitespace <<-EOF &&\n >   \tcapability[]=authtype\n >   \tcapability[]=state\n >   \tprotocol=http\n > @@ -495,7 +502,7 @@ test_expect_success 'access using basic auth with \nwwwauth header mixed continuat\n >   \ttest_config_global credential.helper test-helper &&\n >   \tgit ls-remote \"$HTTPD_URL/custom_auth/repo.git\" &&\n >\n > -\texpect_credential_query get <<-EOF &&\n > +\texpect_credential_query get \"\" --relax-whitespace <<-EOF &&\n >   \tcapability[]=authtype\n >   \tcapability[]=state\n >   \tprotocol=http\n"},{"id":"532477","messageId":"sn7p46s1-4o20-q05n-173r-s6716s8145q6@unkk.fr","threadId":"64648","inReplyTo":"613s97no-7021-pp15-79s4-302o39p7n5r8@unkk.fr","subject":"Re: [PATCH 0/3] test-suite fixes for upcoming curl 8.18.0","fromName":"Daniel Stenberg","fromEmail":"daniel@haxx.se","sentAt":"2025-12-18T16:49:27Z","receivedAt":"2025-12-18T16:49:29Z","isPatch":true,"sender":{"key":"daniel@haxx.se","avatar":"https://gravatar.com/avatar/69fdca87edd17cee21ca2e79fc2ff671d644603c3dc27167430f3cd3dbab7ba8?d=mp&s=160"},"body":"On Thu, 18 Dec 2025, Daniel Stenberg wrote:\n\n>>  [3/3]: t5563: relax whitespace assumptions for unfolded headers\n\n> I did not fully consider the impact this might have on users such as you. \n> Allow me to rework that a little bit further and get the former white-space \n> behavior back. Thanks!\n\nI just merged a fix [1] into curl that should restore the unfolding behavior \nto match previous releases. It would be awesome if you could verify.\n\n[1] = https://github.com/curl/curl/commit/9941e7c95bf26f00fd87888a\n\n-- \n\n  / daniel.haxx.se\n"},{"id":"532521","messageId":"20251219073232.GA3784564@coredump.intra.peff.net","threadId":"64648","inReplyTo":"FRWPR03MB110658677899817CC49A50DE7C0A8A@FRWPR03MB11065.eurprd03.prod.outlook.com","subject":"Re: [PATCH 2/3] t5563: add missing end-of-line in HTTP header","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2025-12-19T07:32:32Z","receivedAt":"2025-12-19T07:32:37Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Dec 18, 2025 at 01:41:54PM +0000, Matthew John Cheetham wrote:\n\n> On 2025-12-18 12:18, Jeff King wrote:\n> \n> > In t5563, we test how various oddly-formatted WWW-Authenticate headers\n> > are passed through curl to git's credential subsystem (and ultimately\n> > out to credential helpers). One test, \"access using basic auth with\n> > wwwauth header mixed line-endings\" does something odd. It does not mix\n> > line endings at all (which must be CRLF according to the RFC anyway),\n> > but omits the line ending entirely for the final header!\n> \n> Aha! Yes, the test should be using *all CRLF line endings*, and is\n> poorly named. I believe the intent here is to test mixed *continuation\n> line* characters.\n> \n> E.g, when a continuation line starts with a space, or a tab character,\n> for the same logical header:\n> \n> WWW-Authenticate: FooBar param1=\"value1\"\\r\\n\n>  \\r\\n\n> \\tparam2=\"value2\"\\r\\n\n\nAh, great. I'm happy that my guess was right and there was not something\ntrickier going on (which would have made coming up with a workaround\nmore difficult!). Thanks for confirming.\n\n-Peff\n"},{"id":"532524","messageId":"20251219075002.GB3784564@coredump.intra.peff.net","threadId":"64648","inReplyTo":"613s97no-7021-pp15-79s4-302o39p7n5r8@unkk.fr","subject":"Re: [PATCH 0/3] test-suite fixes for upcoming curl 8.18.0","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2025-12-19T07:50:02Z","receivedAt":"2025-12-19T07:50:03Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Dec 18, 2025 at 01:37:11PM +0100, Daniel Stenberg wrote:\n\n> >  [1/3]: t5551: handle trailing slashes in expected cookies output\n> \n> This is all benign. As you correctly observed, we no longer keep the\n> \"original\" cookie path around and only work with the sanitized version - so\n> that's the one stored now. It was already the one used for actual\n> comparisons so apart from the change in storage, it *should* not cause any\n> problems.\n\nOK, good. I did wonder if there might be some subtle behavior change\nunder the hood, but figured you probably knew what you were doing\n(especially since the normalization was the point of that commit, and\nnot some unexpected side effect).\n\n> >  [2/3]: t5563: add missing end-of-line in HTTP header\n> \n> I believe I made some code checks a little stricter: header lines MUST end\n> with at CR or LF (or both) to be treated as a valid one. Your fix for this\n> should be good also for older libcurl versions.\n\nMakes sense. I think it's accurate to call what our test was doing\ngarbage that we happened to be lucky was accepted, and the new curl\nbehavior will not hurt any real world cases.\n\n> >  [3/3]: t5563: relax whitespace assumptions for unfolded headers\n> \n> This one is material for me to rethink.\n> \n> I had to completely change our header unfolding logic because we learned\n> that we did not apply it early enough, so some header parsing was wrongly\n> done on pre-unfolded data. In this process, I also changed the logic that\n> appends the following line on the previous line. To avoid having to keep a\n> state, I decided to just append the second line onto the first one without\n> trying to reduce the whitespace characters to a single one.\n> \n> I did not fully consider the impact this might have on users such as you.\n> Allow me to rework that a little bit further and get the former white-space\n> behavior back. Thanks!\n\nI do think you're following the standards in including the extra space,\nso that part isn't wrong per se. But it may be kinder to do a bit of\nwhitespace collapsing. I dunno.\n\nThe more fundamental change is that a CURLOPT_HEADERFUNCTION callback is\nnow fed unfolded headers, rather than getting the lines piecemeal (and\nhaving to do the unfolding itself).\n\nSo I'm not sure that we should be worried about a case where old code\npreferred the unfolded but whitespace-collapsed headers, and will be\nbroken if curl does not keep doing that. There was no such code, because\ncurl was not unfolding at all!\n\nThe only code which would confused is a callback that did its own\nunfolding and somehow implemented it differently than curl does (which\nis what happened here). But every caller should be prepared to take\nunfolded data, since after all the server could have avoided folding in\nthe first place.\n\nSo I dunno. While we did see \"breakage\" here, I am inclined to think it\nwas mostly about how intimate and brittle the tests were, and that any\nreal world use would not run into this.\n\nI'd like to think it probably doesn't matter much in the real world\nconsidering the deprecated status of folding in the first place, but\nthat might be too optimistic. ;)\n\n-Peff\n"},{"id":"532525","messageId":"20251219080409.GC3784564@coredump.intra.peff.net","threadId":"64648","inReplyTo":"sn7p46s1-4o20-q05n-173r-s6716s8145q6@unkk.fr","subject":"Re: [PATCH 0/3] test-suite fixes for upcoming curl 8.18.0","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2025-12-19T08:04:09Z","receivedAt":"2025-12-19T08:04:11Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Dec 18, 2025 at 05:49:27PM +0100, Daniel Stenberg wrote:\n\n> On Thu, 18 Dec 2025, Daniel Stenberg wrote:\n> \n> > >  [3/3]: t5563: relax whitespace assumptions for unfolded headers\n> \n> > I did not fully consider the impact this might have on users such as\n> > you. Allow me to rework that a little bit further and get the former\n> > white-space behavior back. Thanks!\n> \n> I just merged a fix [1] into curl that should restore the unfolding behavior\n> to match previous releases. It would be awesome if you could verify.\n> \n> [1] = https://github.com/curl/curl/commit/9941e7c95bf26f00fd87888a\n\nThanks, I took a look at it. Unfortunately I think it only gets us\nhalfway there. It drops the extra space when folding this:\n\n  printf 'Foo: bar\\r\\n'\n  printf ' \\r\\n'\n  printf ' baz\\r\\n'\n\nwhich will yield:\n\n  Foo: bar baz\n\nand it fixes the first of Git's failing tests. But if we swap out the space for a tab\nlike this:\n\n  printf 'Foo: bar\\r\\n'\n  printf ' \\r\\n'\n  printf '\\tbaz\\r\\n'\n\nthen we get collapsed whitespace, but it's a tab. I.e.:\n\n  Foo: bar\\tbaz\n\n(where \"\\t\" is a literal tab). I think that does violate the standard\n(which says it should become spaces). I think in most headers the\ngrammar allows OWS/RWS fields that are spaces or tabs, so in theory it\nshouldn't matter. But I wouldn't be surprised if that causes some\nsurprises in the real world.\n\nSadly the input buffer to http_parse_headers() is const, so we can't\njust write a space over the original tab. ;) But I think rather than\nwalking back to preserve that final leading whitespace byte, we could\njust always add in our own space separately, like this:\n\ndiff --git a/lib/http.c b/lib/http.c\nindex ea62219542..eaa8bf73c2 100644\n--- a/lib/http.c\n+++ b/lib/http.c\n@@ -4388,6 +4388,7 @@ static CURLcode http_parse_headers(struct Curl_easy *data,\n     {\n       /* preserve the whole original header piece size */\n       size_t header_piece = consumed;\n+      bool did_unfold = false;\n \n       if(data->state.leading_unfold) {\n         /* immediately after an unfold, keep only a single whitespace */\n@@ -4398,17 +4399,18 @@ static CURLcode http_parse_headers(struct Curl_easy *data,\n           blen--;\n         }\n         if(consumed) {\n-          if(iblen > blen) {\n-            /* take one step back */\n-            consumed++;\n-            buf--;\n-            blen++;\n-          }\n           data->state.leading_unfold = FALSE; /* done now */\n+          did_unfold = TRUE;\n         }\n       }\n \n       if(consumed) {\n+        if (did_unfold) {\n+          result = curlx_dyn_addn(&data->state.headerb, \" \", 1);\n+          if(result)\n+            return result;\n+        }\n+\n         result = curlx_dyn_addn(&data->state.headerb, buf, consumed);\n         if(result)\n           return result;\n\n-Peff\n"},{"id":"532528","messageId":"0s72r344-865q-2n3q-o9q9-p701087s0n04@unkk.fr","threadId":"64648","inReplyTo":"20251219080409.GC3784564@coredump.intra.peff.net","subject":"Re: [PATCH 0/3] test-suite fixes for upcoming curl 8.18.0","fromName":"Daniel Stenberg","fromEmail":"daniel@haxx.se","sentAt":"2025-12-19T08:47:37Z","receivedAt":"2025-12-19T08:47:39Z","isPatch":true,"sender":{"key":"daniel@haxx.se","avatar":"https://gravatar.com/avatar/69fdca87edd17cee21ca2e79fc2ff671d644603c3dc27167430f3cd3dbab7ba8?d=mp&s=160"},"body":"On Fri, 19 Dec 2025, Jeff King wrote:\n\n>> [1] = https://github.com/curl/curl/commit/9941e7c95bf26f00fd87888a\n>\n> and it fixes the first of Git's failing tests. But if we swap out the space \n> for a tab like this:\n\nSorry, that was just sloppy of me to not add a test and proper handling for \nthat condition. Allow me to fix that in my end. A leading tab in the folding \npart should be replaced by a space.\n\n-- \n\n  / daniel.haxx.se\n"},{"id":"532570","messageId":"20251219232357.GA3960837@coredump.intra.peff.net","threadId":"64648","inReplyTo":"0s72r344-865q-2n3q-o9q9-p701087s0n04@unkk.fr","subject":"Re: [PATCH 0/3] test-suite fixes for upcoming curl 8.18.0","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2025-12-19T23:23:57Z","receivedAt":"2025-12-19T23:24:04Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Dec 19, 2025 at 09:47:37AM +0100, Daniel Stenberg wrote:\n\n> On Fri, 19 Dec 2025, Jeff King wrote:\n> \n> > > [1] = https://github.com/curl/curl/commit/9941e7c95bf26f00fd87888a\n> > \n> > and it fixes the first of Git's failing tests. But if we swap out the\n> > space for a tab like this:\n> \n> Sorry, that was just sloppy of me to not add a test and proper handling for\n> that condition. Allow me to fix that in my end. A leading tab in the folding\n> part should be replaced by a space.\n\nThanks! I ran Git's test suite against a build using your 6c7bc9871f\n(http: fix for unfolding line starting with TAB, 2025-12-19) and it\nworks without the whitespace-relaxing in my third patch.\n\nI also double-checked against the current tip of curl's master, which\nincludes 3388afd2b6 (http: more unfold fixing, 2025-12-19), and\neverything remains fine. Thanks for a prompt fix.\n\n\nJunio: I think we could just drop the third patch here, if we don't mind\ntest failures against an unreleased version of curl. It's in debian\nunstable now, but presumably they'll move to the released 8.18.0 once\nit's out.\n\n-Peff\n"},{"id":"532572","messageId":"xmqqbjjtvr0h.fsf@gitster.g","threadId":"64648","inReplyTo":"20251219232357.GA3960837@coredump.intra.peff.net","subject":"Re: [PATCH 0/3] test-suite fixes for upcoming curl 8.18.0","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-12-20T02:14:22Z","receivedAt":"2025-12-20T02:14:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> Thanks! I ran Git's test suite against a build using your 6c7bc9871f\n> (http: fix for unfolding line starting with TAB, 2025-12-19) and it\n> works without the whitespace-relaxing in my third patch.\n>\n> I also double-checked against the current tip of curl's master, which\n> includes 3388afd2b6 (http: more unfold fixing, 2025-12-19), and\n> everything remains fine. Thanks for a prompt fix.\n>\n>\n> Junio: I think we could just drop the third patch here, if we don't mind\n> test failures against an unreleased version of curl. It's in debian\n> unstable now, but presumably they'll move to the released 8.18.0 once\n> it's out.\n\nI was wondering about the same thing after Daniel started working on\nthe fix upstream for #3; you caught the behaviour change before it\nmade to their officially released version, so we do not have to have\na workaround on our end.\n\nWill drop the third patch.  Thanks.\n"}]}