cookie: check __Secure- and __Host- case sensitively when read from file#22085
cookie: check __Secure- and __Host- case sensitively when read from file#22085alhudz wants to merge 1 commit into
Conversation
|
Would it be possible to merge this test case into the recently added test 2311? |
|
This is incorrect. See 5af0165 which made the exact opposite change for cookie spec reasons! |
|
I think the fix should rather be to make sure file content is also parsed case sensitively. |
The header path matches these prefixes case sensitively, as 5af0165 made it for cookie spec reasons, but the Netscape cookie-file path still used a case-insensitive match. Align the file path so a differently cased name like __secure-x is treated as an ordinary cookie instead of being put through the prefix integrity checks. Extended test 2311 to cover it.
d408205 to
31f64c2
Compare
|
You're right, the spec wants a case-sensitive match and the header path already does that since 5af0165. I had it backwards. Flipped it round: the file path now uses |
|
Analysis of PR #22085 at 31f64c25: Test ../../tests/http/test_07_upload.py::TestUpload::test_07_31_put_10m[h3] failed, which has NOT been flaky recently, so there could be a real issue in this PR. Note that this test has failed in 2 different CI jobs (the link just goes to one of them). Generated by Testclutch |
|
Thanks! |
Repro: load a cookie file holding
__secure-x(lower case, noSecure) and the name is dropped, while the identical__secure-xreceived over aSet-Cookieheader is kept as an ordinary cookie.test2311covers it.Cause:
parse_netscape()detected the__Secure-/__Host-name prefixes with case-insensitivecurl_strnequal(), but the header path uses case-sensitivestrncmp()and the 6265bis document specifies a case-sensitive match (the reason 5af0165 switched the header path). So on the file path a differently cased name was wrongly put through the prefix integrity checks.Fix: use
strncmp()on the file path too, so both paths match the prefixes case sensitively and agree with the spec. A correctly cased__Secure-/__Host-is still enforced.Earlier I had this the other way round (making the header path case insensitive); that was wrong, the spec wants case-sensitive. Folded the coverage into test 2311 instead of a new file.