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

Routing: PathMatcher.repeat(…) incorrectly matches an empty pattern #2097

Closed
rrwright opened this Issue Jun 29, 2018 · 3 comments

Comments

Projects
None yet
5 participants
@rrwright
Copy link

rrwright commented Jun 29, 2018

A route that contains the following path matcher:
path("node" / PathMatchers.JavaUUID.repeat(1, Int.MaxValue, ",")) { uuids => … }

correctly matches paths like:
http://localhost:8080/node/5fcab910-b9b9-43de-b102-4eb3be66ff4c,00000000-0000-0000-0000-000000000002

but incorrectly still matches paths which contain no UUIDs, like:
http://localhost:8080/node/

The minimum value passed in to the PathMatcher.repeat(…) method is not respected.

I can see why here:
https://github.com/akka/akka-http/blob/master/akka-http/src/main/scala/akka/http/scaladsl/server/PathMatcher.scala#L85

but not being able to understand why there is a hardcoded value of 1 passed in to rec here kept me from submitting a patch:
https://github.com/akka/akka-http/blob/master/akka-http/src/main/scala/akka/http/scaladsl/server/PathMatcher.scala#L83

@raboof

This comment has been minimized.

Copy link
Member

raboof commented Jun 29, 2018

At first glance looks like an oversight/off-by-one - try it and make sure to check with tests?

@Prithvirajbilla

This comment has been minimized.

Copy link
Contributor

Prithvirajbilla commented Jun 29, 2018

Hi, I'd like to pick this up. Thanks

Prithvirajbilla added a commit to Prithvirajbilla/akka-http that referenced this issue Jul 4, 2018

Prithvirajbilla added a commit to Prithvirajbilla/akka-http that referenced this issue Jul 4, 2018

Prithvirajbilla added a commit to Prithvirajbilla/akka-http that referenced this issue Jul 4, 2018

@ktoso ktoso closed this in #2105 Aug 20, 2018

ktoso added a commit that referenced this issue Aug 20, 2018

Fixes bug in PathMatcher.repeat incorrectly matching empty pattern (#…
…2105)

* Fixes bug in PathMatcher.repeat #2097

* added more test

* increased readablity of code.

@ktoso ktoso added this to the 10.1.4 milestone Aug 20, 2018

@ktoso ktoso removed the 3 - in progress label Aug 20, 2018

@ktoso ktoso modified the milestones: 10.1.4, 10.1.5 Aug 20, 2018

@ktoso

This comment has been minimized.

Copy link
Member

ktoso commented Aug 20, 2018

Realised this is 10.1.5 now since 10.1.4 was already tagged.

sanity check @raboof

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.