Setting req.query

[arb]

In my validation package celebrate, I run joi against the parsed query string. If there are any transformations applied (default value, convert "1" to 1, etc.) I want to set that result back to req.query so any downstream functions are working with the validated and updated query object.

In versions 3 and 4 of express, it's as simple as req.query = UPDATED_QUERY. In version 5, because of this change it's not as simple anymore.

What is the recommended way for doing this in version 5?

[dougwilson]

What does your code look like? Would this really just be a custom query parser?

[arb]

Code looks something like this... I've cut it down for brevity:

const Celebrate = (schema, options) => {
  const validateSource = (source) => {
    return (req, callback) => {
      const spec = rules.get(source);
      Joi.validate(req[source], spec, options, (err, value) => {
        if (value !== undefined) {
          // Apply any Joi transforms back to the request
          req[source] = value;
        }
        if (err) {
          err._meta = { source };
          return callback(err);
        }
        return callback(null);
      });
    };
  };
  const middleware = (req, res, next) => {
    Series(null, [
      validateSource('headers'),
      validateSource('params'),
      validateSource('query'),
      validateSource('body')
    ], req, next);
  };

  return middleware;
};

The full source can be found here

[dougwilson]

Ah, I see. It almost looks like what you're doing is trying to add on top of the query parser directly (since you are expecting to modify the result of the parsed query). The change you're seeing in Express 5 came from various feedback (and an issue was just filed a couple week ago that this commit you pointed to fixed in 5.0), so in general it seems like it's a change that we need to do, but I see it causes some kind of issues.

The commit itself is desired for three reasons: (1) the query parameters are no longer parsed until something actually needs them and (2) the "query parser" can be altered at any point and be reflected in the app and finally (3) it now always reflects changes made to req.url.

Without really doing much, a very "get this to go away" change would be to use Object.defineProperty to define a value for req.query that replaces the getter function in your middleware with the updated value. This would be 1-to-1 change that would behave as the above is written for Express 4 and lower.

Express 5 is alpha for just this reason -- to flush out issues not only in Express itself, but the community middlewares and people's apps. It is certainly open to change anything, so if you have a proposal for a better way to achieve the goals for that commit, we're open to whatever your suggestion is as well.

[arb]

Yeah I sort of figured my path forward was fiddling with Object.defineProperty but what my concern was blowing away something that was already attached to req.query. I think Object.definePropert is destructive and doesn't merge in subsequent calls. If this question comes up again, is that going to be the "blessed" approach or just one that kind of works for this instance?

While your suggestion would certainly make this go away, I'm concerned that there will be other changes to req.query that we'd lose with this approach.

[dougwilson]

I'm not sure, and I don't really understand how Object.defineProperty is worse, maybe you can show an example?

This is certainly open to change, so if you have a proposal for a better way to achieve the goals for that commit, we're open to whatever your suggestion is as well.

[arb]

I guess it's not worse in this case... My thinking was that in 3 and 4, req.query was just a POJO, nothing fancy. So replacing it with another POJO that was the same shape wasn't a problem. I guess my concern here is replacing the built-in getter here would impact downstream functions if more logic is added to that getter function. I'll try your suggestion and see how it works out.

As far as suggestions for a different interface, I don't really have one. I think the current approach is fine 👍

Side bar question, is version 5 published with a tag so I can do npm i express@alpha or anything?

[arb]

So I am trying to conditionally determine when I need to do this because my library validates "headers", "params", "query" and "body". Why does Object.getOwnPropertyDescriptor(req, 'query') return undefined? Looking at the code, it doesn't seem to be doing anything to the prototype. Additionally, that same lookup returns valid property descriptors for "headers" and "params".

Sorry for all the questions, just don't understand what is going on.

[wesleytodd]

@arb I know this is not what you want to hear, but I think a better long term solution would be to have your validation library move all of your parsed input to res.locals.query. Then have your downstream middleware use that. Mucking with the built in stuff is both complicated to get right, and dangerous as we move forward.

For example, if you used a third party middleware that thought all values on req.query were strings, they might do something that breaks when it gets your number. It's just safer to stay away from changing any of the input stuff like req.query and req.body. Just my 2c.

[coyotte508]

@arb You can do npm install express@5.0.0-alpha.6

Regarding your second question, I don't know why either. The current code is:

defineGetter(req, 'query', function query(){
  var queryparse = this.app.get('query parser fn');

  if (!queryparse) {
    // parsing is disabled
    return Object.create(null);
  }

  var querystring = parse(this).query;

  return queryparse(querystring);
});

//...

function defineGetter(obj, name, getter) {
  Object.defineProperty(obj, name, {
    configurable: true,
    enumerable: true,
    get: getter
  });
};

req.headers and req.params are not defined through getters however.

Regardless, for express 4, Object.getOwnPropertDescriptor returns something like:

{ value: {},
  writable: true,
  enumerable: true,
  configurable: true }

So maybe you can do a test (a bit hackish) like that:

hasGetters (req) {
  const descriptor = Object.getOwnPropertDescriptor(req);

  return !descriptor || descriptor.get;
}