Case sensitivity of path matching

jeffreythomasprice commented 7 months ago

Steps to reproduce

Launch a feathers app with at least one rest path, e.g. /hello
Hit that path with some letters capitalized differently, e.g. /HELLO

Expected behavior

Any capitalization of the path should hit the same feathers service.

It appears that paths were matched in a case-insensitive manner prior to feathers 5.

Actual behavior

Exact capitalization is required. It does not appear to be configurable.

System configuration

We recently upgraded from feathers 4.5.11 to 5.0.8.
We found relevant code in packages/transport-commons/src/routing/router.ts, which is on 5.0.8.
Node 16.14.0, although I can't imagine that's relevant.

Code Sample

We're using express, and we tried playing with express properties to control case-sensitivity, e.g.

// app is the express app in this snippet, not a feathers app
app.set('case sensitive routing', true);

We played with several variants of constructing an express app and configuring it's path matching, then associating it with a feathers app. After we found packages/transport-commons/src/routing/router.ts it became clear that the path matching isn't using express' built-in behavior at all.

We found some text in this document https://feathersjs.com/guides/whats-new.html that references new path matching behavior, but no mention of case sensitivity. That links to https://feathersjs.com/api/application.html#lookup-path, but ditto.

We've worked around this by changing the path matching behavior in packages/transport-commons/src/routing/router.ts. Our snippet is slightly more complicated, but the relevant change is:

$ git diff
diff --git a/packages/transport-commons/src/routing/router.ts b/packages/transport-commons/src/routing/router.ts
index ad76319b..c46fe0b7 100644
--- a/packages/transport-commons/src/routing/router.ts
+++ b/packages/transport-commons/src/routing/router.ts
@@ -90,7 +90,10 @@ export class RouteNode<T = any> {
     }

     const current = path[this.depth]
-    const child = this.children[current]
+    const currentLowerCase = current.toLowerCase()
+    const child = Object.entries(this.children).find(
+      ([child]) => child.toLowerCase() === currentLowerCase
+    )?.[1]

     if (child) {
       const lookup = child.lookup(path, info)

daffl commented 7 months ago

That is definitely an issue that slipped through. #3353 has a backwards compatible fix that allows case insensitive route lookups by setting

app.routes.caseSensitive = false

jeffreythomasprice commented 7 months ago

Appreciate it! I'll look out for the next release.

feathersjs / feathers