No URI decoding any more of the id in service get method

[idaho]

Steps to reproduce

Since the update to pre-release 5.0.0-pre.16, we no longer have a decoded ID in the service get method. We use the Feathers Express application and have an ID containing a / which is encoded. Before we upgraded to this version, it seems there was a built-in decoding for the ID.

Example

import { Application as eApplication } from '@feathersjs/express';
import { feathers } from '@feathersjs/feathers';
import qs from 'qs';
import { Service } from 'feathers-memory';
import * as express from '@feathersjs/express';

interface ApplicationSettings {
  port: number;
  host: string;
}

type Application = eApplication<{ foo: Service }, Record<string, any> & { configuration: ApplicationSettings }>;

const expressify = express.default;

const application: Application = expressify(feathers());

application.set('configuration', {
  port: 3_040,
  host: 'localhost'
});

application.set('etag', false);
application.disable('etag');
application.disable('x-powered-by');

application.set('query parser', (str: string): string => qs.stringify(str));
application.use(express.json({ limit: '50mb' }));
application.use(express.urlencoded({ limit: '50mb', extended: true }));
application.configure(express.rest());

const service = new Service();

application.use('foo', service);

// eslint-disable-next-line @typescript-eslint/no-floating-promises
(async (): Promise<void> => {
  // now we will create some fake data
  await application.service('foo').create({
    id: 'en/tmp',
    name: 'foo',
    date: new Date().toUTCString()
  });

  const { port } = application.get('configuration');

  const server = await application.listen(port);

  server.on('listening', (): void => {
    // eslint-disable-next-line no-console
    console.log(`server start on http://localhost:${port}`);
  });
})();

If we now run the application and make a request to http://localhost:3040/foo, we see a result list containing the created entry with the ID en/tmp. However, when we send a request to http://localhost:3040/foo/en%2Ftmp, we get the error No record found for ID en%2Ftmp. Before the update, the ID was URI decoded and the record was found.

Currently, we have a workaround where we do the decoding manually in the service method, as shown below.

class Service<T = any> extends memoryService<T> {
  // eslint-disable-next-line no-underscore-dangle, @typescript-eslint/naming-convention
  public async _get (id: Id, params?: Params): Promise<T> {
    const decodedId = decodeURIComponent(id as string);

    // eslint-disable-next-line no-underscore-dangle
    return super._get(decodedId, params);
  }
}

System configuration

feathersjs: since packages 5.0.0-pre.16

NodeJS version: 16.3.2

Operating System: MacOS 12.2.1

Module Loader: typescript / commonjs

Typescript: 4.5.5

[SteffenLanger]

Summary

I dug a little deeper and found the root cause: Feathers does not mount the service middleware in the express app at a route with URL parameters (e.g. users/:userId) but always at /. That prevents express from recognizing and decoding URL params.

Proposed Solution

@daffl, what do you think about decoding the URL params in feathers express within the function servicesMiddleware after the service lookup? I'd be happy to create a pull request if this is the right direction.

Technical Investigation

Package.json extract:

        "@feathersjs/express": "^5.0.25",
        "@feathersjs/feathers": "^5.0.25",

I created a feathers express app like so: const feathersApp: AxApplication = feathersExpress(feathers());

When executing feathersApp.service('users', new UserService()), the service is registered at feathersApp.services. It makes sense that no middleware is registered in express at this point since this code is part of the more generic feathers application itself that needs to work with various transports.

Feathers express registers a middleware function at / during the feathers express initialization. It basically replaces the native express lookup with a feathers-internal lookup.

Since the path / does not include any URL params, express can't recognize those params and won't decode them.