Re: [PATCH] HTTP/2: add fast-path for HTTP/2 requests without request body

2017-03-28 Thread Valentin V. Bartenev 
On Sunday 26 March 2017 01:41:23 Piotr Sikora via nginx-devel wrote:
> # HG changeset patch
> # User Piotr Sikora 
> # Date 1490516712 25200
> #  Sun Mar 26 01:25:12 2017 -0700
> # Node ID f9fd6a8babce9f57f038d304dc1eef82284dde8b
> # Parent  22be63bf21edaa1b8ea916c7d8cd4e5fe4892061
> HTTP/2: add fast-path for HTTP/2 requests without request body.
> 
> Signed-off-by: Piotr Sikora 
> 
> diff -r 22be63bf21ed -r f9fd6a8babce src/http/v2/ngx_http_v2.c
> --- a/src/http/v2/ngx_http_v2.c
> +++ b/src/http/v2/ngx_http_v2.c
> @@ -3494,7 +3494,9 @@ ngx_http_v2_read_request_body(ngx_http_r
>  
>  stream = r->stream;
>  
> -if (stream->skip_data) {
> +if (stream->skip_data
> +|| (stream->in_closed && stream->preread == NULL))
> +{
>  r->request_body_no_buffering = 0;
>  post_handler(r);
>  return NGX_OK;

That doesn't look like a correct patch to me as it changes behavior
of ngx_http_read_client_request_body() specifically for HTTP/2 case.

Note, that in case of HTTP/1.x it always allocates r->request_body
for the main request unless r->discard_body is set.

Even if it doesn't break something in nginx at the first glance,
there's always a chance that some 3rd-party modules can depend
on this.

Anyway, this change should be made either for both protocols or
for none of them.

  wbr, Valentin V. Bartenev

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

Re: [PATCH] HTTP/2: reject HTTP/2 requests without ":scheme" pseudo-header

2017-03-28 Thread Valentin V. Bartenev 
On Tuesday 28 March 2017 05:40:44 Piotr Sikora via nginx-devel wrote:
> Hey Valentin,
> 
> > This part of patch can be added to the style one:
> >
> >   "HTTP/2: style and typos."
> 
> Assuming that this patch gets dropped or even if it gets committed?
> 

IMHO it's not a good idea to combine style fixes with behavior changes.
Behavior changing commits are occasionally reverted.


> > Is there any practical reason to force this restriction?
> 
> None, other than following RFC and providing early detection of broken 
> clients.
> 
> Also, it looks that you have a test for it marked as TODO:
> http://hg.nginx.org/nginx-tests/annotate/a6abbfed42c0/h2_headers.t#l972
> 

That's why it's still TODO (in other words intentionally skipped).
We discussed it with QA and decided to be more tolerant here.

  wbr, Valentin V. Bartenev

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

Re: [PATCH] HTTP/2: add logging of RST_STREAM frames with NO_ERROR code

2017-03-28 Thread Valentin V. Bartenev 
On Sunday 26 March 2017 01:41:20 Piotr Sikora via nginx-devel wrote:
> # HG changeset patch
> # User Piotr Sikora 
> # Date 1490516710 25200
> #  Sun Mar 26 01:25:10 2017 -0700
> # Node ID 31dfcde3ea2ccf1a2dbd2601ebe8f4306887fc0f
> # Parent  22be63bf21edaa1b8ea916c7d8cd4e5fe4892061
> HTTP/2: add logging of RST_STREAM frames with NO_ERROR code.
> 
> Signed-off-by: Piotr Sikora 
> 
> diff -r 22be63bf21ed -r 31dfcde3ea2c src/http/v2/ngx_http_v2.c
> --- a/src/http/v2/ngx_http_v2.c
> +++ b/src/http/v2/ngx_http_v2.c
> @@ -1916,6 +1916,11 @@ ngx_http_v2_state_rst_stream(ngx_http_v2
>  
>  switch (status) {
>  
> +case NGX_HTTP_V2_NO_ERROR:
> +ngx_log_error(NGX_LOG_INFO, fc->log, 0,
> +  "client closed stream %ui", h2c->state.sid);
> +break;
> +
>  case NGX_HTTP_V2_CANCEL:
>  ngx_log_error(NGX_LOG_INFO, fc->log, 0,
>"client canceled stream %ui", h2c->state.sid);

Currently such frames are logged with message:

  "client terminated stream %ui with status 0"

Could you explain why NO_ERROR needs special handling here?

I haven't found in RFC mentions about cases in which clients can
send RST_STREAM with NO_ERROR.

  wbr, Valentin V. Bartenev

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

Re: [PATCH] HTTP/2: emit PROTOCOL_ERROR on invalid WINDOW_UPDATE increments

2017-03-28 Thread Valentin V. Bartenev 
On Tuesday 28 March 2017 03:52:07 Piotr Sikora via nginx-devel wrote:
> # HG changeset patch
> # User Piotr Sikora 
> # Date 1490516706 25200
> #  Sun Mar 26 01:25:06 2017 -0700
> # Node ID ccb36c87291e38d1a63224d143cbeaa4ee4a4287
> # Parent  22be63bf21edaa1b8ea916c7d8cd4e5fe4892061
> HTTP/2: emit PROTOCOL_ERROR on invalid WINDOW_UPDATE increments.
> 
> Signed-off-by: Piotr Sikora 
> 
[..]

Here's my version of the patch.
It's made similar to ngx_http_v2_state_priority().

# HG changeset patch
# User Valentin Bartenev 
# Date 1490721720 -10800
#  Tue Mar 28 20:22:00 2017 +0300
# Node ID 3e798c552767068056c0251d7b6bd9ffd2587fc0
# Parent  ce37362a7a70c0acd14ba01c8c2223b366b62233
HTTP/2: rejecting zero WINDOW_UPDATE with PROTOCOL_ERROR.

It's required by RFC 7540.  While there is no real harm from such frames,
that should help to detect broken clients.

Prodded by Piotr Sikora.

diff -r ce37362a7a70 -r 3e798c552767 src/http/v2/ngx_http_v2.c
--- a/src/http/v2/ngx_http_v2.c Tue Mar 28 18:15:42 2017 +0300
+++ b/src/http/v2/ngx_http_v2.c Tue Mar 28 20:22:00 2017 +0300
@@ -2161,6 +2161,40 @@ ngx_http_v2_state_window_update(ngx_http
"http2 WINDOW_UPDATE frame sid:%ui window:%uz",
h2c->state.sid, window);
 
+if (window == 0) {
+ngx_log_error(NGX_LOG_INFO, h2c->connection->log, 0,
+  "client sent WINDOW_UPDATE frame "
+  "with incorrect window increment 0");
+
+if (h2c->state.sid == 0) {
+return ngx_http_v2_connection_error(h2c,
+NGX_HTTP_V2_PROTOCOL_ERROR);
+}
+
+node = ngx_http_v2_get_node_by_id(h2c, h2c->state.sid, 0);
+
+if (node && node->stream) {
+if (ngx_http_v2_terminate_stream(h2c, node->stream,
+ NGX_HTTP_V2_PROTOCOL_ERROR)
+== NGX_ERROR)
+{
+return ngx_http_v2_connection_error(h2c,
+
NGX_HTTP_V2_INTERNAL_ERROR);
+}
+
+} else {
+if (ngx_http_v2_send_rst_stream(h2c, h2c->state.sid,
+NGX_HTTP_V2_PROTOCOL_ERROR)
+== NGX_ERROR)
+{
+return ngx_http_v2_connection_error(h2c,
+
NGX_HTTP_V2_INTERNAL_ERROR);
+}
+}
+
+return ngx_http_v2_state_complete(h2c, pos, end);
+}
+
 if (h2c->state.sid) {
 node = ngx_http_v2_get_node_by_id(h2c, h2c->state.sid, 0);
 

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

[nginx] Copy filter: wake up subrequests after aio operations.

2017-03-28 Thread Maxim Dounin 
details:   http://hg.nginx.org/nginx/rev/ce37362a7a70
branches:  
changeset: 6951:ce37362a7a70
user:  Maxim Dounin 
date:  Tue Mar 28 18:15:42 2017 +0300
description:
Copy filter: wake up subrequests after aio operations.

Previously, connection write handler was called, resulting in wake up
of the active subrequest.  This change makes it possible to read data
in non-active subrequests as well.  For example, this allows SSI to
process instructions in non-active subrequests earlier and start
additional subrequests if needed, reducing overall response time.

diffstat:

 src/http/ngx_http_copy_filter_module.c |  32 ++--
 1 files changed, 30 insertions(+), 2 deletions(-)

diffs (64 lines):

diff --git a/src/http/ngx_http_copy_filter_module.c 
b/src/http/ngx_http_copy_filter_module.c
--- a/src/http/ngx_http_copy_filter_module.c
+++ b/src/http/ngx_http_copy_filter_module.c
@@ -187,15 +187,24 @@ static void
 ngx_http_copy_aio_event_handler(ngx_event_t *ev)
 {
 ngx_event_aio_t *aio;
+ngx_connection_t*c;
 ngx_http_request_t  *r;
 
 aio = ev->data;
 r = aio->data;
+c = r->connection;
+
+ngx_http_set_log_request(c->log, r);
+
+ngx_log_debug2(NGX_LOG_DEBUG_HTTP, c->log, 0,
+   "http aio: \"%V?%V\"", >uri, >args);
 
 r->main->blocked--;
 r->aio = 0;
 
-r->connection->write->handler(r->connection->write);
+r->write_event_handler(r);
+
+ngx_http_run_posted_requests(c);
 }
 
 
@@ -300,14 +309,33 @@ ngx_http_copy_thread_handler(ngx_thread_
 static void
 ngx_http_copy_thread_event_handler(ngx_event_t *ev)
 {
+ngx_connection_t*c;
 ngx_http_request_t  *r;
 
 r = ev->data;
+c = r->connection;
+
+ngx_http_set_log_request(c->log, r);
+
+ngx_log_debug2(NGX_LOG_DEBUG_HTTP, c->log, 0,
+   "http thread: \"%V?%V\"", >uri, >args);
 
 r->main->blocked--;
 r->aio = 0;
 
-r->connection->write->handler(r->connection->write);
+if (r->done) {
+/*
+ * trigger connection event handler if the subrequest was
+ * already finalized; this can happen if the handler is used
+ * for sendfile() in threads
+ */
+
+c->write->handler(c->write);
+
+} else {
+r->write_event_handler(r);
+ngx_http_run_posted_requests(c);
+}
 }
 
 #endif
___
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel

[nginx] Threads: fixed request hang with aio_write and subrequests.

2017-03-28 Thread Maxim Dounin 
details:   http://hg.nginx.org/nginx/rev/4cb4ffe06785
branches:  
changeset: 6950:4cb4ffe06785
user:  Maxim Dounin 
date:  Tue Mar 28 18:15:41 2017 +0300
description:
Threads: fixed request hang with aio_write and subrequests.

If the subrequest is already finalized, the handler set with aio_write
may still be used by sendfile in threads when using range requests
(see also e4c1f5b32868, and the original note in 9fd738b85fad).  Calling
already finalized subrequest's r->write_event_handler in practice
results in request hang in some cases.

Fix is to trigger connection event handler if the subrequest was already
finalized.

diffstat:

 src/http/ngx_http_upstream.c |  16 +---
 1 files changed, 13 insertions(+), 3 deletions(-)

diffs (26 lines):

diff --git a/src/http/ngx_http_upstream.c b/src/http/ngx_http_upstream.c
--- a/src/http/ngx_http_upstream.c
+++ b/src/http/ngx_http_upstream.c
@@ -3736,9 +3736,19 @@ ngx_http_upstream_thread_event_handler(n
 r->main->blocked--;
 r->aio = 0;
 
-r->write_event_handler(r);
-
-ngx_http_run_posted_requests(c);
+if (r->done) {
+/*
+ * trigger connection event handler if the subrequest was
+ * already finalized; this can happen if the handler is used
+ * for sendfile() in threads
+ */
+
+c->write->handler(c->write);
+
+} else {
+r->write_event_handler(r);
+ngx_http_run_posted_requests(c);
+}
 }
 
 #endif
___
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel

[nginx] Simplified and improved sendfile() code on Linux.

2017-03-28 Thread Maxim Dounin 
details:   http://hg.nginx.org/nginx/rev/ff0c8e11edbc
branches:  
changeset: 6949:ff0c8e11edbc
user:  Maxim Dounin 
date:  Tue Mar 28 18:15:39 2017 +0300
description:
Simplified and improved sendfile() code on Linux.

The ngx_linux_sendfile() function is now used for both normal sendfile()
and sendfile in threads.  The ngx_linux_sendfile_thread() function was
modified to use the same interface as ngx_linux_sendfile(), and is simply
called from ngx_linux_sendfile() when threads are enabled.

Special return code NGX_DONE is used to indicate that a thread task was
posted and no further actions are needed.

If number of bytes sent is less that what we were sending, we now always
retry sending.  This is needed for sendfile() in threads as the number
of bytes we are sending might have been changed since the thread task
was posted.  And this is also needed for Linux 4.3+, as sendfile() might
be interrupted at any time and provides no indication if it was interrupted
or not (ticket #1174).

diffstat:

 src/os/unix/ngx_linux_sendfile_chain.c |  114 +---
 1 files changed, 47 insertions(+), 67 deletions(-)

diffs (203 lines):

diff --git a/src/os/unix/ngx_linux_sendfile_chain.c 
b/src/os/unix/ngx_linux_sendfile_chain.c
--- a/src/os/unix/ngx_linux_sendfile_chain.c
+++ b/src/os/unix/ngx_linux_sendfile_chain.c
@@ -20,8 +20,8 @@ static ssize_t ngx_linux_sendfile(ngx_co
 #error sendfile64() is required!
 #endif
 
-static ngx_int_t ngx_linux_sendfile_thread(ngx_connection_t *c, ngx_buf_t 
*file,
-size_t size, size_t *sent);
+static ssize_t ngx_linux_sendfile_thread(ngx_connection_t *c, ngx_buf_t *file,
+size_t size);
 static void ngx_linux_sendfile_thread_handler(void *data, ngx_log_t *log);
 #endif
 
@@ -56,10 +56,6 @@ ngx_linux_sendfile_chain(ngx_connection_
 ngx_chain_t   *cl;
 ngx_iovec_theader;
 struct iovec   headers[NGX_IOVS_PREALLOCATE];
-#if (NGX_THREADS)
-ngx_int_t  rc;
-ngx_uint_t thread_handled, thread_complete;
-#endif
 
 wev = c->write;
 
@@ -82,10 +78,6 @@ ngx_linux_sendfile_chain(ngx_connection_
 
 for ( ;; ) {
 prev_send = send;
-#if (NGX_THREADS)
-thread_handled = 0;
-thread_complete = 0;
-#endif
 
 /* create the iovec and coalesce the neighbouring bufs */
 
@@ -179,37 +171,18 @@ ngx_linux_sendfile_chain(ngx_connection_
 }
 #endif
 
-#if (NGX_THREADS)
-if (file->file->thread_handler) {
-rc = ngx_linux_sendfile_thread(c, file, file_size, );
-
-switch (rc) {
-case NGX_OK:
-thread_handled = 1;
-break;
-
-case NGX_DONE:
-thread_complete = 1;
-break;
+n = ngx_linux_sendfile(c, file, file_size);
 
-case NGX_AGAIN:
-break;
-
-default: /* NGX_ERROR */
-return NGX_CHAIN_ERROR;
-}
+if (n == NGX_ERROR) {
+return NGX_CHAIN_ERROR;
+}
 
-} else
-#endif
-{
-n = ngx_linux_sendfile(c, file, file_size);
+if (n == NGX_DONE) {
+/* thread task posted */
+return in;
+}
 
-if (n == NGX_ERROR) {
-return NGX_CHAIN_ERROR;
-}
-
-sent = (n == NGX_AGAIN) ? 0 : n;
-}
+sent = (n == NGX_AGAIN) ? 0 : n;
 
 } else {
 n = ngx_writev(c, );
@@ -225,21 +198,27 @@ ngx_linux_sendfile_chain(ngx_connection_
 
 in = ngx_chain_update_sent(in, sent);
 
-if ((size_t) (send - prev_send) != sent) {
-#if (NGX_THREADS)
-if (thread_handled) {
-return in;
-}
-
-if (thread_complete) {
-send = prev_send + sent;
-continue;
-}
-#endif
+if (n == NGX_AGAIN) {
 wev->ready = 0;
 return in;
 }
 
+if ((size_t) (send - prev_send) != sent) {
+
+/*
+ * sendfile() on Linux 4.3+ might be interrupted at any time,
+ * and provides no indication if it was interrupted or not,
+ * so we have to retry till an explicit EAGAIN
+ *
+ * sendfile() in threads can also report less bytes written
+ * than we are prepared to send now, since it was started in
+ * some point in the past, so we again have to retry
+ */
+
+send = prev_send + sent;
+continue;
+}
+
 if (send >= limit || in == NULL) {
 return in;
 }
@@ -258,6 +237,14 @@ ngx_linux_sendfile(ngx_connection_t *c, 
 ssize_tn;
 ngx_err_t  err;
 
+#if (NGX_THREADS)
+
+if (file->file->thread_handler) {
+return ngx_linux_sendfile_thread(c, file,

[njs] Closures support.

2017-03-28 Thread Igor Sysoev 
details:   http://hg.nginx.org/njs/rev/a095dc0cd361
branches:  
changeset: 321:a095dc0cd361
user:  Igor Sysoev 
date:  Tue Mar 28 07:50:05 2017 +0300
description:
Closures support.

diffstat:

 njs/njs_array.c  |2 +-
 njs/njs_boolean.c|2 +-
 njs/njs_date.c   |4 +-
 njs/njs_function.c   |  159 +++-
 njs/njs_function.h   |   29 +++-
 njs/njs_generator.c  |  109 +---
 njs/njs_number.c |2 +-
 njs/njs_parser.c |   87 -
 njs/njs_parser.h |   26 ++-
 njs/njs_string.c |2 +-
 njs/njs_variable.c   |  291 ++
 njs/njs_variable.h   |   18 +-
 njs/njs_vm.c |  237 -
 njs/njs_vm.h |   68 +++---
 njs/njscript.c   |   14 +-
 njs/test/njs_unit_test.c |   35 +-
 16 files changed, 773 insertions(+), 312 deletions(-)

diffs (truncated from 2072 to 1000 lines):

diff -r 0bde7f156477 -r a095dc0cd361 njs/njs_array.c
--- a/njs/njs_array.c   Sat Mar 25 13:42:40 2017 +0300
+++ b/njs/njs_array.c   Tue Mar 28 07:50:05 2017 +0300
@@ -725,7 +725,7 @@ njs_array_prototype_to_string_continuati
 nxt_uint_t nargs, njs_index_t retval)
 {
 /* Skip retval update. */
-vm->frame->skip = 1;
+vm->top_frame->skip = 1;
 
 return NXT_OK;
 }
diff -r 0bde7f156477 -r a095dc0cd361 njs/njs_boolean.c
--- a/njs/njs_boolean.c Sat Mar 25 13:42:40 2017 +0300
+++ b/njs/njs_boolean.c Tue Mar 28 07:50:05 2017 +0300
@@ -34,7 +34,7 @@ njs_boolean_constructor(njs_vm_t *vm, nj
 value = njs_is_true([1]) ? _value_true : _value_false;
 }
 
-if (vm->frame->ctor) {
+if (vm->top_frame->ctor) {
 object = njs_object_value_alloc(vm, value, value->type);
 if (nxt_slow_path(object == NULL)) {
 return NXT_ERROR;
diff -r 0bde7f156477 -r a095dc0cd361 njs/njs_date.c
--- a/njs/njs_date.cSat Mar 25 13:42:40 2017 +0300
+++ b/njs/njs_date.cTue Mar 28 07:50:05 2017 +0300
@@ -91,7 +91,7 @@ njs_date_constructor(njs_vm_t *vm, njs_v
 njs_date_t  *date;
 struct tm   tm;
 
-if (vm->frame->ctor) {
+if (vm->top_frame->ctor) {
 
 if (nargs == 1) {
 time = njs_gettime();
@@ -1892,7 +1892,7 @@ njs_date_prototype_to_json_continuation(
 nxt_uint_t nargs, njs_index_t retval)
 {
 /* Skip retval update. */
-vm->frame->skip = 1;
+vm->top_frame->skip = 1;
 
 return NXT_OK;
 }
diff -r 0bde7f156477 -r a095dc0cd361 njs/njs_function.c
--- a/njs/njs_function.cSat Mar 25 13:42:40 2017 +0300
+++ b/njs/njs_function.cTue Mar 28 07:50:05 2017 +0300
@@ -60,7 +60,9 @@ njs_function_alloc(njs_vm_t *vm)
 njs_function_t *
 njs_function_value_copy(njs_vm_t *vm, njs_value_t *value)
 {
-njs_function_t  *function;
+size_t  size;
+nxt_uint_t  n, nesting;
+njs_function_t  *function, *copy;
 
 function = value->data.u.function;
 
@@ -68,17 +70,36 @@ njs_function_value_copy(njs_vm_t *vm, nj
 return function;
 }
 
-function = nxt_mem_cache_alloc(vm->mem_cache_pool, sizeof(njs_function_t));
+nesting = (function->native) ? 0 : function->u.lambda->nesting;
 
-if (nxt_fast_path(function != NULL)) {
-*function = *value->data.u.function;
-function->object.__proto__ =
->prototypes[NJS_PROTOTYPE_FUNCTION].object;
-function->object.shared = 0;
-value->data.u.function = function;
+size = sizeof(njs_function_t) + nesting * sizeof(njs_closure_t *);
+
+copy = nxt_mem_cache_alloc(vm->mem_cache_pool, size);
+if (nxt_slow_path(copy == NULL)) {
+return copy;
 }
 
-return function;
+value->data.u.function = copy;
+
+*copy = *function;
+copy->object.__proto__ = >prototypes[NJS_PROTOTYPE_FUNCTION].object;
+copy->object.shared = 0;
+
+if (nesting == 0) {
+return copy;
+}
+
+copy->closure = 1;
+
+n = 0;
+
+do {
+/* GC: retain closure. */
+copy->closures[n] = vm->active_frame->closures[n];
+n++;
+} while (n < nesting);
+
+return copy;
 }
 
 
@@ -107,6 +128,7 @@ njs_function_native_frame(njs_vm_t *vm, 
 frame->ctor = ctor;
 
 value = (njs_value_t *) (njs_continuation(frame) + reserve);
+frame->arguments = value;
 
 bound = function->bound;
 
@@ -124,7 +146,6 @@ njs_function_native_frame(njs_vm_t *vm, 
 } while (n != 0);
 }
 
-frame->arguments = value;
 vm->scopes[NJS_SCOPE_CALLEE_ARGUMENTS] = value;
 
 if (args != NULL) {
@@ -140,17 +161,23 @@ njs_function_frame(njs_vm_t *vm, njs_fun
 const njs_value_t *this, njs_value_t *args, nxt_uint_t nargs,
 nxt_bool_t ctor)
 {
-size_t  size;
-nxt_uint_t  n, max_args;
-njs_value_t *value, *bound;
-njs_frame_t *frame;
-njs_native_frame_t  *native_frame;
+size_t

[njs] The "typeof" operation changes.

2017-03-28 Thread Igor Sysoev 
details:   http://hg.nginx.org/njs/rev/0bde7f156477
branches:  
changeset: 320:0bde7f156477
user:  Igor Sysoev 
date:  Sat Mar 25 13:42:40 2017 +0300
description:
The "typeof" operation changes.

diffstat:

 njs/njs_variable.c |  19 +++
 njs/njs_vm.c   |   7 ++-
 2 files changed, 9 insertions(+), 17 deletions(-)

diffs (86 lines):

diff -r 8e1030bce13c -r 0bde7f156477 njs/njs_variable.c
--- a/njs/njs_variable.cTue Mar 21 16:35:36 2017 +0300
+++ b/njs/njs_variable.cSat Mar 25 13:42:40 2017 +0300
@@ -176,14 +176,12 @@ njs_variable_t *
 njs_variable_get(njs_vm_t *vm, njs_parser_node_t *node,
 njs_name_reference_t reference)
 {
-nxt_int_t   ret;
 nxt_array_t *values;
 njs_index_t index;
 njs_value_t *value;
 njs_variable_t  *var;
 njs_parser_scope_t  *scope, *parent, *inclusive;
 nxt_lvlhsh_query_t  lhq;
-const njs_value_t   *initial;
 
 lhq.key_hash = node->variable_name_hash;
 lhq.key = node->u.variable_name;
@@ -209,8 +207,6 @@ njs_variable_get(njs_vm_t *vm, njs_parse
 }
 }
 
-initial = _value_void;
-
 goto found;
 }
 
@@ -229,23 +225,14 @@ njs_variable_get(njs_vm_t *vm, njs_parse
 goto not_found;
 }
 
-/* Add variable referenced by typeof to the global scope. */
-
 var = njs_variable_alloc(vm, , NJS_VARIABLE_TYPEOF);
 if (nxt_slow_path(var == NULL)) {
 return NULL;
 }
 
-lhq.replace = 0;
-lhq.value = var;
-lhq.pool = vm->mem_cache_pool;
+var->index = NJS_INDEX_NONE;
 
-ret = nxt_lvlhsh_insert(>variables, );
-if (nxt_slow_path(ret != NXT_OK)) {
-return NULL;
-}
-
-initial = _value_invalid;
+return var;
 
 found:
 
@@ -285,7 +272,7 @@ found:
 *value = var->value;
 
 } else {
-*value = *initial;
+*value = njs_value_void;
 }
 
 index = scope->next_index;
diff -r 8e1030bce13c -r 0bde7f156477 njs/njs_vm.c
--- a/njs/njs_vm.c  Tue Mar 21 16:35:36 2017 +0300
+++ b/njs/njs_vm.c  Sat Mar 25 13:42:40 2017 +0300
@@ -1444,6 +1444,8 @@ njs_vmcode_post_decrement(njs_vm_t *vm, 
 njs_ret_t
 njs_vmcode_typeof(njs_vm_t *vm, njs_value_t *value, njs_value_t *invld)
 {
+nxt_uint_t  type;
+
 /* ECMAScript 5.1: null, array and regexp are objects. */
 
 static const njs_value_t  *types[] = {
@@ -1466,7 +1468,10 @@ njs_vmcode_typeof(njs_vm_t *vm, njs_valu
 _string_object,
 };
 
-vm->retval = *types[value->type];
+/* A zero index means non-declared variable. */
+type = (value != NULL) ? value->type : NJS_VOID;
+
+vm->retval = *types[type];
 
 return sizeof(njs_vmcode_2addr_t);
 }
___
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel

[nginx] Fixed ngx_open_cached_file() error handling.

2017-03-28 Thread Sergey Kandaurov 
details:   http://hg.nginx.org/nginx/rev/3fb9b5eb75c0
branches:  
changeset: 6948:3fb9b5eb75c0
user:  Sergey Kandaurov 
date:  Tue Mar 28 14:21:38 2017 +0300
description:
Fixed ngx_open_cached_file() error handling.

If of.err is 0, it means that there was a memory allocation error
and no further logging and/or processing is needed.  The of.failed
string can be only accessed if of.err is not 0.

diffstat:

 src/http/modules/ngx_http_index_module.c |  6 +++---
 src/http/modules/ngx_http_log_module.c   |  5 +
 src/http/ngx_http_core_module.c  |  5 +
 src/http/ngx_http_script.c   |  6 ++
 src/stream/ngx_stream_log_module.c   |  5 +
 5 files changed, 24 insertions(+), 3 deletions(-)

diffs (81 lines):

diff -r a8d7c9139831 -r 3fb9b5eb75c0 src/http/modules/ngx_http_index_module.c
--- a/src/http/modules/ngx_http_index_module.c  Tue Mar 28 11:28:51 2017 +0300
+++ b/src/http/modules/ngx_http_index_module.c  Tue Mar 28 14:21:38 2017 +0300
@@ -217,13 +217,13 @@ ngx_http_index_handler(ngx_http_request_
 if (ngx_open_cached_file(clcf->open_file_cache, , , r->pool)
 != NGX_OK)
 {
-ngx_log_debug2(NGX_LOG_DEBUG_HTTP, r->connection->log, of.err,
-   "%s \"%s\" failed", of.failed, path.data);
-
 if (of.err == 0) {
 return NGX_HTTP_INTERNAL_SERVER_ERROR;
 }
 
+ngx_log_debug2(NGX_LOG_DEBUG_HTTP, r->connection->log, of.err,
+   "%s \"%s\" failed", of.failed, path.data);
+
 #if (NGX_HAVE_OPENAT)
 if (of.err == NGX_EMLINK
 || of.err == NGX_ELOOP)
diff -r a8d7c9139831 -r 3fb9b5eb75c0 src/http/modules/ngx_http_log_module.c
--- a/src/http/modules/ngx_http_log_module.cTue Mar 28 11:28:51 2017 +0300
+++ b/src/http/modules/ngx_http_log_module.cTue Mar 28 14:21:38 2017 +0300
@@ -552,6 +552,11 @@ ngx_http_log_script_write(ngx_http_reque
 if (ngx_open_cached_file(llcf->open_file_cache, , , r->pool)
 != NGX_OK)
 {
+if (of.err == 0) {
+/* simulate successful logging */
+return len;
+}
+
 ngx_log_error(NGX_LOG_CRIT, r->connection->log, ngx_errno,
   "%s \"%s\" failed", of.failed, log.data);
 /* simulate successful logging */
diff -r a8d7c9139831 -r 3fb9b5eb75c0 src/http/ngx_http_core_module.c
--- a/src/http/ngx_http_core_module.c   Tue Mar 28 11:28:51 2017 +0300
+++ b/src/http/ngx_http_core_module.c   Tue Mar 28 14:21:38 2017 +0300
@@ -1314,6 +1314,11 @@ ngx_http_core_try_files_phase(ngx_http_r
 if (ngx_open_cached_file(clcf->open_file_cache, , , r->pool)
 != NGX_OK)
 {
+if (of.err == 0) {
+ngx_http_finalize_request(r, NGX_HTTP_INTERNAL_SERVER_ERROR);
+return NGX_OK;
+}
+
 if (of.err != NGX_ENOENT
 && of.err != NGX_ENOTDIR
 && of.err != NGX_ENAMETOOLONG)
diff -r a8d7c9139831 -r 3fb9b5eb75c0 src/http/ngx_http_script.c
--- a/src/http/ngx_http_script.cTue Mar 28 11:28:51 2017 +0300
+++ b/src/http/ngx_http_script.cTue Mar 28 14:21:38 2017 +0300
@@ -1513,6 +1513,12 @@ ngx_http_script_file_code(ngx_http_scrip
 if (ngx_open_cached_file(clcf->open_file_cache, , , r->pool)
 != NGX_OK)
 {
+if (of.err == 0) {
+e->ip = ngx_http_script_exit;
+e->status = NGX_HTTP_INTERNAL_SERVER_ERROR;
+return;
+}
+
 if (of.err != NGX_ENOENT
 && of.err != NGX_ENOTDIR
 && of.err != NGX_ENAMETOOLONG)
diff -r a8d7c9139831 -r 3fb9b5eb75c0 src/stream/ngx_stream_log_module.c
--- a/src/stream/ngx_stream_log_module.cTue Mar 28 11:28:51 2017 +0300
+++ b/src/stream/ngx_stream_log_module.cTue Mar 28 14:21:38 2017 +0300
@@ -443,6 +443,11 @@ ngx_stream_log_script_write(ngx_stream_s
  s->connection->pool)
 != NGX_OK)
 {
+if (of.err == 0) {
+/* simulate successful logging */
+return len;
+}
+
 ngx_log_error(NGX_LOG_CRIT, s->connection->log, ngx_errno,
   "%s \"%s\" failed", of.failed, log.data);
 /* simulate successful logging */
___
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel

[PATCH] HTTP/2: emit PROTOCOL_ERROR on invalid WINDOW_UPDATE increments

2017-03-28 Thread Piotr Sikora via nginx-devel 
# HG changeset patch
# User Piotr Sikora 
# Date 1490516706 25200
#  Sun Mar 26 01:25:06 2017 -0700
# Node ID ccb36c87291e38d1a63224d143cbeaa4ee4a4287
# Parent  22be63bf21edaa1b8ea916c7d8cd4e5fe4892061
HTTP/2: emit PROTOCOL_ERROR on invalid WINDOW_UPDATE increments.

Signed-off-by: Piotr Sikora 

diff -r 22be63bf21ed -r ccb36c87291e src/http/v2/ngx_http_v2.c
--- a/src/http/v2/ngx_http_v2.c
+++ b/src/http/v2/ngx_http_v2.c
@@ -2168,11 +2168,42 @@ ngx_http_v2_state_window_update(ngx_http
 ngx_log_debug0(NGX_LOG_DEBUG_HTTP, h2c->connection->log, 0,
"unknown http2 stream");
 
+if (window == 0) {
+ngx_log_error(NGX_LOG_INFO, h2c->connection->log, 0,
+  "client sent WINDOW_UPDATE frame for unknown "
+  "stream %ui with incorrect window increment 0",
+  h2c->state.sid);
+
+if (ngx_http_v2_send_rst_stream(h2c, h2c->state.sid,
+NGX_HTTP_V2_PROTOCOL_ERROR)
+== NGX_ERROR)
+{
+return ngx_http_v2_connection_error(h2c,
+   NGX_HTTP_V2_INTERNAL_ERROR);
+}
+}
+
 return ngx_http_v2_state_complete(h2c, pos, end);
 }
 
 stream = node->stream;
 
+if (window == 0) {
+ngx_log_error(NGX_LOG_INFO, h2c->connection->log, 0,
+  "client sent WINDOW_UPDATE frame for stream %ui "
+  "with incorrect window increment 0", h2c->state.sid);
+
+if (ngx_http_v2_terminate_stream(h2c, stream,
+ NGX_HTTP_V2_PROTOCOL_ERROR)
+== NGX_ERROR)
+{
+return ngx_http_v2_connection_error(h2c,
+
NGX_HTTP_V2_INTERNAL_ERROR);
+}
+
+return ngx_http_v2_state_complete(h2c, pos, end);
+}
+
 if (window > (size_t) (NGX_HTTP_V2_MAX_WINDOW - stream->send_window)) {
 
 ngx_log_error(NGX_LOG_INFO, h2c->connection->log, 0,
@@ -2211,6 +2242,14 @@ ngx_http_v2_state_window_update(ngx_http
 return ngx_http_v2_state_complete(h2c, pos, end);
 }
 
+if (window == 0) {
+ngx_log_error(NGX_LOG_INFO, h2c->connection->log, 0,
+  "client sent WINDOW_UPDATE frame "
+  "with incorrect window increment 0");
+
+return ngx_http_v2_connection_error(h2c, NGX_HTTP_V2_PROTOCOL_ERROR);
+}
+
 if (window > NGX_HTTP_V2_MAX_WINDOW - h2c->send_window) {
 ngx_log_error(NGX_LOG_INFO, h2c->connection->log, 0,
   "client violated connection flow control: "
___
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel

Re: [PATCH] HTTP/2: reject HTTP/2 requests without ":scheme" pseudo-header

2017-03-28 Thread Piotr Sikora via nginx-devel 
Hey Valentin,

> This part of patch can be added to the style one:
>
>   "HTTP/2: style and typos."

Assuming that this patch gets dropped or even if it gets committed?

> Is there any practical reason to force this restriction?

None, other than following RFC and providing early detection of broken clients.

Also, it looks that you have a test for it marked as TODO:
http://hg.nginx.org/nginx-tests/annotate/a6abbfed42c0/h2_headers.t#l972

Best regards,
Piotr Sikora
___
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel

Re: [PATCH] HTTP/2: emit PROTOCOL_ERROR on invalid ENABLE_PUSH setting value

2017-03-28 Thread Piotr Sikora via nginx-devel 
Hey Valentin,

> Is there any practical reason for the check considering that
> the value is ignored anyway?

None, other than following RFC and providing early detection of broken clients.

Best regards,
Piotr Sikora
___
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel

Re: [PATCH] HTTP/2: emit PROTOCOL_ERROR on invalid WINDOW_UPDATE increments

2017-03-28 Thread Piotr Sikora via nginx-devel 
Hey Valentin,

> I'm not sure that strictly following RFC here is worth the effort.
>
> It seems there's no other "harm" from zero window updates except that it
> allows to reset timers without any progress.  That's only slightly worse
> than 1-bytes window updates.

Flow control interoperability and deadlocks between various HTTP/2
implementations are the biggest issues with the protocol, so while
there is no real harm in allowing 0 window updates, they indicate
broken client, and resetting stream and/or connection as soon as such
thing happens makes it much easier to find issues.

> The downside is additional code and intolerance to potential client bugs.

Catching client bugs early on is the upside, IMHO.

> Also note that in your implementation if zero window update is received
> for unknown stream then it's silently ignored.

Good catch, thanks! I'll send fixed version shortly.

Best regards,
Piotr Sikora
___
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel

[nginx] Core: set nginx_shared_zone name via ngx_str_set().

2017-03-28 Thread Ruslan Ermilov 
details:   http://hg.nginx.org/nginx/rev/a8d7c9139831
branches:  
changeset: 6947:a8d7c9139831
user:  Ruslan Ermilov 
date:  Tue Mar 28 11:28:51 2017 +0300
description:
Core: set nginx_shared_zone name via ngx_str_set().

diffstat:

 src/event/ngx_event.c |  3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diffs (13 lines):

diff -r 99c87770b81b -r a8d7c9139831 src/event/ngx_event.c
--- a/src/event/ngx_event.c Tue Mar 28 11:28:42 2017 +0300
+++ b/src/event/ngx_event.c Tue Mar 28 11:28:51 2017 +0300
@@ -500,8 +500,7 @@ ngx_event_module_init(ngx_cycle_t *cycle
 #endif
 
 shm.size = size;
-shm.name.len = sizeof("nginx_shared_zone") - 1;
-shm.name.data = (u_char *) "nginx_shared_zone";
+ngx_str_set(, "nginx_shared_zone");
 shm.log = cycle->log;
 
 if (ngx_shm_alloc() != NGX_OK) {
___
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel

[nginx] Version bump.

2017-03-28 Thread Ruslan Ermilov 
details:   http://hg.nginx.org/nginx/rev/6e1a0a0d5f04
branches:  
changeset: 6945:6e1a0a0d5f04
user:  Ruslan Ermilov 
date:  Tue Mar 28 11:28:36 2017 +0300
description:
Version bump.

diffstat:

 src/core/nginx.h |  4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diffs (14 lines):

diff -r 22be63bf21ed -r 6e1a0a0d5f04 src/core/nginx.h
--- a/src/core/nginx.h  Fri Mar 24 18:05:06 2017 +0300
+++ b/src/core/nginx.h  Tue Mar 28 11:28:36 2017 +0300
@@ -9,8 +9,8 @@
 #define _NGINX_H_INCLUDED_
 
 
-#define nginx_version  1011012
-#define NGINX_VERSION  "1.11.12"
+#define nginx_version  1011013
+#define NGINX_VERSION  "1.11.13"
 #define NGINX_VER  "nginx/" NGINX_VERSION
 
 #ifdef NGX_BUILD
___
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel

[nginx] Use ngx_array_init() to initialize arrays.

2017-03-28 Thread Ruslan Ermilov 
details:   http://hg.nginx.org/nginx/rev/99c87770b81b
branches:  
changeset: 6946:99c87770b81b
user:  Ruslan Ermilov 
date:  Tue Mar 28 11:28:42 2017 +0300
description:
Use ngx_array_init() to initialize arrays.

diffstat:

 src/core/ngx_cycle.c |  34 +++---
 1 files changed, 15 insertions(+), 19 deletions(-)

diffs (68 lines):

diff -r 6e1a0a0d5f04 -r 99c87770b81b src/core/ngx_cycle.c
--- a/src/core/ngx_cycle.c  Tue Mar 28 11:28:36 2017 +0300
+++ b/src/core/ngx_cycle.c  Tue Mar 28 11:28:42 2017 +0300
@@ -115,16 +115,14 @@ ngx_init_cycle(ngx_cycle_t *old_cycle)
 
 n = old_cycle->paths.nelts ? old_cycle->paths.nelts : 10;
 
-cycle->paths.elts = ngx_pcalloc(pool, n * sizeof(ngx_path_t *));
-if (cycle->paths.elts == NULL) {
+if (ngx_array_init(>paths, pool, n, sizeof(ngx_path_t *))
+!= NGX_OK)
+{
 ngx_destroy_pool(pool);
 return NULL;
 }
 
-cycle->paths.nelts = 0;
-cycle->paths.size = sizeof(ngx_path_t *);
-cycle->paths.nalloc = n;
-cycle->paths.pool = pool;
+ngx_memzero(cycle->paths.elts, n * sizeof(ngx_path_t *));
 
 
 if (ngx_array_init(>config_dump, pool, 1, sizeof(ngx_conf_dump_t))
@@ -175,16 +173,14 @@ ngx_init_cycle(ngx_cycle_t *old_cycle)
 
 n = old_cycle->listening.nelts ? old_cycle->listening.nelts : 10;
 
-cycle->listening.elts = ngx_pcalloc(pool, n * sizeof(ngx_listening_t));
-if (cycle->listening.elts == NULL) {
+if (ngx_array_init(>listening, pool, n, sizeof(ngx_listening_t))
+!= NGX_OK)
+{
 ngx_destroy_pool(pool);
 return NULL;
 }
 
-cycle->listening.nelts = 0;
-cycle->listening.size = sizeof(ngx_listening_t);
-cycle->listening.nalloc = n;
-cycle->listening.pool = pool;
+ngx_memzero(cycle->listening.elts, n * sizeof(ngx_listening_t));
 
 
 ngx_queue_init(>reusable_connections_queue);
@@ -768,15 +764,15 @@ old_shm_zone_done:
 }
 
 n = 10;
-ngx_old_cycles.elts = ngx_pcalloc(ngx_temp_pool,
-  n * sizeof(ngx_cycle_t *));
-if (ngx_old_cycles.elts == NULL) {
+
+if (ngx_array_init(_old_cycles, ngx_temp_pool, n,
+   sizeof(ngx_cycle_t *))
+!= NGX_OK)
+{
 exit(1);
 }
-ngx_old_cycles.nelts = 0;
-ngx_old_cycles.size = sizeof(ngx_cycle_t *);
-ngx_old_cycles.nalloc = n;
-ngx_old_cycles.pool = ngx_temp_pool;
+
+ngx_memzero(ngx_old_cycles.elts, n * sizeof(ngx_cycle_t *));
 
 ngx_cleaner_event.handler = ngx_clean_old_cycles;
 ngx_cleaner_event.log = cycle->log;
___
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel