HostPort needs updates for spec compliance #7269

joakime · 2021-12-13T20:35:49Z

Jetty version(s)
9.4+

Java version/vendor (use: java -version)
All

OS type/version
Alll

Description

Currently HostPort allows odd authorities that make no sense.

Host: :9999 
Host: -
Host: -:8888
Host: *
Host: *:2222
Host: *.eclipse.org

I think HostPort should validate the host portion a bit more, to reject these nonsense hosts with a 400 Bad Request.

The spec for HTTP at https://datatracker.ietf.org/doc/html/rfc7230#section-5.4
Says the uri-host for http is detailed in https://datatracker.ietf.org/doc/html/rfc7230#section-2.7.1
Which points to host in https://datatracker.ietf.org/doc/html/rfc3986#section-3.2.2

which has the following ABNF ...

  uri-host    = <host, see [RFC3986], Section 3.2.2>
  host        = IP-literal / IPv4address / reg-name
  IP-literal  = "[" ( IPv6address / IPvFuture  ) "]"
  IPv6address =                            6( h16 ":" ) ls32
                  /                       "::" 5( h16 ":" ) ls32
                  / [               h16 ] "::" 4( h16 ":" ) ls32
                  / [ *1( h16 ":" ) h16 ] "::" 3( h16 ":" ) ls32
                  / [ *2( h16 ":" ) h16 ] "::" 2( h16 ":" ) ls32
                  / [ *3( h16 ":" ) h16 ] "::"    h16 ":"   ls32
                  / [ *4( h16 ":" ) h16 ] "::"              ls32
                  / [ *5( h16 ":" ) h16 ] "::"              h16
                  / [ *6( h16 ":" ) h16 ] "::"

  ls32        = ( h16 ":" h16 ) / IPv4address
                  ; least-significant 32 bits of address

  h16         = 1*4HEXDIG
                  ; 16 bits of address represented in hexadecimal
  IPvFuture   = "v" 1*HEXDIG "." 1*( unreserved / sub-delims / ":" )
  IPv4address = dec-octet "." dec-octet "." dec-octet "." dec-octet
  dec-octet   = DIGIT                 ; 0-9
                  / %x31-39 DIGIT         ; 10-99
                  / "1" 2DIGIT            ; 100-199
                  / "2" %x30-34 DIGIT     ; 200-249
                  / "25" %x30-35          ; 250-255
  reg-name    = *( unreserved / pct-encoded / sub-delims )
  unreserved  = ALPHA / DIGIT / "-" / "." / "_" / "~"
  pct-encoded = "%" HEXDIG HEXDIG
  sub-delims  = "!" / "$" / "&" / "'" / "(" / ")"
                  / "*" / "+" / "," / ";" / "="

