hsts: when a dupe host adds subdomains, use that#21108
Closed
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes HSTS cache parsing so that when duplicate host entries are loaded, a later (or duplicate) entry that enables includeSubDomains cannot be weakened by an earlier entry, and it adds a regression test to validate the behavior.
Changes:
- Update HSTS cache load logic to keep the strictest
includeSubDomainspolicy when merging duplicate hosts. - Normalize hostnames read from the HSTS cache file (strip leading dot marker and any trailing dot).
- Add new test
test1638and register it in the test list.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
lib/hsts.c |
Merges duplicate cache entries using max expiry while preserving the strictest subdomain policy; trims trailing dot when loading from file. |
lib/curlx/strparse.c |
Adds curlx_str_trim() helper used for trimming parsed Curl_str values. |
lib/curlx/strparse.h |
Exposes the curlx_str_trim() prototype. |
tests/data/test1638 |
New regression test covering duplicate HSTS entries where the later entry adds subdomains. |
tests/data/Makefile.am |
Adds test1638 to the known test list. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Analysis of PR #21108 at 9f9c1ed2: Test 1638 failed, which has NOT been flaky recently, so there could be a real issue in this PR. Note that this test has failed in 84 different CI jobs (the link just goes to one of them). Generated by Testclutch |
Otherwise a weaker earlier entry is allowed to override a later more restrictive one. Add test 1638 to verify. Closes #21108
bagder
force-pushed
the
bagder/hsts-dupe
branch
from
cf97c59 to
353f8fc
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Otherwise a weaker earlier entry is allowed to override a later more restrictive one.
Add test 1638 to verify.