-
-
Notifications
You must be signed in to change notification settings - Fork 982
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
Matching parameters containing the delimiter #781
Comments
It looks like the closest test cases are tr.InsertRoute(mGET, "/articles/files/{file}.{ext}", hStub12) {m: mGET, r: "/articles/files/file.zip", h: hStub12, k: []string{"file", "ext"}, v: []string{"file", "zip"}},
{m: mGET, r: "/articles/files/photos.tar.gz", h: hStub12, k: []string{"file", "ext"}, v: []string{"photos", "tar.gz"}},
{m: mGET, r: "/articles/files/photos.tar.gz", h: hStub12, k: []string{"file", "ext"}, v: []string{"photos", "tar.gz"}}, where the |
Out of curiosity, have you tried using regex matcher or match the whole filename |
Thank you for the test case. It's an example which doesn't work anymore if the first parameter is greedy. Matching the whole filename works but requires manual routing. In my usecase I need the routes
My current workaround can be seen here: https://github.com/go-gitea/gitea/pull/22404/files#diff-0a90ef0e0fa95e2969bf6f0dc0aad6245eee6b078e1fd98ea7d201defb7f5e12R305-R309 r.Get("/{version}", func(ctx *context.Context) {
version := ctx.Params("version")
if strings.HasSuffix(version, ".zip") {
...
} else if strings.HasSuffix(version, ".json") {
...
} else {
...
}
}) |
This is also the root cause behind #758. Most other routers seem to only allow a single param per path segment (without static prefixes or suffixes), probably to avoid pitfalls like this (i haven't looked at them all).
I think he wants It may be beneficial to add some documentation regarding the non greedy nature of param matching. |
Regex returns the longest leftmost match. So if I put two greedy groups next to each other, I expect the left group to be "more" greedy than the right group. See https://regex101.com/r/6MfS1E/1. So in this case, I'd expect the same. For Go's |
Maybe a little bit more matching logic could help:
So the tree must know if there is a single placeholder in a segment and if there is prefix and/or suffix or if there are multiple placeholders. |
well I did a workaround for now: https://github.com/go-gitea/gitea/pull/23874/files but it's ugly |
testcase: r.Group("/{stringParam}", func() {
r.Get("", func(ctx *context.Context) {
p := ctx.Params("stringParam")
// on `curl http://localhost/test.it` expect it to be "test.it"
})
r.Get(".png", func(ctx *context.Context) {
p := ctx.Params("stringParam")
// on `curl http://localhost/test.it.png` expect it to be "test.it"
})
}) |
… ... (#23874) - close #22301 workaround for go-chi/chi#781
well I did open #811 ... let's see |
Example:
This fails because of (how I think) the routing algorithm works. For
{name}.json
the first character after the placeholder is saved as "tail delimiter". The route search now resolves the placeholder by searching for the delimiter in the string. For/test.json
this searches for.
and the result istest
. However if the request is/1.0.json
this logic fails to resolve the correct value because the delimiter.
is contained in the value. The result is1
instead of the expected1.0
.A possible fix would be to search for the whole non-placeholder-suffix
.json
. That would still fail for something like{name}.
or{name}.{extension}
.Another fix would be to make the search greedy and read the value until the last occurence of the delimiter. As this works now fine for my case it fails for
{name}.{other}.json
even without the delimiter inside the values. Still I think the greedy matching is more often what you would expect.Greedy example:
The text was updated successfully, but these errors were encountered: