Skip to content

Conversation

@MihaZupan
Copy link
Member

@MihaZupan MihaZupan commented Oct 21, 2025

Follow-up to #120908

Adds \ to the list of allowed delimiters.
File-based schemes allow it, and other schemes have other existing logic to reject it.

I'm not sure that anyone is actually using such URIs in practice (UNC has limitations around IPv6), but this lowers the risk of us breaking some odd existing scenario.

For implicit files (file uris that don't start with file:), we also disallow ? or # to match IPv4/host logic.

@MihaZupan MihaZupan added this to the 11.0.0 milestone Oct 21, 2025
@Copilot Copilot AI review requested due to automatic review settings October 21, 2025 14:26
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 enhances IPv6 URI parsing by adding backslash (\) as an allowed delimiter after IPv6 addresses and expands test coverage for various URI schemes with IPv6 hosts.

Key Changes:

  • Adds \ to the list of valid delimiters that can follow an IPv6 address in URIs
  • Expands test coverage to validate IPv6 parsing across different URI schemes (http, file, custom file-based, and custom http-based)
  • Tests both file-based schemes (which allow backslash delimiters) and non-file schemes (which reject them)

Reviewed Changes

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

File Description
src/libraries/System.Private.Uri/src/System/Uri.cs Adds backslash to the valid delimiter pattern after IPv6 addresses
src/libraries/System.Private.Uri/tests/FunctionalTests/UriIpHostTest.cs Expands test coverage with parameterized tests across multiple schemes and validates backslash handling

@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.

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