Optionally, this validation could exist as a RejectInvalidAuthorityCustomizer (like proposed in PR #7251).

The text was updated successfully, but these errors were encountered:

joakime · 2021-12-14T01:54:16Z

RFC3986 has been updated for IPv6 with zone literals in
https://datatracker.ietf.org/doc/html/rfc6874

And support for IPv4 within IPv6 is documented in the obsolete
https://tools.ietf.org/html/rfc2732#section-2
I cannot find IPv4 within IPv6 listed as a supported feature in up to date RFCs about http or URI specs.

joakime · 2021-12-14T02:11:16Z

Per reg-name the characters not allowed are ...

 / \ : @ ^ [ ] { } < > # | " `

joakime · 2021-12-14T02:12:49Z

How far do we go?

Are these considered valid?

*.eclipse.org:888
[this:is:odd]:888
how:about:this:too

+ Updates to HttpURI in regards to handling of no-port to conform to java.net.URI behaviors of no-port as well Signed-off-by: Joakim Erdfelt <[email protected]>

joakime · 2021-12-14T14:12:55Z

Opened PR #7279 to conform to specs, and perform Host validation that just ensures it follows reg-name (simply by rejecting host names that violate the generous reg-name scope of characters)

Signed-off-by: Joakim Erdfelt <[email protected]>

joakime · 2022-07-14T13:16:57Z

Along with the general cleanup, we should standardize our use of "no port".

Since we go into and out of the java.net.URI class often, we should use their definition of "no port" and have it be a constant of value -1 in HostPort like HostPort.NO_PORT.

A value of 0 (zero) is something I think we should avoid, as it's not a port we can connect to.

Here's a demo of the behavior of java.net.URI ...

package uri;

import java.net.URI;
import java.net.URISyntaxException;

public class UriBadPortTest
{
    public static void main(String[] args)
    {
        dumpUri("http://host:0/path");
        dumpUri("http://host:-1/path");
        dumpUri("http://host:-2/path");
        dumpUri("http://host:/path");
        dumpUri("http://host: 777 /path");
    }

    private static void dumpUri(String raw)
    {
        try
        {
            URI uri = new URI(raw);
            System.out.printf("Raw \"%s\" -> host:\"%s\" port:%d%n", raw, uri.getHost(), uri.getPort());
        }
        catch (URISyntaxException e)
        {
            System.out.printf("Invalid URI \"%s\": (%s) %s%n",
                raw, e.getClass().getName(), e.getMessage()
            );
        }
    }
}

Results in the output ...

Raw "http://host:0/path" -> host:"host" port:0
Raw "http://host:-1/path" -> host:"null" port:-1
Raw "http://host:-2/path" -> host:"null" port:-1
Raw "http://host:/path" -> host:"host" port:-1
Invalid URI "http://host: 777 /path": (java.net.URISyntaxException) Illegal character in authority at index 7: http://host: 777 /path

joakime · 2022-07-14T13:22:37Z

Some other things that HostPort doesn't support very well ...

(Presented as tests of valid authorities in the HostPortTest)

IDN values

          // IDN example
          Arguments.of("пример.рф", "пример.рф", null),
          Arguments.of("пример.рф:8888", "пример.рф", 8888)

IPv6 with zone identifiers

          // Examples of IPv6 with zone identifier from https://datatracker.ietf.org/doc/html/rfc6874
          Arguments.of("[fe80::a%en1]", "[fe80::a%en1]", null),
          Arguments.of("[fe80::a%25ee1]", "[fe80::a%25ee1]", null),
          Arguments.of("[fe80::a%en1]:7777", "[fe80::a%en1]", 7777),
          Arguments.of("[fe80::a%25ee1]:7777", "[fe80::a%25ee1]", 7777),

Scheme based normalization

          // scheme based normalization https://datatracker.ietf.org/doc/html/rfc3986#section-6.2.3
          Arguments.of("host:", "host", null),
          Arguments.of("127.0.0.1:", "127.0.0.1", null),

Whitespace in weird places

          // Whitespace
          Arguments.of("host   ", "host", null),
          Arguments.of("    host   ", "host", null),
          Arguments.of("host   :777", "host", 777),
          Arguments.of("    host   :777", "host", 777),
          Arguments.of("    host   :   777", "host", 777),
          Arguments.of("    host   :777   ", "host", 777),
          Arguments.of("    host   :   777   ", "host", 777),

joakime · 2022-07-14T13:25:51Z

I also think that all of the following should be reported as Invalid Authorities.

    private static Stream<Arguments> invalidAuthorityProvider()
    {
        return Stream.of(
                // Empty / Null / Blank authority
                null,
                "", // TODO: if addressing edge case with absolute-uri and empty Host header (Issue #7278)
                "    ", // TODO: if addressing edge case with absolute-uri and empty Host header (Issue #7278)
                // Invalid Ports
                "-:-",
                "host:xxx",
                "127.0.0.1:xxx",
                "[0::0::0::0::1]:xxx",
                "host:-80",
                "127.0.0.1:-80", // negative port
                "[0::0::0::0::1]:-80", // negative port
                "127.0.0.1:65536", // port too big
                "jetty.eclipse.org:88007386429567428956488", // port too big
                "jetty.eclipse.org:22,333", // port with commas
                // Empty / Null / Blank Hosts
                ":",
                ":44",
                "::",
                // Bad quoting
                "'eclipse.org:443'",
                "eclipse.org:443\"", // bad end quoting that made it through
                "':88'",
                // Bad Host Names (invalid IP-Literals)
                "[jetty.eclipse.org]:80", // invalid/mimic ipv6 with port
                "[sheep:cheese:of:rome]:80", // invalid/mimic ipv6 with port
                "[pecorino:romano]", // invalid/mimic ipv6 without port
                // Bad Host Names (invalid reg-name) - note: an invalid IPv4address looks like a reg-name
                "this:that:or:the:other.com:222",  // multiple ':' with port
                "and:also:th.is",  // multiple ':' without port
                // reg-name identified invalid printable characters - / \ : @ ^ [ ] { } < > # | " `
                "a/slash.com",
                "a\\backslash.edu",
                "[email protected]",
                "a^caret.net",
                "some[arbitrary]brackets.org",
                "more{curly}braces.io",
                "html<elements>here.au",
                "hash#octothor.pe",
                "ceci-n'est-pas-une|pipe.fr",
                "we-sell-\"quotes\".com",
                "back`ticks`bbq.au",
                // reg-name invalid control characters
                "how\ttabulous.net",
                "null\u0000.com",
                "bell-\u0007-tolls.edu",
                "del-\u007F-mar.au"
            )
            .map(Arguments::of);
    }

    @ParameterizedTest
    @MethodSource("invalidAuthorityProvider")
    public void testInvalidAuthority(String authority)
    {
        assertThrows(IllegalArgumentException.class, () ->
        {
            new HostPort(authority);
        });
    }

github-actions · 2023-07-15T00:00:13Z

This issue has been automatically marked as stale because it has been a
full year without activity. It will be closed if no further activity occurs.
Thank you for your contributions.

github-actions · 2024-07-15T00:00:18Z

This issue has been automatically marked as stale because it has been a
full year without activity. It will be closed if no further activity occurs.
Thank you for your contributions.

gregw · 2024-07-15T00:40:03Z

@joakime Ofthe examples listed above, almost all of them result in an IAE from HostPort (both the ones you say are valid and invalid).
The only exceptions are explicit handling for an empty string like "" and a very trunctated IPv6 address like "::". Note you say that that "" should both be valid and invalid?

I'm not sure if there is anything really to do here? Can you review this please, else we can close this

joakime added the Bug For general bugs on Jetty side label Dec 13, 2021

joakime mentioned this issue Dec 14, 2021

Properly support HTTP/1.1 with non-http scheme absolute-uri and empty Host header #7278

Open

joakime added a commit that referenced this issue Dec 14, 2021

Issue #7269 - Refinement of Host behavior based on Specs

148ed76

+ Updates to HttpURI in regards to handling of no-port to conform to java.net.URI behaviors of no-port as well Signed-off-by: Joakim Erdfelt <[email protected]>

joakime added a commit that referenced this issue Dec 14, 2021

Issue #7269 - Refinement of Host behavior based on Specs

4a733ba

+ Updates to HttpURI in regards to handling of no-port to conform to java.net.URI behaviors of no-port as well Signed-off-by: Joakim Erdfelt <[email protected]>

joakime linked a pull request Dec 14, 2021 that will close this issue

Issue #7269 - Refinement of HostPort behavior based on Specs #7279

Closed

joakime changed the title ~~HostPort should do some basic validation of Host~~ HostPort needs updates for spec compliance Dec 14, 2021

joakime added a commit that referenced this issue Dec 14, 2021

Issue #7269 - Fixing Host with empty port tests

f6829e4

Signed-off-by: Joakim Erdfelt <[email protected]>

joakime added a commit that referenced this issue Dec 14, 2021

Issue #7269 - Expand HostPortTest scenarios

594d956

Signed-off-by: Joakim Erdfelt <[email protected]>

joakime added a commit that referenced this issue Dec 14, 2021

Issue #7269 - LocalConnector bad assumptions

19f4e74

Signed-off-by: Joakim Erdfelt <[email protected]>

joakime added a commit that referenced this issue Dec 15, 2021

Issue #7269 - Updates per review

e55d6dc

Signed-off-by: Joakim Erdfelt <[email protected]>

joakime mentioned this issue Jul 14, 2022

Issue #7269 - Refinement of HostPort behavior based on Specs #7279

Closed

github-actions bot added the Stale For auto-closed stale issues and pull requests label Jul 15, 2023

sbordet removed the Stale For auto-closed stale issues and pull requests label Jul 15, 2023

cowwoc mentioned this issue Mar 4, 2024

Inconsistent default port number in HttpURI and HostPort #11488

Closed

github-actions bot added the Stale For auto-closed stale issues and pull requests label Jul 15, 2024

github-actions bot removed the Stale For auto-closed stale issues and pull requests label Jul 16, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

HostPort needs updates for spec compliance #7269

HostPort needs updates for spec compliance #7269

joakime commented Dec 13, 2021

joakime commented Dec 14, 2021 •

edited

Loading

joakime commented Dec 14, 2021 •

edited

Loading

joakime commented Dec 14, 2021

joakime commented Dec 14, 2021 •

edited

Loading

joakime commented Jul 14, 2022

joakime commented Jul 14, 2022

joakime commented Jul 14, 2022

github-actions bot commented Jul 15, 2023

github-actions bot commented Jul 15, 2024

gregw commented Jul 15, 2024

HostPort needs updates for spec compliance #7269

HostPort needs updates for spec compliance #7269

Comments

joakime commented Dec 13, 2021

joakime commented Dec 14, 2021 • edited Loading

joakime commented Dec 14, 2021 • edited Loading

joakime commented Dec 14, 2021

joakime commented Dec 14, 2021 • edited Loading

joakime commented Jul 14, 2022

joakime commented Jul 14, 2022

joakime commented Jul 14, 2022

github-actions bot commented Jul 15, 2023

github-actions bot commented Jul 15, 2024

gregw commented Jul 15, 2024

joakime commented Dec 14, 2021 •

edited

Loading

joakime commented Dec 14, 2021 •

edited

Loading

joakime commented Dec 14, 2021 •

edited

Loading