Skip to content
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

Set Range header correctly in AddRange #113720

Merged
merged 1 commit into from
Mar 24, 2025
Merged

Conversation

bgrainger
Copy link
Contributor

Fixes #113690.

New tests verify the behavior that was observed under .NET Framework 4.7.2.

@Copilot Copilot bot review requested due to automatic review settings March 20, 2025 03:06
@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Mar 20, 2025

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 the behavior of the Range header in HttpWebRequest by correctly concatenating multiple range values rather than resetting the header. It also introduces new tests to verify correct behavior when adding single and multiple ranges.

  • Fix the Range header concatenation logic in AddRange.
  • Add tests to verify multiple range additions.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
src/libraries/System.Net.Requests/tests/HttpWebRequestTest.cs Adds test cases for Range header handling when multiple ranges are added.
src/libraries/System.Net.Requests/src/System/Net/HttpWebRequest.cs Modifies AddRange method to correctly append new range values.
Comments suppressed due to low confidence (1)

src/libraries/System.Net/HttpWebRequest.cs:1546

  • Changing from 'curRange = string.Empty;' to 'curRange += ",";' correctly preserves the existing header value when adding a second range. Confirm that this change handles all cases of Range header formatting as expected.
curRange += ",";
@ManickaP
Copy link
Member

/ba-g failure unrelated

Copy link
Member

@ManickaP ManickaP left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the fix!

Copy link
Member

@wfurt wfurt left a comment

Choose a reason for hiding this comment

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

LGTM

@ManickaP ManickaP self-assigned this Mar 20, 2025
@ManickaP ManickaP merged commit ed32992 into dotnet:main Mar 24, 2025
81 of 85 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-System.Net community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

HttpWebRequest.AddRange generates incorrect header if called twice
3 participants