7564 Ordering with JSON attributes (#7565)

* Two birds with one stone.  Adds support for ordering by JSON columns and removes vulnerability for injections on JSON attributes.

* Updates the changelog for the addition of json attributes.

* Fixes lint error with prefer arrow.

* Removes specific error because it may change depending on dialect and engine version.

* Removed duplicate tests and moved location of SQL injection test and find order test.

* Optimizes item.split performance by only performing the split once.

* Fixes test for sqlite dialect json.  JSON is handled differently in sqlite where the json value is extracted from a path.  If the path doesn't exist sqlite returns null instead of throwing an error like postgres.

changelog.md

@@ -68,6 +68,7 @@
 - [REMOVED] Removes support for `{raw: 'injection goes here'}` for order and group. [#7188](https://github.com/sequelize/sequelize/issues/7188)
 - [FIXED] `showIndex` breaks with newline characters [#7492](https://github.com/sequelize/sequelize/pull/7492)
 - [FIXED] Update or soft delete breaks when querying on `JSON/JSONB` [#7376](https://github.com/sequelize/sequelize/issues/7376) [#7400](https://github.com/sequelize/sequelize/issues/7400) [#7444](https://github.com/sequelize/sequelize/issues/7444)
+- [ADDED] Support for JSON attributes in orders and groups. [#7564](https://github.com/sequelize/sequelize/issues/7564)
 - [REMOVED] Removes support for interpretation of raw properties and values in the where object. [#7568](https://github.com/sequelize/sequelize/issues/7568)
 - [FIXED] Upsert now updates all changed fields by default
 

lib/dialects/abstract/query-generator.js

@@ -795,6 +795,25 @@ const QueryGenerator = {
             } else if (previousModel.rawAttributes !== undefined && previousModel.rawAttributes[item] && item !== previousModel.rawAttributes[item].field) {
               // convert the item attribute from it's alias
               item = previousModel.rawAttributes[item].field;
+            } else if (
+              item.indexOf('.') !== -1
+              && previousModel.rawAttributes !== undefined
+            ) {
+              const itemSplit = item.split('.');
+
+              if (previousModel.rawAttributes[itemSplit[0]].type instanceof DataTypes.JSON) {
+                // just quote identifiers for now
+                const identifier = this.quoteIdentifiers(previousModel.name  + '.' + previousModel.rawAttributes[itemSplit[0]].field);
+
+                // get path
+                const path = itemSplit.slice(1);
+
+                // extract path
+                item = this.jsonPathExtractionQuery(identifier, path);
+
+                // literal because we don't want to append the model name when string
+                item = this.sequelize.literal(item);
+              }
             }
           }
         }
@@ -2389,21 +2408,6 @@ const QueryGenerator = {
     }, []);
   },
 
-  /**
-   * Generates an SQL query that extract JSON property of given path.
-   *
-   * @param   {String}               column  The JSON column
-   * @param   {String|Array<String>} [path]  The path to extract (optional)
-   * @returns {String}                       The generated sql query
-   * @private
-   */
-  jsonPathExtractionQuery(column, path) {
-    const paths = _.toPath(path);
-    const pathStr = `{${paths.join(',')}}`;
-    const quotedColumn = this.isIdentifierQuoted(column) ? column : this.quoteIdentifier(column);
-    return `${quotedColumn}#>>'${pathStr}'`;
-  },
-
   isIdentifierQuoted(string) {
     return /^\s*(?:([`"'])(?:(?!\1).|\1{2})*\1\.?)+\s*$/i.test(string);
   },

lib/dialects/postgres/query-generator.js

@@ -196,9 +196,9 @@ const QueryGenerator = {
    */
   jsonPathExtractionQuery(column, path) {
     const paths = _.toPath(path);
-    const pathStr = `{${paths.join(',')}}`;
+    const pathStr = this.escape(`{${paths.join(',')}}`);
     const quotedColumn = this.isIdentifierQuoted(column) ? column : this.quoteIdentifier(column);
-    return `(${quotedColumn}#>>'${pathStr}')`;
+    return `(${quotedColumn}#>>${pathStr})`;
   },
 
   handleSequelizeMethod(smth, tableName, factory, options, prepend) {

lib/dialects/sqlite/query-generator.js

@@ -148,13 +148,13 @@ const QueryGenerator = {
    */
   jsonPathExtractionQuery(column, path) {
     const paths = _.toPath(path);
-    const pathStr = ['$']
+    const pathStr = this.escape(['$']
       .concat(paths)
       .join('.')
-      .replace(/\.(\d+)(?:(?=\.)|$)/g, (_, digit) => `[${digit}]`);
+      .replace(/\.(\d+)(?:(?=\.)|$)/g, (_, digit) => `[${digit}]`));
 
     const quotedColumn = this.isIdentifierQuoted(column) ? column : this.quoteIdentifier(column);
-    return `json_extract(${quotedColumn}, '${pathStr}')`;
+    return `json_extract(${quotedColumn}, ${pathStr})`;
   },
 
   handleSequelizeMethod(smth, tableName, factory, options, prepend) {

test/integration/model/json.test.js

@@ -399,6 +399,78 @@ describe(Support.getTestDialectTeaser('Model'), () => {
             });
           });
         });
+
+        it('should be possible to query a nested value and order results', function() {
+          return this.Event.create({
+            data: {
+              name: {
+                first: 'Homer',
+                last: 'Simpson'
+              },
+              employment: 'Nuclear Safety Inspector'
+            }
+          }).then(() => {
+            return Promise.join(
+              this.Event.create({
+                data: {
+                  name: {
+                    first: 'Marge',
+                    last: 'Simpson'
+                  },
+                  employment: 'Housewife'
+                }
+              }),
+              this.Event.create({
+                data: {
+                  name: {
+                    first: 'Bart',
+                    last: 'Simpson'
+                  },
+                  employment: 'None'
+                }
+              })
+            );
+          }).then(() => {
+            return this.Event.findAll({
+              where: {
+                data: {
+                  name: {
+                    last: 'Simpson'
+                  }
+                }
+              },
+              order: [
+                ['data.name.first']
+              ]
+            }).then(events => {
+              expect(events.length).to.equal(3);
+
+              expect(events[0].get('data')).to.eql({
+                name: {
+                  first: 'Bart',
+                  last: 'Simpson'
+                },
+                employment: 'None'
+              });
+
+              expect(events[1].get('data')).to.eql({
+                name: {
+                  first: 'Homer',
+                  last: 'Simpson'
+                },
+                employment: 'Nuclear Safety Inspector'
+              });
+
+              expect(events[2].get('data')).to.eql({
+                name: {
+                  first: 'Marge',
+                  last: 'Simpson'
+                },
+                employment: 'Housewife'
+              });
+            });
+          });
+        });
       });
 
       describe('destroy', () => {
@@ -505,7 +577,78 @@ describe(Support.getTestDialectTeaser('Model'), () => {
             });
           });
         });
+
+        it('should query an instance with JSONB data and order while trying to inject', function () {
+          return this.Event.create({
+            data: {
+              name: {
+                first: 'Homer',
+                last: 'Simpson'
+              },
+              employment: 'Nuclear Safety Inspector'
+            }
+          }).then(() => {
+            return Promise.join(
+              this.Event.create({
+                data: {
+                  name: {
+                    first: 'Marge',
+                    last: 'Simpson'
+                  },
+                  employment: 'Housewife'
+                }
+              }),
+              this.Event.create({
+                data: {
+                  name: {
+                    first: 'Bart',
+                    last: 'Simpson'
+                  },
+                  employment: 'None'
+                }
+              })
+            );
+          }).then(() => {
+            if (current.options.dialect === 'sqlite') {
+              return this.Event.findAll({
+                where: {
+                  data: {
+                    name: {
+                      last: 'Simpson'
+                    }
+                  }
+                },
+                order: [
+                  ["data.name.first}'); INSERT INJECTION HERE! SELECT ('"]
+                ]
+              }).then(events => {
+                expect(events).to.be.ok;
+                expect(events[0].get('data')).to.eql({
+                  name: {
+                    first: 'Homer',
+                    last: 'Simpson'
+                  },
+                  employment: 'Nuclear Safety Inspector'
+                });
+              });
+            } else if (current.options.dialect === 'postgres') {
+              return expect(this.Event.findAll({
+                where: {
+                  data: {
+                    name: {
+                      last: 'Simpson'
+                    }
+                  }
+                },
+                order: [
+                  ["data.name.first}'); INSERT INJECTION HERE! SELECT ('"]
+                ]
+              })).to.eventually.be.rejectedWith(Error);
+            }
+          });
+        });
       });
     });
   }
+
 });