Re: Core: close pid file while writing it failed

2020-06-02 Thread Jim T
Hello,  Maxim Dounin.

How about below patch, could you help me review it? And I found the problem
still exists in release-1.19.0, or do I miss anything?

---

Hello!

As far as I understand it, `ngx_create_pidfile` is a function that works
independently. There is no action to close the pid file externally, so we
need to close the pid file when the writing it failed. There are also
reports here https://github.com/nginx/nginx/pull/52.

# HG changeset patch
# User Jinhua Tan <312841...@qq.com>
# Date 1590068494 -28800
#  Thu May 21 21:41:34 2020 +0800
# Node ID 6084ea4d9a4d2ae32f3fc4e2e3b9032ab0b71e30
# Parent  3242f98298975e556a7e87130611ce84799fe935
Core: close pid file while writing it failed.

diff -r 3242f9829897 -r 6084ea4d9a4d src/core/ngx_cycle.c
--- a/src/core/ngx_cycle.c  Wed May 20 12:24:05 2020 +0800
+++ b/src/core/ngx_cycle.c  Thu May 21 21:41:34 2020 +0800
@@ -1036,6 +1036,12 @@
 len = ngx_snprintf(pid, NGX_INT64_LEN + 2, "%P%N", ngx_pid) - pid;

 if (ngx_write_file(&file, pid, len, 0) == NGX_ERROR) {
+
+if (ngx_close_file(file.fd) == NGX_FILE_ERROR) {
+ngx_log_error(NGX_LOG_ALERT, log, ngx_errno,
+  ngx_close_file_n " \"%s\" failed",
file.name.data);
+}
+
 return NGX_ERROR;
 }
 }

Thank you!
___
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel

[njs] Fixed parsing of invalid binary expressions.

2020-06-02 Thread Alexander Borisov
details:   https://hg.nginx.org/njs/rev/ea1754b79e7a
branches:  
changeset: 1414:ea1754b79e7a
user:  Alexander Borisov 
date:  Tue Jun 02 17:53:28 2020 +0300
description:
Fixed parsing of invalid binary expressions.

The issue was introduced in 86f55a7dc4a4.

diffstat:

 src/njs_parser.c |  48 
 src/test/njs_unit_test.c |   3 +++
 2 files changed, 43 insertions(+), 8 deletions(-)

diffs (176 lines):

diff -r 8923d0751c28 -r ea1754b79e7a src/njs_parser.c
--- a/src/njs_parser.c  Tue Jun 02 17:53:27 2020 +0300
+++ b/src/njs_parser.c  Tue Jun 02 17:53:28 2020 +0300
@@ -3086,6 +3086,10 @@ njs_parser_expression_node(njs_parser_t 
 {
 njs_parser_node_t  *node;
 
+if (parser->ret != NJS_OK) {
+return njs_parser_failed(parser);
+}
+
 if (parser->target != NULL) {
 parser->target->right = parser->node;
 parser->target->right->dest = parser->target;
@@ -3107,7 +3111,7 @@ njs_parser_expression_node(njs_parser_t 
 node->left = parser->node;
 node->left->dest = node;
 
-return njs_parser_after(parser, current, node, 1, after);
+return njs_parser_after(parser, current, node, 0, after);
 }
 
 
@@ -3398,6 +3402,10 @@ njs_parser_exponentiation_expression_mat
 {
 njs_parser_node_t  *node;
 
+if (parser->ret != NJS_OK) {
+return njs_parser_failed(parser);
+}
+
 if (parser->target != NULL) {
 parser->target->right = parser->node;
 parser->target->right->dest = parser->target;
@@ -3423,7 +3431,7 @@ njs_parser_exponentiation_expression_mat
 
 njs_parser_next(parser, njs_parser_exponentiation_expression);
 
-return njs_parser_after(parser, current, node, 1,
+return njs_parser_after(parser, current, node, 0,
 njs_parser_exponentiation_expression_match);
 }
 
@@ -3449,6 +3457,10 @@ njs_parser_multiplicative_expression_mat
 njs_parser_node_t   *node;
 njs_vmcode_operation_t  operation;
 
+if (parser->ret != NJS_OK) {
+return njs_parser_failed(parser);
+}
+
 if (parser->target != NULL) {
 parser->target->right = parser->node;
 parser->target->right->dest = parser->target;
@@ -3485,7 +3497,7 @@ njs_parser_multiplicative_expression_mat
 
 njs_parser_next(parser, njs_parser_exponentiation_expression);
 
-return njs_parser_after(parser, current, node, 1,
+return njs_parser_after(parser, current, node, 0,
 njs_parser_multiplicative_expression_match);
 }
 
@@ -3511,6 +3523,10 @@ njs_parser_additive_expression_match(njs
 njs_parser_node_t   *node;
 njs_vmcode_operation_t  operation;
 
+if (parser->ret != NJS_OK) {
+return njs_parser_failed(parser);
+}
+
 if (parser->target != NULL) {
 parser->target->right = parser->node;
 parser->target->right->dest = parser->target;
@@ -3543,7 +3559,7 @@ njs_parser_additive_expression_match(njs
 
 njs_parser_next(parser, njs_parser_multiplicative_expression);
 
-return njs_parser_after(parser, current, node, 1,
+return njs_parser_after(parser, current, node, 0,
 njs_parser_additive_expression_match);
 }
 
@@ -3569,6 +3585,10 @@ njs_parser_shift_expression_match(njs_pa
 njs_parser_node_t   *node;
 njs_vmcode_operation_t  operation;
 
+if (parser->ret != NJS_OK) {
+return njs_parser_failed(parser);
+}
+
 if (parser->target != NULL) {
 parser->target->right = parser->node;
 parser->target->right->dest = parser->target;
@@ -3605,7 +3625,7 @@ njs_parser_shift_expression_match(njs_pa
 
 njs_parser_next(parser, njs_parser_additive_expression);
 
-return njs_parser_after(parser, current, node, 1,
+return njs_parser_after(parser, current, node, 0,
 njs_parser_shift_expression_match);
 }
 
@@ -3631,6 +3651,10 @@ njs_parser_relational_expression_match(n
 njs_parser_node_t   *node;
 njs_vmcode_operation_t  operation;
 
+if (parser->ret != NJS_OK) {
+return njs_parser_failed(parser);
+}
+
 if (parser->target != NULL) {
 parser->target->right = parser->node;
 parser->target->right->dest = parser->target;
@@ -3679,7 +3703,7 @@ njs_parser_relational_expression_match(n
 
 njs_parser_next(parser, njs_parser_shift_expression);
 
-return njs_parser_after(parser, current, node, 1,
+return njs_parser_after(parser, current, node, 0,
 njs_parser_relational_expression_match);
 }
 
@@ -3705,6 +3729,10 @@ njs_parser_equality_expression_match(njs
 njs_parser_node_t   *node;
 njs_vmcode_operation_t  operation;
 
+if (parser->ret != NJS_OK) {
+return njs_parser_failed(parser);
+}
+
 if (parser->target != NULL) {
 parser->target->right = parser->node;
 parser->target->right->dest = parser->target;
@@ -3745,7 +3773,7 @@ njs_parser_equality_expressi

[njs] Added necessary checks for obligatory grammar symbols.

2020-06-02 Thread Alexander Borisov
details:   https://hg.nginx.org/njs/rev/4117ec04714b
branches:  
changeset: 1415:4117ec04714b
user:  Alexander Borisov 
date:  Tue Jun 02 17:53:29 2020 +0300
description:
Added necessary checks for obligatory grammar symbols.

The issue was introduced in 86f55a7dc4a4.

diffstat:

 src/njs_parser.c |  14 +-
 src/test/njs_unit_test.c |  12 
 2 files changed, 25 insertions(+), 1 deletions(-)

diffs (67 lines):

diff -r ea1754b79e7a -r 4117ec04714b src/njs_parser.c
--- a/src/njs_parser.c  Tue Jun 02 17:53:28 2020 +0300
+++ b/src/njs_parser.c  Tue Jun 02 17:53:29 2020 +0300
@@ -888,6 +888,10 @@ static njs_int_t
 njs_parser_close_parenthesis(njs_parser_t *parser, njs_lexer_token_t *token,
 njs_queue_link_t *current)
 {
+if (parser->ret != NJS_OK) {
+return njs_parser_failed(parser);
+}
+
 if (token->type != NJS_TOKEN_CLOSE_PARENTHESIS) {
 return njs_parser_failed(parser);
 }
@@ -1590,6 +1594,10 @@ njs_parser_array_after(njs_parser_t *par
 {
 njs_int_t  ret;
 
+if (parser->ret != NJS_OK) {
+return njs_parser_failed(parser);
+}
+
 ret = njs_parser_array_item(parser, parser->target, parser->node);
 if (ret != NJS_OK) {
 return NJS_ERROR;
@@ -1617,7 +1625,7 @@ static njs_int_t
 njs_parser_array_spread_element(njs_parser_t *parser, njs_lexer_token_t *token,
 njs_queue_link_t *current)
 {
-if (token->type != NJS_TOKEN_CLOSE_BRACKET) {
+if (parser->ret != NJS_OK || token->type != NJS_TOKEN_CLOSE_BRACKET) {
 return njs_parser_failed(parser);
 }
 
@@ -6262,6 +6270,10 @@ njs_parser_catch_finally(njs_parser_t *p
 {
 njs_parser_node_t  *node;
 
+if (parser->ret != NJS_OK) {
+return njs_parser_failed(parser);
+}
+
 node = njs_parser_node_new(parser, NJS_TOKEN_FINALLY);
 if (node == NULL) {
 return NJS_ERROR;
diff -r ea1754b79e7a -r 4117ec04714b src/test/njs_unit_test.c
--- a/src/test/njs_unit_test.c  Tue Jun 02 17:53:28 2020 +0300
+++ b/src/test/njs_unit_test.c  Tue Jun 02 17:53:29 2020 +0300
@@ -16663,6 +16663,18 @@ static njs_unit_test_t  njs_test[] =
 
 { njs_str("{name; {/ / /}"),
   njs_str("SyntaxError: Unexpected token \"}\" in 1") },
+
+{ njs_str("[(]"),
+  njs_str("SyntaxError: Unexpected token \"]\" in 1") },
+
+{ njs_str("[...]"),
+  njs_str("SyntaxError: Unexpected token \"]\" in 1") },
+
+{ njs_str("switch () {}"),
+  njs_str("SyntaxError: Unexpected token \")\" in 1") },
+
+{ njs_str("switch ([(]) {}"),
+  njs_str("SyntaxError: Unexpected token \"]\" in 1") },
 };
 
 
___
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel


[njs] Fixed checking return value in primary expression.

2020-06-02 Thread Alexander Borisov
details:   https://hg.nginx.org/njs/rev/8923d0751c28
branches:  
changeset: 1413:8923d0751c28
user:  Alexander Borisov 
date:  Tue Jun 02 17:53:27 2020 +0300
description:
Fixed checking return value in primary expression.

The issue was introduced in 86f55a7dc4a4.

diffstat:

 src/njs_parser.c |  10 +++---
 src/test/njs_unit_test.c |   9 -
 2 files changed, 15 insertions(+), 4 deletions(-)

diffs (60 lines):

diff -r ff39edb94acf -r 8923d0751c28 src/njs_parser.c
--- a/src/njs_parser.c  Mon Jun 01 18:09:29 2020 +0300
+++ b/src/njs_parser.c  Tue Jun 02 17:53:27 2020 +0300
@@ -994,7 +994,7 @@ njs_parser_primary_expression_test(njs_p
 
 ret = njs_parser_escape_string_create(parser, token, &node->u.value);
 if (ret != NJS_TOKEN_STRING) {
-return NJS_DONE;
+return NJS_ERROR;
 }
 
 parser->node = node;
@@ -1005,7 +1005,7 @@ njs_parser_primary_expression_test(njs_p
 
 njs_parser_syntax_error(parser, "Unterminated string \"%V\"",
 &token->text);
-return NJS_DONE;
+return NJS_ERROR;
 
 /* ArrayLiteral */
 case NJS_TOKEN_OPEN_BRACKET:
@@ -1086,7 +1086,7 @@ njs_parser_primary_expression_test(njs_p
 
 ret = njs_parser_regexp_literal(parser, token, current);
 if (ret != NJS_OK) {
-return NJS_DONE;
+return NJS_ERROR;
 }
 
 goto done;
@@ -2210,6 +2210,10 @@ njs_parser_member_expression(njs_parser_
 return NJS_OK;
 }
 
+if (njs_is_error(&parser->vm->retval)) {
+return NJS_DONE;
+}
+
 return ret;
 }
 
diff -r ff39edb94acf -r 8923d0751c28 src/test/njs_unit_test.c
--- a/src/test/njs_unit_test.c  Mon Jun 01 18:09:29 2020 +0300
+++ b/src/test/njs_unit_test.c  Tue Jun 02 17:53:27 2020 +0300
@@ -16652,7 +16652,14 @@ static njs_unit_test_t  njs_test[] =
   "function foo() {}"),
   njs_str("undefined") },
 
-
+{ njs_str("new\""),
+  njs_str("SyntaxError: Unterminated string \"\"\" in 1") },
+
+{ njs_str("new\"\\U"),
+  njs_str("SyntaxError: Unterminated string \"\"\\U\" in 1") },
+
+{ njs_str("new/la"),
+  njs_str("SyntaxError: Unterminated RegExp \"/la\" in 1") },
 };
 
 
___
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel


[njs] Fixed endless loop in Array.prototype.sort().

2020-06-02 Thread Dmitry Volyntsev
details:   https://hg.nginx.org/njs/rev/1ce9a0895630
branches:  
changeset: 1417:1ce9a0895630
user:  Dmitry Volyntsev 
date:  Tue Jun 02 14:59:30 2020 +
description:
Fixed endless loop in Array.prototype.sort().

With non-consistent comparison function.

The issue was introduced in 1d0825906438.

diffstat:

 src/njs_utils.c  |  7 +++
 src/test/njs_unit_test.c |  3 +++
 2 files changed, 6 insertions(+), 4 deletions(-)

diffs (37 lines):

diff -r 13e36f31e331 -r 1ce9a0895630 src/njs_utils.c
--- a/src/njs_utils.c   Tue Jun 02 14:59:27 2020 +
+++ b/src/njs_utils.c   Tue Jun 02 14:59:30 2020 +
@@ -241,7 +241,7 @@ void
 njs_qsort(void *arr, size_t n, size_t esize, njs_sort_cmp_t cmp, void *ctx)
 {
 intr;
-u_char *base, *lt, *gt, *p, *end;
+u_char *base, *lt, *gt, *c, *p, *end;
 njs_uint_t m4;
 njs_swap_t swap;
 njs_qsort_state_t  stack[NJS_MAX_DEPTH], *sp;
@@ -321,9 +321,8 @@ njs_qsort(void *arr, size_t n, size_t es
 /* Insertion sort. */
 
 for (p = base + esize; p < end; p += esize) {
-while (p > base && cmp(p, p - esize, ctx) < 0) {
-swap(p, p - esize, esize);
-p -= esize;
+for (c = p; c > base && cmp(c, c - esize, ctx) < 0; c -= esize) {
+swap(c, c - esize, esize);
 }
 }
 }
diff -r 13e36f31e331 -r 1ce9a0895630 src/test/njs_unit_test.c
--- a/src/test/njs_unit_test.c  Tue Jun 02 14:59:27 2020 +
+++ b/src/test/njs_unit_test.c  Tue Jun 02 14:59:30 2020 +
@@ -6101,6 +6101,9 @@ static njs_unit_test_t  njs_test[] =
   "a.sort((a, b) => b.r - a.r).map(v=>v.n).join('')"),
   njs_str("BDEAC") },
 
+{ njs_str("[1,2,3].sort(()=>-1)"),
+  njs_str("3,2,1") },
+
 { njs_str("var count = 0;"
   "[4,3,2,1].sort(function(x, y) { if (count++ == 2) {throw 
Error('Oops'); }; return x - y })"),
   njs_str("Error: Oops") },
___
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel


[njs] Fixed typo introduced in db77713e0536.

2020-06-02 Thread Dmitry Volyntsev
details:   https://hg.nginx.org/njs/rev/13e36f31e331
branches:  
changeset: 1416:13e36f31e331
user:  Dmitry Volyntsev 
date:  Tue Jun 02 14:59:27 2020 +
description:
Fixed typo introduced in db77713e0536.

diffstat:

 src/njs_array.c  |  2 +-
 src/test/njs_unit_test.c |  9 +
 2 files changed, 10 insertions(+), 1 deletions(-)

diffs (31 lines):

diff -r 4117ec04714b -r 13e36f31e331 src/njs_array.c
--- a/src/njs_array.c   Tue Jun 02 17:53:29 2020 +0300
+++ b/src/njs_array.c   Tue Jun 02 14:59:27 2020 +
@@ -1361,7 +1361,7 @@ njs_array_prototype_reverse(njs_vm_t *vm
 array->start[l] = hvalue;
 
 } else {
-array->start[h] = njs_value_invalid;
+array->start[l] = njs_value_invalid;
 }
 
 } else if (hret == NJS_OK) {
diff -r 4117ec04714b -r 13e36f31e331 src/test/njs_unit_test.c
--- a/src/test/njs_unit_test.c  Tue Jun 02 17:53:29 2020 +0300
+++ b/src/test/njs_unit_test.c  Tue Jun 02 14:59:27 2020 +
@@ -4504,6 +4504,15 @@ static njs_unit_test_t  njs_test[] =
 { njs_str("var a = [1,2,3,4]; a.reverse()"),
   njs_str("4,3,2,1") },
 
+{ njs_str("[1,2,3,,,].reverse()"),
+  njs_str(",,3,2,1") },
+
+{ njs_str("[,2,3,,,].reverse()"),
+  njs_str(",,3,2,") },
+
+{ njs_str("[,,,3,2,1].reverse()"),
+  njs_str("1,2,3,,,") },
+
 { njs_str("var o = {1:true, 2:'', length:-2}; 
Array.prototype.reverse.call(o) === o"),
   njs_str("true") },
 
___
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel


[njs] Ensuring Array.prototype.sort() stability.

2020-06-02 Thread Dmitry Volyntsev
details:   https://hg.nginx.org/njs/rev/c2fa0f18e58d
branches:  
changeset: 1418:c2fa0f18e58d
user:  Dmitry Volyntsev 
date:  Tue Jun 02 12:08:04 2020 +
description:
Ensuring Array.prototype.sort() stability.

For fast arrays with empty or undefined elements.

This closes #312 issue on Github.

diffstat:

 src/njs_array.c  |  69 ---
 src/test/njs_unit_test.c |   6 
 2 files changed, 41 insertions(+), 34 deletions(-)

diffs (128 lines):

diff -r 1ce9a0895630 -r c2fa0f18e58d src/njs_array.c
--- a/src/njs_array.c   Tue Jun 02 14:59:30 2020 +
+++ b/src/njs_array.c   Tue Jun 02 12:08:04 2020 +
@@ -3216,8 +3216,8 @@ static njs_int_t
 njs_array_prototype_sort(njs_vm_t *vm, njs_value_t *args, njs_uint_t nargs,
 njs_index_t unused)
 {
-int64_ti, und, inv, len, nlen, length;
-njs_int_t  ret;
+int64_ti, und, len, nlen, length;
+njs_int_t  ret, fast_path;
 njs_array_t*array;
 njs_value_t*this, *comparefn, *start, *strings;
 njs_array_sort_ctx_t   ctx;
@@ -3260,50 +3260,46 @@ njs_array_prototype_sort(njs_vm_t *vm, n
 ctx.strings.pointer = 0;
 ctx.exception = 0;
 
-if (njs_fast_path(njs_is_fast_array(this))) {
+fast_path = njs_is_fast_array(this);
+
+if (njs_fast_path(fast_path)) {
 array = njs_array(this);
 start = array->start;
 
-/**
- * Moving undefined and invalid elements to the end.
- * x x x | undefineds | invalids
- */
+slots = njs_mp_alloc(vm->mem_pool,
+ sizeof(njs_array_sort_slot_t) * length);
+if (njs_slow_path(slots == NULL)) {
+return NJS_ERROR;
+}
 
 und = 0;
-inv = length;
-
-for (i = length - 1; i >= 0; i--) {
-if (njs_is_undefined(&start[i])) {
-start[i] = start[inv - und - 1];
-start[inv - und - 1] = njs_value_undefined;
+p = slots;
+
+for (i = 0; i < length; i++) {
+if (njs_slow_path(!njs_is_valid(&start[i]))) {
+fast_path = 0;
+njs_mp_free(vm->mem_pool, slots);
+slots = NULL;
+goto slow_path;
+}
+
+if (njs_slow_path(njs_is_undefined(&start[i]))) {
 und++;
 continue;
 }
 
-if (!njs_is_valid(&start[i])) {
-start[i] = start[inv - und - 1];
-start[inv - und - 1] = njs_value_undefined;
-start[inv - 1] = njs_value_invalid;
-inv--;
-continue;
-}
+p->value = start[i];
+p->pos = i;
+p->str = NULL;
+p++;
 }
 
-len = inv - und;
-
-slots = njs_mp_alloc(vm->mem_pool,
- sizeof(njs_array_sort_slot_t) * len);
-if (njs_slow_path(slots == NULL)) {
-return NJS_ERROR;
-}
-
-for (i = 0; i < len; i++) {
-slots[i].value = start[i];
-slots[i].pos = i;
-slots[i].str = NULL;
-}
+len = p - slots;
 
 } else {
+
+slow_path:
+
 und = 0;
 p = NULL;
 end = NULL;
@@ -3369,13 +3365,18 @@ njs_array_prototype_sort(njs_vm_t *vm, n
 goto exception;
 }
 
-if (njs_fast_path(njs_is_fast_array(this))) {
+if (njs_fast_path(fast_path)) {
 array = njs_array(this);
 start = array->start;
+
 for (i = 0; i < len; i++) {
 start[i] = slots[i].value;
 }
 
+for (i = len; und-- > 0; i++) {
+start[i] = njs_value_undefined;
+}
+
 } else {
 for (i = 0; i < len; i++) {
 if (slots[i].pos != i) {
diff -r 1ce9a0895630 -r c2fa0f18e58d src/test/njs_unit_test.c
--- a/src/test/njs_unit_test.c  Tue Jun 02 14:59:30 2020 +
+++ b/src/test/njs_unit_test.c  Tue Jun 02 12:08:04 2020 +
@@ -6104,6 +6104,12 @@ static njs_unit_test_t  njs_test[] =
 { njs_str("[1,2,3].sort(()=>-1)"),
   njs_str("3,2,1") },
 
+{ njs_str("njs.dump([undefined,1,2,3].sort(()=>0))"),
+  njs_str("[1,2,3,undefined]") },
+
+{ njs_str("njs.dump([1,,2,3].sort(()=>0))"),
+  njs_str("[1,2,3,]") },
+
 { njs_str("var count = 0;"
   "[4,3,2,1].sort(function(x, y) { if (count++ == 2) {throw 
Error('Oops'); }; return x - y })"),
   njs_str("Error: Oops") },
___
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel