Bad request returned for percent-encoded get_by_attribute

[roman-khimov]

Current Behavior

https://rest.fs.neo.org/7ZVSQHtioVRiDT2btN9xEmQhczXV9vryCEAHUawfD8Sc/7-1712215152/index.html#suites/4f5ca4977289636df3b53e1debc15dcf/84697d2930280bd1/

Exception: Failed to get object via HTTP gate:
request: /v1/get_by_attribute/A9pnctophx5PFt4eUd8d2aa6otjgnQZNzyvt3HCeygFD/cat%25jpeg/cat%25jpeg,
response: {"message":"Invalid format for parameter attrKey: error unescaping path parameter 'attrKey': invalid URL escape "%jp""}
,
status code: 400 Bad Request

Expected Behavior

No error, the URL seems to be fine here.

[smallhive]

The short problem description is double parameter unescaping.

Let's talk about the details. The incoming request has URI value /v1/get_by_attribute/29o9cpj1qo7KWFrrqpw3LtUe4o91TYWcf5mHv7ZGMLKL/cat%25jpeg/cat%25jpeg.

Standart HTTP lib reads the request and makes the first unescape approach. On this step, the URI changed to /v1/get_by_attribute/A9pnctophx5PFt4eUd8d2aa6otjgnQZNzyvt3HCeygFD/cat%jpeg/cat%jpeg.
The next step processes swagger parameters and BindStyledParameterWithOptions tries unescape the variable cat%jpeg one more time.

It sees the % symbol and tries to represent it as a valid escape symbol. Unfortunately %jp is not a valid symbol.
As a result, we have error {"message": "Invalid format for parameter attrKey: error unescaping path parameter 'attrKey': invalid URL escape \"%jp\"}

I hope the allowReserved keyword will help us, but it is still not implemented oapi-codegen/oapi-codegen#1342.

Fast and in some way a dirty hack is using double parameter escaping in case it contains %.
The next version works fine /v1/get_by_attribute/29o9cpj1qo7KWFrrqpw3LtUe4o91TYWcf5mHv7ZGMLKL/cat%2525jpeg/cat%2525jpeg.

Also, the testcases should be updated to use the latest REST gate version for tests. Actual is 0.8.3, tests are using 0.6.0

[roman-khimov]

We can't solve it at the moment, dependencies need to be fixed and updated.

[roman-khimov]

Associated test case will be removed for now.

[roman-khimov]

BTW, have you looked at https://github.com/labstack/echo/blob/447c92d842e2f91c07eb032f619bea212beeee60/echo.go#L951? It's supposed to be using RawPath which is unescaped.

[roman-khimov]

In fact, this change to echo fixes the problem:

--- a/echo.go
+++ b/echo.go
@@ -657,12 +657,12 @@ func (e *Echo) ServeHTTP(w http.ResponseWriter, r *http.Request) {
        var h HandlerFunc
 
        if e.premiddleware == nil {
-               e.findRouter(r.Host).Find(r.Method, GetPath(r), c)
+               e.findRouter(r.Host).Find(r.Method, r.URL.EscapedPath(), c)
                h = c.Handler()
                h = applyMiddleware(h, e.middleware...)
        } else {
                h = func(c Context) error {
-                       e.findRouter(r.Host).Find(r.Method, GetPath(r), c)
+                       e.findRouter(r.Host).Find(r.Method, r.URL.EscapedPath(), c)
                        h := c.Handler()
                        h = applyMiddleware(h, e.middleware...)
                        return h(c)

but it's a huge can of worms:
labstack/echo#2168
labstack/echo#1974

which was flipped back and forth already:
labstack/echo#1628
labstack/echo#1799

[roman-khimov]

Maybe it's worth trying -generate std-http oapi-codegen option. They say it's Go 1.22+ only, so be it, but I have some hope it works a bit better than any of the alternatives. We don't need a lot from a router, really.

[roman-khimov]

⬆️ can be tried now that we have Go 1.22+ required anyway.

[roman-khimov]

In 1.22, each segment of a pattern is unescaped; this was not done in 1.21. For example, in 1.22 the pattern "/%61" matches the path "/a" ("%61" being the URL escape sequence for "a"), but in 1.21 it would match only the path "/%2561" (where "%25" is the escape for the percent sign).
When matching patterns to paths, in 1.22 each segment of the path is unescaped; in 1.21, the entire path is unescaped. This change mostly affects how paths with %2F escapes adjacent to slashes are treated. See https://go.dev/issue/21955 for details.

[roman-khimov]

oapi-codegen/runtime#35

[roman-khimov]

https://github.com/nuts-foundation/nuts-node/pull/2832/files ?

[roman-khimov]

While what we want to do here is perfectly fine as per HTTP specification, we can't fit this into our current stack. If something changes in libraries we can return to this problem, but for now it's WONTFIX.

[roman-khimov]

The case itself is not that popular, btw, so this is a minor limitation. And one can always search/fetch by oid if needed.