Re: [PATCH 0/3] test-suite fixes for upcoming curl 8.18.0
- From
Jeff King <peff@peff.net>
- Date
- Dec 19, 2025, 07:50 UTC
- Message-ID
- <20251219075002.GB3784564@coredump.intra.peff.net>
- In-Reply-To
- <613s97no-7021-pp15-79s4-302o39p7n5r8@unkk.fr>
On Thu, Dec 18, 2025 at 01:37:11PM +0100, Daniel Stenberg wrote:
Show 7 quoted lines
> > [1/3]: t5551: handle trailing slashes in expected cookies output > > This is all benign. As you correctly observed, we no longer keep the > "original" cookie path around and only work with the sanitized version - so > that's the one stored now. It was already the one used for actual > comparisons so apart from the change in storage, it *should* not cause any > problems.
OK, good. I did wonder if there might be some subtle behavior change under the hood, but figured you probably knew what you were doing (especially since the normalization was the point of that commit, and not some unexpected side effect).
Show 5 quoted lines
> > [2/3]: t5563: add missing end-of-line in HTTP header > > I believe I made some code checks a little stricter: header lines MUST end > with at CR or LF (or both) to be treated as a valid one. Your fix for this > should be good also for older libcurl versions.
Makes sense. I think it's accurate to call what our test was doing garbage that we happened to be lucky was accepted, and the new curl behavior will not hurt any real world cases.
Show 14 quoted lines
> > [3/3]: t5563: relax whitespace assumptions for unfolded headers > > This one is material for me to rethink. > > I had to completely change our header unfolding logic because we learned > that we did not apply it early enough, so some header parsing was wrongly > done on pre-unfolded data. In this process, I also changed the logic that > appends the following line on the previous line. To avoid having to keep a > state, I decided to just append the second line onto the first one without > trying to reduce the whitespace characters to a single one. > > I did not fully consider the impact this might have on users such as you. > Allow me to rework that a little bit further and get the former white-space > behavior back. Thanks!
I do think you're following the standards in including the extra space, so that part isn't wrong per se. But it may be kinder to do a bit of whitespace collapsing. I dunno.
The more fundamental change is that a CURLOPT_HEADERFUNCTION callback is now fed unfolded headers, rather than getting the lines piecemeal (and having to do the unfolding itself).
So I'm not sure that we should be worried about a case where old code preferred the unfolded but whitespace-collapsed headers, and will be broken if curl does not keep doing that. There was no such code, because curl was not unfolding at all!
The only code which would confused is a callback that did its own unfolding and somehow implemented it differently than curl does (which is what happened here). But every caller should be prepared to take unfolded data, since after all the server could have avoided folding in the first place.
So I dunno. While we did see "breakage" here, I am inclined to think it was mostly about how intimate and brittle the tests were, and that any real world use would not run into this.
I'd like to think it probably doesn't matter much in the real world considering the deprecated status of folding in the first place, but that might be too optimistic. ;)
-Peff