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

Issues with path params and routes ending with /* #2395

Closed
synth3 opened this issue Mar 23, 2023 · 2 comments
Closed

Issues with path params and routes ending with /* #2395

synth3 opened this issue Mar 23, 2023 · 2 comments
Labels

Comments

@synth3
Copy link
Contributor

synth3 commented Mar 23, 2023

Version

4.4.0

Context

I encountered an some problems regarding path matching with path params and routes ending with /* while writing tests for an application that uses the vertx http-server and router:

  • Some requests for paths that would match without path params do not match when path params are used

Do you have a reproducer?

New router tests in #2396

Steps to reproduce

Create the following Routes

router.route("/a/:id/b/:id2/c/*").handler(rc -> {
    assertEquals("123", rc.pathParam("id"));
    assertEquals("1234", rc.pathParam("id2"));
    rc.response().end("r4");
});

router.route("/a/:id/b1/*").handler(rc -> {
    assertEquals("123", rc.pathParam("id"));
    rc.response().end("r3");
});

router.route("/a/:id/*").handler(rc -> {
    assertEquals("123", rc.pathParam("id"));
    rc.response().end("r2");
});

router.route("/:id/*").handler(rc -> {
    assertEquals("123", rc.pathParam("id"));
    rc.end("r1");
});

Test against these routes

Working:

// should call "/:id/*"
testRequest(HttpMethod.GET, "/123/foo", 200, "OK", "r1");
testRequest(HttpMethod.GET, "/123/", 200, "OK", "r1");

// should call "/a/:id/*"
testRequest(HttpMethod.GET, "/a//123/", 200, "OK", "r2");
testRequest(HttpMethod.GET, "/a/123/foo/bar", 200, "OK", "r2");

// should call "/a/:id/b1/*"
testRequest(HttpMethod.GET, "/a//123/b1/", 200, "OK", "r3");
testRequest(HttpMethod.GET, "/a/123/b1/foo", 200, "OK", "r3");

// should call "/a/:id/b/:id2/c/*"
testRequest(HttpMethod.GET, "/a//123/b/1234/c/", 200, "OK", "r4");
testRequest(HttpMethod.GET, "/a/123/b/1234/c/foo", 200, "OK", "r4");

404:

// should call "/:id/*"
testRequest(HttpMethod.GET, "/123", 200, "OK", "r1");

// should call "/a/:id/*"
testRequest(HttpMethod.GET, "/a/123", 200, "OK", "r2");
testRequest(HttpMethod.GET, "/a//123", 200, "OK", "r2");

// should call "/a/:id/b1/*"
testRequest(HttpMethod.GET, "/a/123/b1", 200, "OK", "r3");
testRequest(HttpMethod.GET, "/a//123//b1", 200, "OK", "r3");

// should call "/a/:id/b/:id2/c/*"
testRequest(HttpMethod.GET, "/a/123/b/1234/c", 200, "OK", "r4");

Wrong handler called (problem with normalization?):

// called handler of route "/a/:id/*" instead of route "/a/:id/b/:id2/c/*"
testRequest(HttpMethod.GET, "/a//123//b//1234//c", 200, "OK", "r4");
@pmlopes
Copy link
Member

pmlopes commented Mar 24, 2023

The rules are not as expected in this issue. I've updated the docs to reflect the correct rules.

@synth3
Copy link
Contributor Author

synth3 commented Mar 24, 2023

@pmlopes Thanks alot for looking into this!

My assumtions have been based on the behavior of routes without path params and this documentation (which currently reflects that behavior):

Maybe that documentation should be updated too.

The same Tests without the path params do all work (for comparability I substituted the params with the static segment noparam):

@Test
public void testPathPrefixWithoutPathParams() throws Exception {
  router.route("/a/noparam/b/noparam/c/*").handler(rc -> {
    rc.response().end("r4");
  });

  router.route("/a/noparam/b1/*").handler(rc -> {
    rc.response().end("r3");
  });

  router.route("/a/noparam/*").handler(rc -> {
    rc.response().end("r2");
  });

  router.route("/noparam/*").handler(rc -> {
    rc.end("r1");
  });

  // should call "/noparam/*"
  testRequest(HttpMethod.GET, "/noparam/foo", 200, "OK", "r1");
  testRequest(HttpMethod.GET, "/noparam/", 200, "OK", "r1");
  // should call "/a/noparam/*"
  testRequest(HttpMethod.GET, "/a//noparam/", 200, "OK", "r2");
  testRequest(HttpMethod.GET, "/a/noparam/foo/bar", 200, "OK", "r2");
  // should call "/a/noparam/b1/*"
  testRequest(HttpMethod.GET, "/a//noparam/b1/", 200, "OK", "r3");
  testRequest(HttpMethod.GET, "/a/noparam/b1/foo", 200, "OK", "r3");
  // should call "/a/noparam/b/noparam/c/*"
  testRequest(HttpMethod.GET, "/a//noparam/b/noparam/c/", 200, "OK", "r4");
  testRequest(HttpMethod.GET, "/a/noparam/b/noparam/c/foo", 200, "OK", "r4");


  // should call "/noparam/*"
  testRequest(HttpMethod.GET, "/noparam", 200, "OK", "r1");
  // should call "/a/noparam/*"
  testRequest(HttpMethod.GET, "/a/noparam", 200, "OK", "r2");
  testRequest(HttpMethod.GET, "/a//noparam", 200, "OK", "r2");
  // should call "/a/noparam/b1/*"
  testRequest(HttpMethod.GET, "/a/noparam/b1", 200, "OK", "r3");
  testRequest(HttpMethod.GET, "/a//noparam//b1", 200, "OK", "r3");
  // should call "/a/noparam/b/noparam/c/*"
  testRequest(HttpMethod.GET, "/a/noparam/b/noparam/c", 200, "OK", "r4");
  testRequest(HttpMethod.GET, "/a//noparam//b//noparam//c", 200, "OK", "r4");
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

2 participants