Skip to content

Loosen the validation rule for URI authority - #5854

Merged
ikhoon merged 9 commits into
line:mainfrom
ikhoon:allow_underscore_authority
Aug 8, 2024
Merged

Loosen the validation rule for URI authority#5854
ikhoon merged 9 commits into
line:mainfrom
ikhoon:allow_underscore_authority

Conversation

@ikhoon

@ikhoon ikhoon commented Aug 2, 2024

Copy link
Copy Markdown
Contributor

Motivation:

The authority part in a URI was validated by
URI.parseServerAuthority() which only allows alphanumeric characters, . and -.
https://github.com/openjdk/jdk/blob/dc35f3e8a84c8f622a4cabb8aee0f96de2e2ea30/src/java.base/share/classes/java/net/URI.java#L3513-L3515

As a result, if underscore (_) is set in an authority, URISyntaxException is raised. We think the rule is too strict because a request can also be sent to an instance when CSLB is used. _ is a valid character in a DNS record. Users may want to send a request to a host whose name is beta_api.cloud.instance-123.somewhere.com.

Related: #5814

Modifications:

  • Remove the usage of URI.parseServerAuthority() in SchemeAndAuthority
  • Parse a hostname and a port from a raw authority.

Result:

Motivation:

The authority part in a URI was validated by
`URI.parseServerAuthority()` which only allows alphanumeric characters,
`.` and `-`.
https://github.com/openjdk/jdk/blob/dc35f3e8a84c8f622a4cabb8aee0f96de2e2ea30/src/java.base/share/classes/java/net/URI.java#L3513-L3515

As a result, if underscore (`_`) is set in an authority,
`URISyntaxException` is raised. We think the rule is too strict because
a request can also be sent to an instance when CSLB is used.
`_` is a valid character in a DNS record. Users may want to send a host
whose name is `beta_api.cloud.somewhere.com`.

Related: line#5814

Modifications:

- Remove the usage of `URI.parseServerAuthority()` in
  `SchemeAndAuthority`
- Parse a hostname and a port from a raw authority.

Result:

- Validation is relaxed to permit underscores (_) in URI's authority.
- Closes line#5814
@ikhoon ikhoon added the defect label Aug 2, 2024
@ikhoon ikhoon added this to the 1.30.0 milestone Aug 2, 2024
Comment thread core/src/main/java/com/linecorp/armeria/internal/common/SchemeAndAuthority.java Outdated
Comment thread core/src/main/java/com/linecorp/armeria/internal/common/SchemeAndAuthority.java Outdated
Comment thread core/src/main/java/com/linecorp/armeria/internal/common/SchemeAndAuthority.java Outdated
"foo#bar", // Authority with fragment
"[192.168.0.1]", // Bracketed IPv4
"[::1", "::1]", // Incomplete IPv6
"[::1]%eth0", // IPv6 with scope

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think an IPv6 address with scope is represented like this:

  • [::1%eth0]:8080

In this case, we should simply strip the scope and normalize into:

  • [::1]:8080

However, [::1]%eth0 should remain failing.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

TIL 📝

@ikhoon
ikhoon force-pushed the allow_underscore_authority branch from bf7bbf2 to 5ab1d61 Compare August 3, 2024 05:42

@jrhee17 jrhee17 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👍 👍 👍

@minwoox minwoox left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👍 👍 👍

@ikhoon
ikhoon merged commit fde4260 into line:main Aug 8, 2024
@ikhoon
ikhoon deleted the allow_underscore_authority branch August 22, 2024 04:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Loosen the validation rule for Endpoint.host()

4 participants