From: Jeff King <peff@peff•net>
To: git@vger•kernel.org
Cc: Matthew John Cheetham <mjcheetham@outlook•com>,
Daniel Stenberg <daniel@haxx•se>
Subject: [PATCH 3/3] t5563: relax whitespace assumptions for unfolded headers
Date: Thu, 18 Dec 2025 07:22:04 -0500 [thread overview]
Message-ID: <20251218122204.GC3758205@coredump.intra.peff.net> (raw)
In-Reply-To: <20251218121120.GA3252258@coredump.intra.peff.net>
In t5563 we check the handling of WWW-Authenticate headers that have
been folded (i.e., where a continuation line starts with extra
whitespace). Traditionally curl handed each line to us individually, but
in the upcoming v8.18.0, it hands us full lines that have been unfolded.
But it doesn't produce exactly the same unfolding that we did!
In particular, two of the tests send an extra blank continuation line.
Something like this:
printf 'WWW-Authenticate: foo param1="value1"\r\n'
printf ' \r\n'
printf ' param2="value2"\r\n"
We unfold that into:
WWW-Authenticate: foo param1="value1" param2="value2"
But curl will give us a string with an extra space:
WWW-Authenticate: foo param1="value1" param2="value2"
I think curl is actually correct here. RFC 7230 says:
A user agent that receives an obs-fold in a response message that is
not within a message/http container MUST replace each received
obs-fold with one or more SP octets prior to interpreting the field
value.
So each folded instance turns the initial whitespace into "one or more"
spaces, and the "blank" line becomes a single space. Whereas Git's
unfolding code explicitly avoids this, with the comment "Do not bother
appending the new value if this continuation header is itself empty." in
fwrite_wwwauth().
I think it's mostly academic at this point. These folded continuations
have been deprecated entirely since RFC 7230 came out in 2014, and
there's very little reason for a server to add a blank continuation line
at all. And anybody parsing the unfolded header contents should skip
past the extra whitespace (which is allowed to be present according to
the RFC).
But our tests do a byte-wise comparison, so they care about the
difference between the two outputs. We have two options here:
1. We can modify Git's unfolding code to behave like modern curl.
2. We can relax the tests to be happy with either output.
I picked (2) here, just because it seemed less risky to touch only the
tests and not the code (though if any real-world systems _do_ care about
the distinction, they will eventually run into problems when libcurl is
upgraded).
There is one further curiosity here. There's a second test which mixes
tabs and spaces for continuation, like this:
printf 'WWW-Authenticate: foo param1="value1"\r\n'
printf '\t\r\n'
printf ' param2="value2"\r\n"
From the snippet of RFC quoted above, I believe this should produce the
exact same output (the continuation whitespace is replaced with one or
more spaces, even though it is a tab here). But curl retains the tab
instead!
So to implement the "relaxed whitespace" mode in the test, we just
convert any run of multiple whitespace characters to a single space.
This is a bit hacky and over-zealous, but it's easy to do and good
enough for our purposes here. We only enable the relaxed mode for the
two tests which trigger this issue.
Signed-off-by: Jeff King <peff@peff•net>
---
Note that when built against this new version of curl, Git's unfolding
code should never trigger at all. In the long run we should be able to
rip it out, but we probably need to wait a decade or so before we can
bump the minimum libcurl version to 8.18.0.
I guess we could make it a conditional in the code (which would help us
remember to eventually rip it out), but it felt weird to start adding
version conditionals for a version that isn't even released yet. ;)
t/t5563-simple-http-auth.sh | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/t/t5563-simple-http-auth.sh b/t/t5563-simple-http-auth.sh
index c1febbae9d..0967cd501c 100755
--- a/t/t5563-simple-http-auth.sh
+++ b/t/t5563-simple-http-auth.sh
@@ -47,6 +47,13 @@ set_credential_reply () {
expect_credential_query () {
local suffix="$(test -n "$2" && echo "-$2")"
cat >"$TRASH_DIRECTORY/$1-expect$suffix.cred" &&
+ if $(test "$3" = "--relax-whitespace")
+ then
+ HT=' ' &&
+ sed "s/[ $HT][ $HT]*/ /g" \
+ <"$TRASH_DIRECTORY/$1-query$suffix.cred" >tmp &&
+ mv tmp "$TRASH_DIRECTORY/$1-query$suffix.cred"
+ fi &&
test_cmp "$TRASH_DIRECTORY/$1-expect$suffix.cred" \
"$TRASH_DIRECTORY/$1-query$suffix.cred"
}
@@ -451,7 +458,7 @@ test_expect_success 'access using basic auth with wwwauth header empty continuat
test_config_global credential.helper test-helper &&
git ls-remote "$HTTPD_URL/custom_auth/repo.git" &&
- expect_credential_query get <<-EOF &&
+ expect_credential_query get "" --relax-whitespace <<-EOF &&
capability[]=authtype
capability[]=state
protocol=http
@@ -495,7 +502,7 @@ test_expect_success 'access using basic auth with wwwauth header mixed continuat
test_config_global credential.helper test-helper &&
git ls-remote "$HTTPD_URL/custom_auth/repo.git" &&
- expect_credential_query get <<-EOF &&
+ expect_credential_query get "" --relax-whitespace <<-EOF &&
capability[]=authtype
capability[]=state
protocol=http
--
2.52.0.595.gac9d83db54
next prev parent reply other threads:[~2025-12-18 12:22 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-12-18 12:11 [PATCH 0/3] test-suite fixes for upcoming curl 8.18.0 Jeff King
2025-12-18 12:13 ` [PATCH 1/3] t5551: handle trailing slashes in expected cookies output Jeff King
2025-12-18 12:18 ` [PATCH 2/3] t5563: add missing end-of-line in HTTP header Jeff King
2025-12-18 13:41 ` Matthew John Cheetham
2025-12-19 7:32 ` Jeff King
2025-12-18 12:22 ` Jeff King [this message]
2025-12-18 13:45 ` [PATCH 3/3] t5563: relax whitespace assumptions for unfolded headers Matthew John Cheetham
2025-12-18 12:37 ` [PATCH 0/3] test-suite fixes for upcoming curl 8.18.0 Daniel Stenberg
2025-12-18 16:49 ` Daniel Stenberg
2025-12-19 8:04 ` Jeff King
2025-12-19 8:47 ` Daniel Stenberg
2025-12-19 23:23 ` Jeff King
2025-12-20 2:14 ` Junio C Hamano
2025-12-19 7:50 ` Jeff King
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20251218122204.GC3758205@coredump.intra.peff.net \
--to=peff@peff$(echo .)net \
--cc=daniel@haxx$(echo .)se \
--cc=git@vger$(echo .)kernel.org \
--cc=mjcheetham@outlook$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox