Node6 incompatibilities with querystrings

[cmidkiff87]

Node 6 changed the querystring.parse to return an object whose __proto__ property no longer points back to Object. This was done to allow for more esoteric query string names, such as https://example.com/route?__proto__=some-value, which would conflict with the native js object properties.

However, this causes js-data to error when calling hasOwnProperty() if you pass it the querystring object.

This is especially problematic when using js-data-http as it automatically sends the requests with the url's query strings in the format js-data expects.

It can be fixed by using

Object.prototype.hasOwnProperty(query, propKey);

in place of

query.hasOwnProperty(propKey);

---

The libraries involved are

• api:
  • node 5.11.1 // yikes
  • node 6.17.1
  • js-data 3.0.8
  • koa 1.7.0
• webapp
  • js-data 3.0.0-rc.4
  • js-data-http 3.0.0-rc.2

---

It can also be fixed externally by shimming the hasOwnProperty() function onto the querystring

exports.jsData = function() {
  // koa 1.x middleware
  return function*(next) {
    this.jsData = {};

    // `this.query` is the result of the `querystring.parse()` function
    this.jsData.query = this.query; 
    
    function hasOwnPropertyShim(key) {
      // `this` === `this.jsData.query`
      return Object.hasOwnProperty.call(this, key);
    }

    // make the prop unenumerable, or it will get included in the sql query
    Object.defineProperty(this.jsData.query, 'hasOwnProperty', {
      value: hasOwnPropertyShim,
      enumerable: false,
    });

    // ...
  }
}

However, that solution isn't super intuitive without some digging

[zuzusik]

Node 6 has been out of support for quite a time now. Is it also reproducible in newer versions of Node?

[cmidkiff87]

Yes, I can't remember exactly which versions I've tried it with, but the latest (14.x) docs state that

The object returned by the querystring.parse() method does not prototypically inherit from the JavaScript Object. This means that typical Object methods such as obj.toString(), obj.hasOwnProperty(), and others are not defined and will not work.

Of course, this is only an issue when passing the parsed querystring object directly to js-data (as we have), and is "fixable" by shimming the object with the required functions.

[crobinson42]

I do this on server side middleware for requests: // Patch request query to be an object for operating on it as an Object // this is because node.js strips down the prototype for performance/speed (ctx, next) => { if (ctx.request.query && typeof ctx.request.query === 'object') { // node.js removed the Object.prototype on querystring nodejs/node#6055 // coerce query to inherit from Object.prototype const def = { value: { ...ctx.request.query }, writable: false, } Object.defineProperty(ctx.request, 'query', def) Object.defineProperty(ctx, 'query', def) } return next() },

…

On Jul 13, 2020, at 12:41 PM, cmidkiff87 ***@***.***> wrote: Yes, I can't remember exactly which versions I've tried it with, but the latest (14.x) docs <https://nodejs.org/docs/latest-v14.x/api/querystring.html#querystring_querystring_parse_str_sep_eq_options> state that The object returned by the querystring.parse() method does not prototypically inherit from the JavaScript Object. This means that typical Object methods such as obj.toString(), obj.hasOwnProperty(), and others are not defined and will not work. Of course, this is only an issue when passing the parsed querystring object directly to js-data (as we have), and is "fixable" by shimming the object with the required functions. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#571 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABNSMS3IRAHG4AZDGT5YE63R3NPORANCNFSM4ODAKVUA>.

[cmidkiff87]

Then perhaps there should be a note about this somewhere in the documentation, rather than updating js-data?

(since this is probably more of an issue for upgrading legacy projects than it is for new projects)

[crobinson42]

I’m not sure that I see this is anything specific to js-data itself, it’s a Node.js quark.

…

On Jul 13, 2020, at 12:55 PM, cmidkiff87 ***@***.***> wrote: Then perhaps there should be a note about this somewhere in the documentation, rather than updating js-data? (since this is probably more of an issue for upgrading legacy projects than it is for new projects) — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#571 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABNSMS45DQJ4ARL6NF6MBULR3NRE5ANCNFSM4ODAKVUA>.

[zuzusik]

I think it might be handy to add this as some utility to js-data, which can be optionally called if required and print some warnings in console when it comes to the described situation with the advice to run this utility пн, 13 лип. 2020 о 12:55 cmidkiff87 <notifications@github.com> пише:

…

Then perhaps there should be a note about this somewhere in the documentation, rather than updating js-data? (since this is probably more of an issue for upgrading legacy projects than it is for new projects) — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#571 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABAJO6BXD7TMU6EGSJ2YTI3R3NRE5ANCNFSM4ODAKVUA> .