Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Jan 9, 2026

The simple_url parser was accepting unencoded whitespace in URL components, violating RFC 3986 which requires whitespace to be percent-encoded.

Changes

  • Validation: Added whitespace checks for username, password, and path components using char::is_whitespace
  • Error handling: Returns InvalidDomainCharacter for consistency with existing host validation
  • Tests: Added coverage for space, tab, and newline characters in each component

Example

// Now correctly rejected
gix_url::parse("http://user name@example.com/path".into())  // Error
gix_url::parse("http://example.com/ path".into())           // Error

// Percent-encoded whitespace still accepted (correct per RFC 3986)
gix_url::parse("http://user%20name@example.com/path".into()) // Ok

The validation is applied during URL parsing in ParsedUrl::parse(), catching invalid characters before the URL components are stored.

Original prompt

Validate invariants according to the URL specification when gix_url::simple_url parses URLs. For instance, right now it allows whitespace in http:// URLs which isn't correct.


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI changed the title [WIP] Fix URL parsing to validate invariants Validate URL components per RFC 3986: reject unencoded whitespace Jan 9, 2026
Copilot AI requested a review from Byron January 9, 2026 05:33
Copilot AI and others added 2 commits January 9, 2026 09:46
…ssword and path

Co-authored-by: Byron <63622+Byron@users.noreply.github.com>
@Byron Byron force-pushed the copilot/validate-url-invariants branch from e83a4dd to 5c99c68 Compare January 9, 2026 08:54
@Byron Byron marked this pull request as ready for review January 9, 2026 08:56
@Byron
Copy link
Member

Byron commented Jan 9, 2026

Fine, but I realise that percent-encoding isn't handled consistently. Everything needs to be percent-decoded. Will be done in a follow-up.

@Byron Byron enabled auto-merge January 9, 2026 08:56
@Byron Byron merged commit 9081b8f into main Jan 9, 2026
30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants