Njs: weird behavior about query string

[keliq]

why name is not in r.args?

nginx.js

function test(r) {
  var obj = {}
  obj['name in r.args'] = 'name' in r.args
  obj['name in Object.keys(r.args)'] = Object.keys(r.args).includes('name')
  r.return(200, JSON.stringify(obj))
}

server.conf

js_import nginx.js;

server {
  listen 8003;
  location /test {
    js_content nginx.test;
  }
}

$ curl 'localhost:8003/test?age=12&name'
{"name in r.args":false,"name in Object.keys(r.args)":true}

why name is not in r.args?

why querystring convert undefined to 'undefined'?

test.js

var querystring = require('querystring')
var obj = { name: undefined, age: 12 }
console.log(querystring.stringify(obj))

$ node test.js 
name=&age=12
$ njs test.js
name=undefined&age=12

how can I get raw query string(not r.args object) ?

$ curl 'localhost:8003/test?age=12&name'

how to get 'age=12&name' in njs ?

[xeioex]

@keliq

how can I get raw query string(not r.args object) ?

Use r.variables.args.

why querystring convert undefined to 'undefined'?

looks like a bug, @lexborisov please take a look.

why name is not in r.args?

nginx treats an empty string as the absence of the string.

[keliq]

@xeioex Thanks for your explanation. But if name is absence, why it appears in Object.keys(r.args)?

function test(r) {
  r.return(200, Object.keys(r.args))
}

$ curl 'localhost:8003/test?age=12&name'
age,name

In JavaScript, absence means no such key in object. njs behaves the same as node in cli:

test.js

var obj = { name: undefined, age: 12 }
console.log('name' in obj, Object.keys(obj).includes('name'))

$ node test.js
true true
$ njs test.js
true true

[drsm]

@xeioex
@lexborisov

why querystring convert undefined to 'undefined'?

looks like a bug, @lexborisov please take a look.

It conforms URLSearchParams interface :)

> var sp = new URLSearchParams()
undefined
> sp.set('a', undefined)
undefined
> sp
URLSearchParams { 'a' => 'undefined' }
> var sp = new URLSearchParams({ a: undefined })
undefined
> sp
URLSearchParams { 'a' => 'undefined' }

[drsm]

@xeioex
@lexborisov

I'm going to fix this, and some other bugs and incompatibilities, like

>> qs.stringify([])
'length=0'

[lexborisov]

@drsm

I think to do this after the Promises, but if you wish, then the task is your.

[drsm]

@lexborisov
@xeioex

Fixed according to the docs:

It serializes the following types of values passed in obj: string | number | boolean | string[] | number[] | boolean[]. Any other input values will be coerced to empty strings.

Here is the patch, PTAL:

# HG changeset patch
# User Artem S. Povalyukhin <[email protected]>
# Date 1604174403 -10800
#      Sat Oct 31 23:00:03 2020 +0300
# Node ID 1c527668b65f73eeb0d34ed5c6e35cacdb6615de
# Parent  fa3ffb3a159e0f8fa804f636e473101778893fd4
Fixed querystring.stringify().

diff -r fa3ffb3a159e -r 1c527668b65f src/njs_query_string.c
--- a/src/njs_query_string.c    Thu Oct 29 12:51:21 2020 +0000
+++ b/src/njs_query_string.c    Sat Oct 31 23:00:03 2020 +0300
@@ -549,8 +549,8 @@ njs_inline njs_int_t
 njs_query_string_push(njs_vm_t *vm, njs_chb_t *chain, njs_value_t *key,
     njs_value_t *value, njs_string_prop_t *eq, njs_function_t *encoder)
 {
+    double     num;
     njs_int_t  ret, length;
-    njs_str_t  str;

     length = 0;

@@ -561,25 +561,35 @@ njs_query_string_push(njs_vm_t *vm, njs_

     length += ret;

-    if (!njs_is_string(value)) {
-        ret = njs_value_to_string(vm, value, value);
+    njs_chb_append(chain, eq->start, eq->size);
+    length += eq->length;
+
+    switch (value->type) {
+    case NJS_NUMBER:
+        num = njs_number(value);
+        if (njs_slow_path(isnan(num) || isinf(num))) {
+            break;
+        }
+        /* Fall through */
+
+    case NJS_BOOLEAN:
+        ret = njs_primitive_value_to_string(vm, value, value);
         if (njs_slow_path(ret != NJS_OK)) {
             return NJS_ERROR;
         }
-    }
-
-    njs_string_get(value, &str);
+        /* Fall through */

-    if (str.length > 0) {
-        njs_chb_append(chain, eq->start, eq->size);
-        length += eq->length;
-
+    case NJS_STRING:
         ret = njs_query_string_encoder_call(vm, chain, encoder, value);
         if (njs_slow_path(ret < 0)) {
             return NJS_ERROR;
         }

         length += ret;
+        break;
+
+    default:
+        break;
     }

     return length;
@@ -673,7 +683,7 @@ njs_query_string_stringify(njs_vm_t *vm,
     njs_chb_init(&chain, vm->mem_pool);

     keys = njs_value_own_enumerate(vm, object, NJS_ENUM_KEYS, NJS_ENUM_STRING,
-                                   1);
+                                   0);
     if (njs_slow_path(keys == NULL)) {
         return NJS_ERROR;
     }
@@ -692,7 +702,7 @@ njs_query_string_stringify(njs_vm_t *vm,
                 array = njs_array(&value);

                 for (i = 0; i < array->length; i++) {
-                    if (i != 0) {
+                    if (chain.last != NULL || i != 0) {
                         njs_chb_append(&chain, sep.start, sep.size);
                         length += sep.length;
                     }
@@ -721,7 +731,7 @@ njs_query_string_stringify(njs_vm_t *vm,
                     goto failed;
                 }

-                if (i != 0) {
+                if (chain.last != NULL || i != 0) {
                     njs_chb_append(&chain, sep.start, sep.size);
                     length += sep.length;
                 }
diff -r fa3ffb3a159e -r 1c527668b65f src/test/njs_unit_test.c
--- a/src/test/njs_unit_test.c  Thu Oct 29 12:51:21 2020 +0000
+++ b/src/test/njs_unit_test.c  Sat Oct 31 23:00:03 2020 +0300
@@ -18233,7 +18233,45 @@ static njs_unit_test_t  njs_test[] =

     { njs_str("var qs = require('querystring');"
               "qs.stringify({X:{toString(){return 3}}})"),
-      njs_str("X=3") },
+      njs_str("X=") },
+
+    { njs_str("var qs = require('querystring');"
+              "qs.stringify({ name: undefined, age: 12 })"),
+      njs_str("name=&age=12") },
+
+    { njs_str("var qs = require('querystring');"
+              "qs.stringify(Object.create({ name: undefined, age: 12 }))"),
+      njs_str("") },
+
+    { njs_str("var qs = require('querystring');"
+              "qs.stringify([])"),
+      njs_str("") },
+
+    { njs_str("var qs = require('querystring');"
+              "qs.stringify(['','',''])"),
+      njs_str("0=&1=&2=") },
+
+    { njs_str("var qs = require('querystring');"
+              "qs.stringify([undefined, null, Symbol(), Object(0), Object('test'), Object(false),,,])"),
+      njs_str("0=&1=&2=&3=&4=&5=") },
+
+#if 0
+    { njs_str("var qs = require('querystring');"
+              "qs.stringify([NaN, Infinity, -Infinity, 2**69, 2**70])"),
+      njs_str("0=&1=&2=&3=590295810358705700000&4=1.1805916207174113e%2B21") },
+#else
+    { njs_str("var qs = require('querystring');"
+              "qs.stringify([NaN, Infinity, -Infinity, 2**69, 2**70])"),
+      njs_str("0=&1=&2=&3=590295810358705700000&4=1.1805916207174114e%2B21") },
+#endif
+
+    { njs_str("var qs = require('querystring');"
+              "qs.stringify([[1,2,3],[4,5,6]])"),
+      njs_str("0=1&0=2&0=3&1=4&1=5&1=6") },
+
+    { njs_str("var qs = require('querystring');"
+              "qs.stringify([[1,,,],[2,,,]])"),
+      njs_str("0=1&0=&0=&1=2&1=&1=") },

     { njs_str("var qs = require('querystring');"
               "qs.escape('abcααααdef')"),

[drsm]

I've updated the patch, 'cause NaN and Infinity were forgotten.

[xeioex]

@drsm Thanks, committed.