Skip to content

hsts: cap max-age to two years - #22878

Closed
bagder wants to merge 4 commits into
masterfrom
bagder/hsts-large-maxage
Closed

bagder wants to merge 4 commits into
masterfrom
bagder/hsts-large-maxage

Conversation

@bagder

@bagder bagder commented Sep 8, 2026

Copy link
Copy Markdown
Member

And ignore problems saving individual HSTS entries - a single entry problem should not affect storing other entries.

Added test 1862 to verify.

@bagder bagder changed the title hsts: cap max-time to two years hsts: cap max-age to two years Sep 8, 2026
And ignore problems saving individual HSTS entries - a single entry
problem should not affect storing other entries.

Added test 1862 to verify.
@bagder
bagder force-pushed the bagder/hsts-large-maxage branch from f2a5e04 to 0372696 Compare September 8, 2026 14:07
@github-actions github-actions Bot added the tests label Sep 8, 2026
@bagder
bagder requested a balanced review from Copilot September 8, 2026 14:26
@bagder
bagder marked this pull request as ready for review September 8, 2026 14:26

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The new date-conversion failure path lacks regression coverage.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Caps HSTS max-age at two years and allows file serialization to continue when an entry’s date conversion fails.

Changes:

  • Caps parsed HSTS lifetimes at 730 days.
  • Skips entries with unconvertible expiry dates.
  • Adds an integration test for extreme max-age.
File summaries
File Description
lib/hsts.c Implements lifetime capping and nonfatal serialization.
tests/data/test1862 Tests extreme max-age handling.
tests/data/Makefile.am Registers test 1862.
Review details
  • Files reviewed: 3/3 changed files
  • Comments generated: 2
  • Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread lib/hsts.c
Comment thread tests/data/test1862 Outdated
@testclutch

Copy link
Copy Markdown

Analysis of PR #22878 at b2fed2ba:

Test ../../tests/http/test_21_resolve.py::TestResolve::test_21_07_dnsd_a[DoH] 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).

Test ../../tests/http/test_21_resolve.py::TestResolve::test_21_08_dnsd_aaaa[DoH] 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).

Test ../../tests/http/test_21_resolve.py::TestResolve::test_21_09_dnsd_a_delay[DoH] 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).

Test ../../tests/http/test_21_resolve.py::TestResolve::test_21_10_dnsd_aaaa_delay[DoH] 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).

Test ../../tests/http/test_21_resolve.py::TestResolve::test_21_13_dnsd_nxdomain_cached[DoH] 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).

Test 440 failed, which has NOT been flaky recently, so there could be a real issue in this PR. Note that this test has failed in 12 different CI jobs (the link just goes to one of them).

Generated by Testclutch

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@bagder bagder closed this in 08d50a7 Sep 8, 2026
@bagder
bagder deleted the bagder/hsts-large-maxage branch September 8, 2026 21:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Development

Successfully merging this pull request may close these issues.

3 participants