Skip to content

Fix SetCookieHeaderValue.TryParse throwing ArgumentOutOfRangeException for Max-Age overflow #63323

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Aug 21, 2025

Conversation

Copilot
Copy link
Contributor

@Copilot Copilot AI commented Aug 18, 2025

The SetCookieHeaderValue.TryParse method was throwing ArgumentOutOfRangeException when the Max-Age value exceeded TimeSpan's maximum representable seconds (922337203685), violating the expected behavior of TryParse methods which should return false for invalid input rather than throwing exceptions.

Problem

// This would throw ArgumentOutOfRangeException instead of returning false
bool result = SetCookieHeaderValue.TryParse("name=value; max-age=922337203686", out var cookie);

The issue occurred because TimeSpan.FromSeconds(maxAge) throws when maxAge exceeds the valid range, but the calling code didn't validate the input before the conversion.

Solution

Added overflow validation in GetSetCookieLength before calling TimeSpan.FromSeconds:

// Check if maxAge would cause TimeSpan.FromSeconds to overflow
const long MaxTimeSpanSeconds = 922337203685L;
const long MinTimeSpanSeconds = -922337203685L;
if (maxAge > MaxTimeSpanSeconds || maxAge < MinTimeSpanSeconds)
{
    // MaxAge value would overflow TimeSpan, abort
    return 0;
}

Now overflow cases return false as expected:

// These now correctly return false instead of throwing
bool result1 = SetCookieHeaderValue.TryParse("name=value; max-age=922337203686", out var cookie1);   // false
bool result2 = SetCookieHeaderValue.TryParse("name=value; max-age=-922337203686", out var cookie2);  // false

// Valid edge cases still work
bool result3 = SetCookieHeaderValue.TryParse("name=value; max-age=922337203685", out var cookie3);   // true

Added comprehensive tests covering both overflow cases (which should fail parsing) and valid edge cases (which should succeed).

Fixes #61735.


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Co-authored-by: danmoseley <6385855+danmoseley@users.noreply.github.com>
@Copilot Copilot AI changed the title [WIP] Microsoft.Net.Http.Headers.SetCookieHeaderValue.TryParse throws System.ArgumentOutOfRangeException Fix SetCookieHeaderValue.TryParse throwing ArgumentOutOfRangeException for Max-Age overflow Aug 19, 2025
@Copilot Copilot AI requested a review from danmoseley August 19, 2025 00:06
Copilot finished work on behalf of danmoseley August 19, 2025 00:06
… validation

Co-authored-by: danmoseley <6385855+danmoseley@users.noreply.github.com>
Copilot finished work on behalf of danmoseley August 19, 2025 00:23
@Copilot Copilot AI requested a review from danmoseley August 19, 2025 00:23
@danmoseley danmoseley marked this pull request as ready for review August 19, 2025 00:26
@Copilot Copilot AI review requested due to automatic review settings August 19, 2025 00:26
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes a bug where SetCookieHeaderValue.TryParse would throw ArgumentOutOfRangeException instead of returning false when parsing Max-Age values that exceed TimeSpan's maximum representable seconds (922337203685). The fix ensures proper TryParse behavior by handling overflow conditions gracefully.

Key changes:

  • Added exception handling in GetSetCookieLength to catch ArgumentOutOfRangeException from TimeSpan.FromSeconds
  • Added comprehensive test coverage for both overflow cases and valid edge cases

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/Http/Headers/src/SetCookieHeaderValue.cs Added try-catch block to handle TimeSpan overflow in Max-Age parsing
src/Http/Headers/test/SetCookieHeaderValueTest.cs Added test methods covering Max-Age overflow scenarios and valid edge cases

@danmoseley
Copy link
Member

@BrennanConroy any concerns

@danmoseley
Copy link
Member

Also, is this worth porting into RC2?

@danmoseley
Copy link
Member

@copilot please go back to original approach without try catch

…n overflow handling

Co-authored-by: danmoseley <6385855+danmoseley@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Aug 19, 2025

@copilot please go back to original approach without try catch

Done! Reverted to the manual validation approach without try-catch as requested. The code now uses explicit bounds checking instead of exception handling. See commit 74c8f83.

@Copilot Copilot AI requested a review from danmoseley August 19, 2025 21:13
Copilot finished work on behalf of danmoseley August 19, 2025 21:13
@danmoseley danmoseley enabled auto-merge (squash) August 19, 2025 22:48
Co-authored-by: Günther Foidl <gue@korporal.at>
@halter73
Copy link
Member

halter73 commented Aug 21, 2025

Also, is this worth porting into RC2?

Sure. It's a very safe improvement, but I don't think anything in the framework calls SetCookieHeaderValue.TryParse internally, so it might not be worth it to backport further.

@BrennanConroy
Copy link
Member

/ba-g Test failure is quarantined

@danmoseley danmoseley merged commit d963ce4 into main Aug 21, 2025
27 of 29 checks passed
@danmoseley danmoseley deleted the copilot/fix-61735 branch August 21, 2025 22:07
@dotnet-policy-service dotnet-policy-service bot added this to the 11.0-preview1 milestone Aug 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Microsoft.Net.Http.Headers.SetCookieHeaderValue.TryParse throws System.ArgumentOutOfRangeException
6 participants