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

Redo validation errors in the IPv4 parser #739

Merged
merged 2 commits into from
Jan 20, 2023
Merged

Conversation

annevk
Copy link
Member

@annevk annevk commented Jan 17, 2023

ab0e820 didn't consider the impact on the validation error infrastructure.

This also makes some minor additional changes:

  • Modernizes the "Host parsing" section.
  • Clarifies that the IPv4 parser cannot be invoked directly.
  • Clarifies that the IPv6 parser can, but you should still not do it.

Fixes #706.


Preview | Diff

ab0e820 didn't consider the impact on the validation error infrastructure.

This also makes some minor additional changes:
* Modernizes the "Host parsing" section.
* Clarifies that the IPv4 parser cannot be invoked directly.
* Clarifies that the IPv6 parser can, but you should still not do it.

Fixes #706.
@annevk annevk mentioned this pull request Jan 17, 2023
@TRowbotham
Copy link
Contributor

This looks sensible to me.

Copy link
Member

@domenic domenic left a comment

Choose a reason for hiding this comment

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

LGTM. These action-at-a-distance validation errors are going to be kind of a pain in the implementation (there's no object passed to the host/IPv4/IPv6 parsers to thread them back to), but I think that's best left as an implementation problem.

reporting them before we are confident we want to parse <var>input</var> as an IPv4 address as the
<a>host parser</a> almost always invokes the <a>IPv4 parser</a>.
<p class=note>The <a for=/>IPv4 parser</a> is not to be invoked directly. Instead check that the
return value of the <a for=/>host parser</a> is an <a for=/>IPv4 address</a>.
Copy link
Member

Choose a reason for hiding this comment

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

I don't understand why IPv4 and IPv6 would be different...

Copy link
Member Author

Choose a reason for hiding this comment

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

The host parser does percent-decoding and ToASCII before determining if something is an IPv4 address. The only thing it does before IPv6 is removing square brackets. In practice though I think people should always use the host parser and therefore neither the IPv4 nor the IPv6 parser are exported.

@annevk annevk merged commit f0806d6 into main Jan 20, 2023
@annevk annevk deleted the annevk/validationError branch January 20, 2023 11:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Defered validation error reporting in IPv4 parser
3 participants