From 7cf2e49e03529f73036aed9ec0953ebda52af988 Mon Sep 17 00:00:00 2001
From: Yuki von Kanel <poison@ruadh.io>
Date: Fri, 17 Apr 2020 18:46:18 -0500
Subject: [PATCH] Fix input not normalising to ObjectId for certain query
 operators
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The following operators were affected and did not have suitable input
values normalised to ObjectId instances prior:
  • >
  • <
  • >=
  • <=

A tryNormalizeMongoObjectId helper function was added to minimise
repetition while implementing this fix.
---
 .../private/build-mongo-where-clause.js       | 21 +++++++++---
 .../private/try-normalize-mongo-object-id.js  | 32 +++++++++++++++++++
 2 files changed, 49 insertions(+), 4 deletions(-)
 create mode 100644 lib/private/machines/private/try-normalize-mongo-object-id.js

diff --git a/lib/private/machines/private/build-mongo-where-clause.js b/lib/private/machines/private/build-mongo-where-clause.js
index 26d74afe7..4c0f018db 100644
--- a/lib/private/machines/private/build-mongo-where-clause.js
+++ b/lib/private/machines/private/build-mongo-where-clause.js
@@ -6,6 +6,7 @@ var util = require('util');
 var assert = require('assert');
 var _ = require('@sailshq/lodash');
 var normalizeMongoObjectId = require('./normalize-mongo-object-id');
+var tryNormalizeMongoObjectId = require('./try-normalize-mongo-object-id');
 
 
 /**
@@ -131,19 +132,31 @@ module.exports = function buildMongoWhereClause(whereClause, WLModel, meta) {
     switch (modifierKind) {
 
       case '<':
-        constraint['$lt'] = modifier;
+        // Apply our modifier as an ObjectId where appropriate.
+        constraint['$lt'] = (doCompareAsObjectIdIfPossible && _.isString(modifier))
+          ? tryNormalizeMongoObjectId(modifier)
+          : modifier;
         break;
 
       case '<=':
-        constraint['$lte'] = modifier;
+        // Apply our modifier as an ObjectId where appropriate.
+        constraint['$lte'] = (doCompareAsObjectIdIfPossible && _.isString(modifier))
+          ? tryNormalizeMongoObjectId(modifier)
+          : modifier;
         break;
 
       case '>':
-        constraint['$gt'] = modifier;
+        // Apply our modifier as an ObjectId where appropriate.
+        constraint['$gt'] = (doCompareAsObjectIdIfPossible && _.isString(modifier))
+          ? tryNormalizeMongoObjectId(modifier)
+          : modifier;
         break;
 
       case '>=':
-        constraint['$gte'] = modifier;
+        // Apply our modifier as an ObjectId where appropriate.
+        constraint['$gte'] = (doCompareAsObjectIdIfPossible && _.isString(modifier))
+          ? tryNormalizeMongoObjectId(modifier)
+          : modifier;
         break;
 
       case '!=':
diff --git a/lib/private/machines/private/try-normalize-mongo-object-id.js b/lib/private/machines/private/try-normalize-mongo-object-id.js
new file mode 100644
index 000000000..585152b39
--- /dev/null
+++ b/lib/private/machines/private/try-normalize-mongo-object-id.js
@@ -0,0 +1,32 @@
+/**
+ * Module dependencies
+ */
+
+var normalizeMongoObjectId = require('./normalize-mongo-object-id');
+
+/**
+ * tryNormalizeMongoObjectId()
+ *
+ * Attempts to convert the given value to a Mongo ObjectId and return it. If the value
+ * simply cannot be interpreted as an ObjectId, it will be returned as-is.
+ *
+ * This function is equivalent to calling the `normalizeMongoObjectId()` function with
+ * tolerance for the `E_CANNOT_INTERPRET_AS_OBJECTID` error whereby the input value is
+ * yielded instead in such cases, allowing one to avoid inlining try-catch blocks.
+ *
+ * - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+ * @param value - Value to attempt normalization of.
+ * - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+ * @returns Returns the given value normalized to an `ObjectId` if possible. Otherwise
+ * returns the original value.
+ */
+module.exports = function tryNormalizeMongoObjectId(value) {
+  try {
+    return normalizeMongoObjectId(value);
+  } catch (e) {
+    switch (e.code) {
+      case 'E_CANNOT_INTERPRET_AS_OBJECTID': return value;
+      default: throw e;
+    }
+  }
+};