From 7986c6d769dfeb19056224f0f087b85c668ba920 Mon Sep 17 00:00:00 2001
From: Robert Kawecki <robert@geo-com.pl>
Date: Wed, 21 Dec 2016 11:40:01 +0100
Subject: [PATCH] Disable coercion of null values to string:null in
 Query#toString

This also includes a refactoring of the query module's `encodeValue` function for clarity and easier modification later, if necessary. Comments included.

For persvr/rql#76
---
 query.js | 69 ++++++++++++++++++++++++++++++++++++--------------------
 1 file changed, 44 insertions(+), 25 deletions(-)

diff --git a/query.js b/query.js
index 24718de..5ee71fe 100644
--- a/query.js
+++ b/query.js
@@ -76,37 +76,56 @@ function encodeString(s) {
 		return s;
 }
 
+/**
+ * Encode a value of a part's argument for use in an RQL string.
+ * @param {*} val - The value to encode.
+ * @returns {string} The encoded value, URI-safe.
+ */
 exports.encodeValue = function(val) {
-		var encoded;
-		if (val === null) val = 'null';
+		// Remember whether any encoding took place. The function force-encodes
+		//  the value as a plain string at the end as a last resort.
+		var encoded = false;
+
+		// Try encoding the value. If it throws, go to our failsafe.
 		try {
-			if (val !== parser.converters["default"]('' + (
-					val.toISOString && val.toISOString() || val.toString()
-			))) {
-					var type = typeof val;
-					if(val instanceof RegExp){
-						// TODO: control whether to we want simpler glob() style
-						val = val.toString();
-						var i = val.lastIndexOf('/');
-						type = val.substring(i).indexOf('i') >= 0 ? "re" : "RE";
-						val = encodeString(val.substring(1, i));
-						encoded = true;
-					}
-					if(type === "object"){
-							type = "epoch";
-							val = val.getTime();
-							encoded = true;
-					}
-					if(type === "string") {
-							val = encodeString(val);
-							encoded = true;
-					}
-					val = [type, val].join(":");
+			// Check whether casting the value to a string and parsing it would
+			//  result in the input value (i.e. if the value can be left prefix-less).
+			var stringVal = '' + ((val && val.toISOString) ? val.toISOString() : val);
+			var parsedStringVal = parser.converters["default"](stringVal);
+			if (val === parsedStringVal) {
+				return encodeString(String(val));
+			}
+			// Now, only non-trivial cases are left:
+			var type = typeof val;
+			if (val instanceof RegExp) {
+				// TODO: control whether to we want simpler glob() style
+				val = val.toString();
+				var i = val.lastIndexOf('/');
+				type = val.substring(i).indexOf('i') >= 0 ? "re" : "RE";
+				val = encodeString(val.substring(1, i));
+				encoded = true;
 			}
+			if (type === "object") {
+				// Assume it's a date - it's the only other object type we support:
+				type = "epoch";
+				val = val.getTime();
+				encoded = true;
+			}
+			if (type === "string") {
+				// It's a string, but it doesn't parse to its own exact value.
+				// This happens when the input is a String('null') for example -
+				//  the auto-converter parses null to type null, so we need a prefix.
+				val = encodeString(val);
+				encoded = true;
+			}
+			val = [type, val].join(":");
 		} catch (conversionError) {
 			// The conversion necessity check has failed and we have not encoded the value. The next conditional block will take care of it.
 		}
-		if (!encoded && typeof val === "string") val = encodeString(val);
+		if (!encoded && typeof val === "string") {
+			val = encodeString(val);
+		}
+		val = String(val);
 		return val;
 };