Giraffe icon indicating copy to clipboard operation
Giraffe copied to clipboard

One very specific subRoute that fails

Open reinux opened this issue 4 years ago • 3 comments

There seems to be one very specific subRoute (and longer versions of it) that inexplicably fails:

       subRoute "/admin/a7mPfJBOVuxLpXCCSYO5Lf2Ro" Admin.Dashboard.handler // OK
       subRoute "/admin/7mPfJBOVuxLpXCCSYO5Lf2Ro" Admin.Dashboard.handler  // Fails
       subRoute "/admin/7mPfJB" Admin.Dashboard.handler                    // OK
       subRoute "/admin/7mPfJBO" Admin.Dashboard.handler                   // Fails
       subRoute "/7mPfJBO" Admin.Dashboard.handler                         // OK
       subRoute "/bdmin/7mPfJBO" Admin.Dashboard.handler                   // OK

/admin/7mPfJBOVuxLpXCCSYO5Lf2Ro?asdf is also accepted.

Any idea what could be going on here?

reinux avatar Feb 17 '21 17:02 reinux

From your example it seems that subRoute "/admin/7mPfJB" Admin.Dashboard.handler is too greedy and swallows up the other routes too because it is a substring of those:

subRoute "/admin/7mPfJBOVuxLpXCCSYO5Lf2Ro" Admin.Dashboard.handler
subRoute "/admin/7mPfJBO" Admin.Dashboard.handler

I'll dig into it to see if it can be easily fixed. Thanks for reporting!

dustinmoris avatar Feb 28 '21 16:02 dustinmoris

Ahh, you're right. I didn't notice it only happens when there's something that starts the same prior.

Thanks!

reinux avatar Feb 28 '21 16:02 reinux

I noticed that the subRoute function calls the routeStartsWith (path: string), which basically checks if the request path .StartsWith a specific pattern.

The description of this function states that:

Filters an incoming HTTP request based on a part of the request path (case sensitive). Subsequent route handlers inside the given handler function should omit the already validated path.

With this in mind, I assume this function is working correctly.

If you need to have the endpoint validating the full path @reinux, you must use route instead of subRoute.

64J0 avatar Sep 23 '23 13:09 64J0