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

4.x: HttpClient first URI segment is omitted if path starts with // #7474

Closed
romain-grecourt opened this issue Aug 26, 2023 · 4 comments · Fixed by #7657
Closed

4.x: HttpClient first URI segment is omitted if path starts with // #7474

romain-grecourt opened this issue Aug 26, 2023 · 4 comments · Fixed by #7657
Assignees
Labels
4.x Version 4.x bug Something isn't working P2 webclient
Milestone

Comments

@romain-grecourt
Copy link
Contributor

Environment Details

  • Helidon Version: 4.0.0-M1
  • Helidon SE

When the path of a URI starts with // the first segment is omitted.
E.g. http://localhost:8080//foo/bar will make a request to /bar !

This is awkward as it is common to have a trailing slash in a URI (E.g. http://localhost:8080/), resolving that URI without normalizing can lead to a path that starts with a double slash.

E.g.

String uri = "http://localhost:8080/"
String baseUri = uri + "/foo/bar";
URI uri = URI.create("http://localhost:8080/")
String baseUri1 = uri + "/foo/bar"; // bad
URI baseUri2 = uri.resolve("/foo/bar"); // good
String baseUri3 = uri.normalize() + "/foo/bar"; // good

Reproducer:

WebServer server = WebServer.builder()
        .routing(r -> r.any("/{+}", (req, res) -> res.send(req.requestedUri().path().toString())))
        .build()
        .start();
try {
    Http1Client client = Http1Client.create();
    String baseUri = "http://localhost:" + server.port();
    System.out.println(client.get(baseUri + "//foo/bar").request(String.class).entity());
    // prints /bar
    System.out.println(client.get(baseUri + "/foo/bar").request(String.class).entity());
    // prints /foo/bar
} finally {
    server.stop();
}

Note that I ran into this while writing a unit test that retrieved the URI for given socket:

@BeforeAll
static void init(@Socket("backend") URI uri) {
    // uri has a trailing /
}
@romain-grecourt romain-grecourt added bug Something isn't working webclient 4.x Version 4.x labels Aug 26, 2023
@tomas-langer
Copy link
Member

We also need to support (from conversation with @romain-grecourt, and other expected cases):

WebClient.builder().baseUri("http://localhost:8080//foo/bar")
webclient.get()
 .uri("http://localhost//foo/bar")

This is only for the initial leading / being duplicated, do not fix anywhere else

@m0mus m0mus added this to the 4.0.0 milestone Sep 14, 2023
@m0mus m0mus added the P2 label Sep 14, 2023
@danielkec
Copy link
Contributor

danielkec commented Sep 24, 2023

@romain-grecourt @tomas-langer This is actually a server issue eg.: URI.create("//foo/bar").normalize().getPath() -> "/bar", do we also want to introduce URI normalization to the client?

To avoid client yielding //foo/bar in the case mentioned by Tomas above?:

WebClient.builder().baseUri("http://localhost:8080//foo/bar").build()
        .get().uri().path()

danielkec added a commit to danielkec/helidon that referenced this issue Sep 24, 2023
@danielkec danielkec linked a pull request Sep 24, 2023 that will close this issue
@danielkec
Copy link
Contributor

Feels like a bug in the java.net.URI - JDK-8316799

danielkec added a commit to danielkec/helidon that referenced this issue Sep 24, 2023
@danielkec
Copy link
Contributor

Huh not really a JDK bug, TIL about net_path //foo is a net_path as defined by RFC 2396

absoluteURI = scheme ":" ( hier_part | opaque_part )
hier_part = ( net_path | abs_path ) [ "?" query ]
net_path = "//" authority [ abs_path ]
abs_path = "/" path_segments
opaque_part = uric_no_slash *uric
uric_no_slash = unreserved | escaped | ";" | "?" | ":" | "@" |"&" | "=" | "+" | "$" | ","

danielkec added a commit to danielkec/helidon that referenced this issue Sep 24, 2023
danielkec added a commit that referenced this issue Sep 25, 2023
* Double-slash URI issue #7474

* Flaky test fix

* Review issues 2
@barchetta barchetta modified the milestones: 4.0.0, 4.0.0-RC1 Sep 25, 2023
@m0mus m0mus added this to Backlog Aug 12, 2024
@m0mus m0mus moved this to Closed in Backlog Aug 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4.x Version 4.x bug Something isn't working P2 webclient
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

5 participants