Skip to content

Conversation

@MihaZupan
Copy link
Member

For an input like http://[::]extra, we would previously hallucinate a slash after the ], so parsing would succeed and AbsolutePath would return /extra.

This change enforces that the ] is followed by a delimiter, same as we do for all other host types.

@MihaZupan MihaZupan added this to the 11.0.0 milestone Oct 20, 2025
@MihaZupan MihaZupan requested a review from a team October 20, 2025 15:34
@MihaZupan MihaZupan self-assigned this Oct 20, 2025
Copilot AI review requested due to automatic review settings October 20, 2025 15:34
Copy link
Contributor

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 URI parsing bug where IPv6 addresses followed by invalid characters (without proper delimiters) would incorrectly succeed and hallucinate path components. For example, http://[::]extra would previously parse successfully with /extra as the path, when it should fail.

Key Changes:

  • Added validation to ensure IPv6 host addresses are followed by valid delimiters (:, /, ?, #, or end of string)
  • Enhanced test coverage with multiple scenarios testing both invalid cases and valid delimiter combinations

Reviewed Changes

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

File Description
src/libraries/System.Private.Uri/src/System/Uri.cs Added delimiter validation after IPv6 address parsing to reject malformed URIs
src/libraries/System.Private.Uri/tests/FunctionalTests/UriIpHostTest.cs Refactored test helper and added comprehensive test cases for delimiter validation

@dotnet-policy-service
Copy link
Contributor

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

Copy link
Member

@rzikm rzikm left a comment

Choose a reason for hiding this comment

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

LGTM

@MihaZupan MihaZupan merged commit 1d1760c into dotnet:main Oct 21, 2025
85 of 87 checks passed
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.

2 participants