Re:[patch] Slice filter: support for empty file.
Hi, >416 will be returned when the request has no Range HEADER and the target file >is >empty. Apparently, it does not conform to the HTTP protocol. Empty file seems >inevitable in the CDN service where Nginx is heavily used. > ># HG changeset patch ># User hucongcong># Date 1497892764 -28800 ># Tue Jun 20 01:19:24 2017 +0800 ># Node ID 79d38b2d27d4eb92395cf1ff43bbfe23498bc69a ># Parent d1816a2696de8c2faa1cd913a151e5f62a8620f3 >Slice filter: support for empty file. The following modifications may be more appropriate. # HG changeset patch # User hucongcong # Date 1497926137 -28800 # Tue Jun 20 10:35:37 2017 +0800 # Node ID e42abb7e28c4f0dd3d66cb81aa2623e7fae8b4da # Parent a39bc74873faf9e5bea616561b43f6ecc55229f9 Slice filter: support for empty file. diff -r a39bc74873fa -r e42abb7e28c4 src/http/modules/ngx_http_slice_filter_module.c --- a/src/http/modules/ngx_http_slice_filter_module.c Mon Jun 19 14:25:42 2017 +0300 +++ b/src/http/modules/ngx_http_slice_filter_module.c Tue Jun 20 10:35:37 2017 +0800 @@ -114,7 +114,21 @@ ngx_http_slice_header_filter(ngx_http_re } if (r->headers_out.status != NGX_HTTP_PARTIAL_CONTENT) { + if (r == r->main) { +if (r->headers_in.range == NULL +&& r->headers_out.status == NGX_HTTP_RANGE_NOT_SATISFIABLE) +{ +r->header_only = 1; +ngx_str_null(>headers_out.content_type); + +r->headers_out.status = NGX_HTTP_OK; +r->headers_out.status_line.len = 0; +r->headers_out.content_length_n = 0; +r->headers_out.content_range->hash = 0; +r->headers_out.content_range = NULL; +} + ngx_http_set_ctx(r, NULL, ngx_http_slice_filter_module); return ngx_http_next_header_filter(r); } ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
[nginx] slice module issue
Hi! Have a look at the following example first. server { listen 80; location / { slice 10; proxy_set_header Range $slice_range; proxy_pass http://127.0.0.1:81; } } server { listen 81; root html; } Then we start a request with curl. > curl http://my.test.com/ -x 127.1:80 -H "range: bytes=1-50, 2-51" We get a response of the whole file that differs from expectation (1-50, 2-51). It seems that slice module doesn't support multi-range (separated by commas), but it's confused $slice_range variable is valid. Please confirm this question and the following patch, thanks! diff -r 5e05118678af src/http/modules/ngx_http_slice_filter_module.c --- a/src/http/modules/ngx_http_slice_filter_module.c Mon May 29 23:33:38 2017 +0300 +++ b/src/http/modules/ngx_http_slice_filter_module.c Mon Jun 19 09:35:24 2017 -0400 @@ -389,6 +389,7 @@ ngx_http_variable_value_t *v, uintptr_t data) { u_char *p; +off_t start; ngx_http_slice_ctx_t *ctx; ngx_http_slice_loc_conf_t *slcf; @@ -407,6 +408,13 @@ return NGX_OK; } +start = ngx_http_slice_get_start(r); + +if (start == -1) { +v->not_found = 1; +return NGX_OK; +} + ctx = ngx_pcalloc(r->pool, sizeof(ngx_http_slice_ctx_t)); if (ctx == NULL) { return NGX_ERROR; @@ -419,7 +427,7 @@ return NGX_ERROR; } -ctx->start = slcf->size * (ngx_http_slice_get_start(r) / slcf->size); +ctx->start = slcf->size * (start / slcf->size); ctx->range.data = p; ctx->range.len = ngx_sprintf(p, "bytes=%O-%O", ctx->start, @@ -460,7 +468,7 @@ p = h->value.data + 6; if (ngx_strchr(p, ',')) { -return 0; +return -1; } while (*p == ' ') { p++; } And this is a better conf. map $slice_range $x_slice_range { default $http_range; ~ $slice_range; } server { listen 80; location / { slice 10; proxy_set_header Range $x_slice_range; proxy_pass http://127.0.0.1:81; } } ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
[patch] Slice filter: support for empty file.
Hi, 416 will be returned when the request has no Range HEADER and the target file is empty. Apparently, it does not conform to the HTTP protocol. Empty file seems inevitable in the CDN service where Nginx is heavily used. # HG changeset patch # User hucongcong# Date 1497892764 -28800 # Tue Jun 20 01:19:24 2017 +0800 # Node ID 79d38b2d27d4eb92395cf1ff43bbfe23498bc69a # Parent d1816a2696de8c2faa1cd913a151e5f62a8620f3 Slice filter: support for empty file. diff -r d1816a2696de -r 79d38b2d27d4 src/http/modules/ngx_http_slice_filter_module.c --- a/src/http/modules/ngx_http_slice_filter_module.c Fri Jun 16 18:15:58 2017 +0300 +++ b/src/http/modules/ngx_http_slice_filter_module.c Tue Jun 20 01:19:24 2017 +0800 @@ -22,6 +22,7 @@ typedef struct { ngx_str_tetag; unsigned last:1; unsigned active:1; +unsigned no_range:1; ngx_http_request_t *sr; } ngx_http_slice_ctx_t; @@ -114,7 +115,21 @@ ngx_http_slice_header_filter(ngx_http_re } if (r->headers_out.status != NGX_HTTP_PARTIAL_CONTENT) { + if (r == r->main) { +if (ctx->no_range +&& r->headers_out.status == NGX_HTTP_RANGE_NOT_SATISFIABLE) +{ +r->header_only = 1; +ngx_str_null(>headers_out.content_type); + +r->headers_out.status = NGX_HTTP_OK; +r->headers_out.status_line.len = 0; +r->headers_out.content_length_n = 0; +r->headers_out.content_range->hash = 0; +r->headers_out.content_range = NULL; +} + ngx_http_set_ctx(r, NULL, ngx_http_slice_filter_module); return ngx_http_next_header_filter(r); } @@ -440,9 +455,10 @@ ngx_http_slice_range_variable(ngx_http_r static off_t ngx_http_slice_get_start(ngx_http_request_t *r) { -off_t start, cutoff, cutlim; -u_char *p; -ngx_table_elt_t *h; +off_t start, cutoff, cutlim; +u_char *p; +ngx_table_elt_t*h; +ngx_http_slice_ctx_t *ctx; if (r->headers_in.if_range) { return 0; @@ -454,6 +470,8 @@ ngx_http_slice_get_start(ngx_http_reques || h->value.len < 7 || ngx_strncasecmp(h->value.data, (u_char *) "bytes=", 6) != 0) { +ctx = ngx_http_get_module_ctx(r, ngx_http_slice_filter_module); +ctx->no_range = 1; return 0; } ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: [patch] Http image_filter: return 405 when method is HEAD andbody is empty.
Hi, On Tuesday, Jun 20, 2017 1:21 AM +0300, Maxim Dounin wrote: >On Tue, Jun 20, 2017 at 12:56:15AM +0800, 胡聪 (hucc) wrote: > >> Returning 415 does not conform to the HTTP protocol when image and proxy_pass >> configured in same location. >> >> # HG changeset patch >> # User hucongcong>> # Date 1497890354 -28800 >> # Tue Jun 20 00:39:14 2017 +0800 >> # Node ID af3a94de6a6549dec5e1205514eda1893313a14c >> # Parent d1816a2696de8c2faa1cd913a151e5f62a8620f3 >> Http image_filter: return 405 when method is HEAD and body is empty. >> >> diff -r d1816a2696de -r af3a94de6a65 >> src/http/modules/ngx_http_image_filter_module.c >> --- a/src/http/modules/ngx_http_image_filter_module.c Fri Jun 16 18:15:58 >> 2017 +0300 >> +++ b/src/http/modules/ngx_http_image_filter_module.c Tue Jun 20 00:39:14 >> 2017 +0800 >> @@ -330,6 +330,12 @@ ngx_http_image_body_filter(ngx_http_requ >> } >> } >> >> +if (r->method & NGX_HTTP_HEAD) { >> +return ngx_http_filter_finalize_request(r, >> + >> _http_image_filter_module, >> + NGX_HTTP_NOT_ALLOWED); >> +} >> + >> return ngx_http_filter_finalize_request(r, >>_http_image_filter_module, >> >> NGX_HTTP_UNSUPPORTED_MEDIA_TYPE); > >Please clarify why you think that the current code is wrong. I >don't see any problems with returning 415 to HEAD requests as long >we are going to return 415 to GETs. Ok, the problem is that nginx will return 200 to GET request and 415 to HEAD request. The configuration looks like: #proxy_methodGET;#not configured location / { imageresize 180 360; #... proxy_passhttp://test_upstream$uri; } Best wishes -hucc ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: [PATCH] Proxy: add "proxy_ssl_alpn" directive
Hello! On Mon, Jun 19, 2017 at 07:31:11AM -0700, Piotr Sikora via nginx-devel wrote: > Hey Maxim, > > > It doesn't look like this patch make sense by its own. > > > > If this patch is a part of a larger work, please consider > > submitting a patch series with the whole feature instead. > > Individual patches which doesn't make much sense > > by its own are hard to review, and are likely to be rejected. > > Actually, it does, the only missing part (from HTTP/2 patchset) is: > > case NGX_HTTP_VERSION_11: > ngx_str_set(, NGX_HTTP_11_ALPN_ADVERTISE); > break; > + > +#if (NGX_HTTP_V2) > +case NGX_HTTP_VERSION_20: > +ngx_str_set(, NGX_HTTP_V2_ALPN_ADVERTISE); > +break; > +#endif > } > > if (ngx_ssl_alpn_protos(cf, plcf->upstream.ssl, ) != NGX_OK) { > > ...so this patch is perfectly reviewable on its own. > > In case you expected ALPN-negotiation - I didn't add it, since it > would require rewrite of upstream logic, i.e. u->create_request() > would need to be called after upstream is already connected, and > possibly again in case of retries that negotiated different ALPN > protocol, which would add complexity to the already big patchset. > Also, I'm not sure how useful this is for upstream connections in > reverse proxies. > > Let me know if that's a must-have, but IMHO we could always add > "proxy_http_version alpn" in the future, without blocking this and > HTTP/2 patchset, which effectively implement "prior knowledge". I don't see how this patch is useful by its own, without additional HTTP/2 patchset, and it clearly adds additional user-visible complexity to the proxy module. And hence the suggestion is to include this patch into the HTTP/2 patchset as it is clearly a part of this patchset. Note well that at this point it might be a good idea to implement HTTP/2 to upstreams as a separate module, as it is done for other protocols like FastCGI, SCGI, uWSGI, and so on, instead of trying to built it into the proxy module which is intended to talk HTTP, not HTTP/2. It will better follow the generic concept we use now, "one protocol - one module", and will also imply much less blocking in general. -- Maxim Dounin http://nginx.org/ ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: [patch] Http image_filter: return 405 when method is HEAD and body is empty.
Hello! On Tue, Jun 20, 2017 at 12:56:15AM +0800, 胡聪 (hucc) wrote: > Returning 415 does not conform to the HTTP protocol when image and proxy_pass > configured in same location. > > # HG changeset patch > # User hucongcong> # Date 1497890354 -28800 > # Tue Jun 20 00:39:14 2017 +0800 > # Node ID af3a94de6a6549dec5e1205514eda1893313a14c > # Parent d1816a2696de8c2faa1cd913a151e5f62a8620f3 > Http image_filter: return 405 when method is HEAD and body is empty. > > diff -r d1816a2696de -r af3a94de6a65 > src/http/modules/ngx_http_image_filter_module.c > --- a/src/http/modules/ngx_http_image_filter_module.c Fri Jun 16 18:15:58 > 2017 +0300 > +++ b/src/http/modules/ngx_http_image_filter_module.c Tue Jun 20 00:39:14 > 2017 +0800 > @@ -330,6 +330,12 @@ ngx_http_image_body_filter(ngx_http_requ > } > } > > +if (r->method & NGX_HTTP_HEAD) { > +return ngx_http_filter_finalize_request(r, > + > _http_image_filter_module, > + NGX_HTTP_NOT_ALLOWED); > +} > + > return ngx_http_filter_finalize_request(r, >_http_image_filter_module, > > NGX_HTTP_UNSUPPORTED_MEDIA_TYPE); Please clarify why you think that the current code is wrong. I don't see any problems with returning 415 to HEAD requests as long we are going to return 415 to GETs. -- Maxim Dounin http://nginx.org/ ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
[patch] Http image_filter: return 405 when method is HEAD and body is empty.
Hi, Returning 415 does not conform to the HTTP protocol when image and proxy_pass configured in same location. # HG changeset patch # User hucongcong# Date 1497890354 -28800 # Tue Jun 20 00:39:14 2017 +0800 # Node ID af3a94de6a6549dec5e1205514eda1893313a14c # Parent d1816a2696de8c2faa1cd913a151e5f62a8620f3 Http image_filter: return 405 when method is HEAD and body is empty. diff -r d1816a2696de -r af3a94de6a65 src/http/modules/ngx_http_image_filter_module.c --- a/src/http/modules/ngx_http_image_filter_module.c Fri Jun 16 18:15:58 2017 +0300 +++ b/src/http/modules/ngx_http_image_filter_module.c Tue Jun 20 00:39:14 2017 +0800 @@ -330,6 +330,12 @@ ngx_http_image_body_filter(ngx_http_requ } } +if (r->method & NGX_HTTP_HEAD) { +return ngx_http_filter_finalize_request(r, + _http_image_filter_module, + NGX_HTTP_NOT_ALLOWED); +} + return ngx_http_filter_finalize_request(r, _http_image_filter_module, NGX_HTTP_UNSUPPORTED_MEDIA_TYPE); ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: [PATCH] add reload_delay directive
Hello, Maxim, Thanks for your detailed explanation and I truly agree with you. 2017-06-19 23:28 GMT+08:00 Maxim Dounin: > Hello! > > On Mon, Jun 19, 2017 at 11:09:58PM +0800, Peng Fang wrote: > > > # HG changeset patch > > # User RocFang > > # Date 1497882783 0 > > # Node ID 8b9e416ef7f9f8e7f96eaa53b479062683464481 > > # Parent a39bc74873faf9e5bea616561b43f6ecc55229f9 > > Introduced reload_delay. > > > > Previously, the master process will sleep 100ms before sending a > > SHUTDOWN signal to old worker processes when reload. This patch > > make the sleep time configurable, because in some scenarios, the > > new workers may spend more than 100ms to get ready. For example, > > the init_prcess hook of some 3rd modules may be time-consuming. > > The sleep in question is intended to let OS some time to actually > start the process, and not intended to allow time-consuming work > to happen on a worker process start. > > In general, no time-consuming operations are expected to be done > during a worker process start. Instead, time-consuming > preparatory work is expected to happen in the context of the > master process during configuration parsing and init module hooks. > If a module does something time-consuming in the init process > hook, it might be a good idea to change the module logic. > > Unless there is something more specific than a "the init_prcess > hook of some 3rd modules may be time-consuming", I would rather > reject the patch, as it introduces unneeded user-level complexity > by adding a directive, and encourages bad module writing practice. > > Nevertheless, thank you for the patch. > > -- > Maxim Dounin > http://nginx.org/ > ___ > nginx-devel mailing list > nginx-devel@nginx.org > http://mailman.nginx.org/mailman/listinfo/nginx-devel > ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: [PATCH] add reload_delay directive
Hello! On Mon, Jun 19, 2017 at 11:09:58PM +0800, Peng Fang wrote: > # HG changeset patch > # User RocFang> # Date 1497882783 0 > # Node ID 8b9e416ef7f9f8e7f96eaa53b479062683464481 > # Parent a39bc74873faf9e5bea616561b43f6ecc55229f9 > Introduced reload_delay. > > Previously, the master process will sleep 100ms before sending a > SHUTDOWN signal to old worker processes when reload. This patch > make the sleep time configurable, because in some scenarios, the > new workers may spend more than 100ms to get ready. For example, > the init_prcess hook of some 3rd modules may be time-consuming. The sleep in question is intended to let OS some time to actually start the process, and not intended to allow time-consuming work to happen on a worker process start. In general, no time-consuming operations are expected to be done during a worker process start. Instead, time-consuming preparatory work is expected to happen in the context of the master process during configuration parsing and init module hooks. If a module does something time-consuming in the init process hook, it might be a good idea to change the module logic. Unless there is something more specific than a "the init_prcess hook of some 3rd modules may be time-consuming", I would rather reject the patch, as it introduces unneeded user-level complexity by adding a directive, and encourages bad module writing practice. Nevertheless, thank you for the patch. -- Maxim Dounin http://nginx.org/ ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
[PATCH] add reload_delay directive
exporting patch: # HG changeset patch # User RocFang# Date 1497882783 0 # Node ID 8b9e416ef7f9f8e7f96eaa53b479062683464481 # Parent a39bc74873faf9e5bea616561b43f6ecc55229f9 Introduced reload_delay. Previously, the master process will sleep 100ms before sending a SHUTDOWN signal to old worker processes when reload. This patch make the sleep time configurable, because in some scenarios, the new workers may spend more than 100ms to get ready. For example, the init_prcess hook of some 3rd modules may be time-consuming. diff -r a39bc74873fa -r 8b9e416ef7f9 src/core/nginx.c --- a/src/core/nginx.c Mon Jun 19 14:25:42 2017 +0300 +++ b/src/core/nginx.c Mon Jun 19 14:33:03 2017 + @@ -152,6 +152,13 @@ 0, NULL }, +{ ngx_string("reload_delay"), + NGX_MAIN_CONF|NGX_DIRECT_CONF|NGX_CONF_TAKE1, + ngx_conf_set_msec_slot, + 0, + offsetof(ngx_core_conf_t, reload_delay), + NULL }, + ngx_null_command }; @@ -1022,6 +1029,7 @@ ccf->master = NGX_CONF_UNSET; ccf->timer_resolution = NGX_CONF_UNSET_MSEC; ccf->shutdown_timeout = NGX_CONF_UNSET_MSEC; +ccf->reload_delay = NGX_CONF_UNSET_MSEC; ccf->worker_processes = NGX_CONF_UNSET; ccf->debug_points = NGX_CONF_UNSET; @@ -1051,6 +1059,7 @@ ngx_conf_init_value(ccf->master, 1); ngx_conf_init_msec_value(ccf->timer_resolution, 0); ngx_conf_init_msec_value(ccf->shutdown_timeout, 0); +ngx_conf_init_msec_value(ccf->reload_delay, 100); ngx_conf_init_value(ccf->worker_processes, 1); ngx_conf_init_value(ccf->debug_points, 0); diff -r a39bc74873fa -r 8b9e416ef7f9 src/core/ngx_cycle.h --- a/src/core/ngx_cycle.h Mon Jun 19 14:25:42 2017 +0300 +++ b/src/core/ngx_cycle.h Mon Jun 19 14:33:03 2017 + @@ -89,6 +89,7 @@ ngx_msec_ttimer_resolution; ngx_msec_tshutdown_timeout; +ngx_msec_treload_delay; ngx_int_t worker_processes; ngx_int_t debug_points; diff -r a39bc74873fa -r 8b9e416ef7f9 src/os/unix/ngx_process_cycle.c --- a/src/os/unix/ngx_process_cycle.c Mon Jun 19 14:25:42 2017 +0300 +++ b/src/os/unix/ngx_process_cycle.c Mon Jun 19 14:33:03 2017 + @@ -245,7 +245,7 @@ ngx_start_cache_manager_processes(cycle, 1); /* allow new processes to start */ -ngx_msleep(100); +ngx_msleep(ccf->reload_delay); live = 1; ngx_signal_worker_processes(cycle, ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: [PATCH] Avoid using the result of i2d_SSL_SESSION when the session is invalid
Hello! On Mon, Jun 19, 2017 at 04:09:43PM +0200, Bart Warmerdam wrote: > According to the man-page of i2d_SSL_SESSION the result can be NULL or > 0, but case the actual result can also be -1 in case of a failed > CRYPTO_malloc. The call trace for this function is: > > Call chain: > i2d_SSL_SESSION > i2d_SSL_SESSION_ASN1 > ASN1_item_i2d > asn1_item_flags_i2d > > > The preprocessor output generates the following code: > > static int asn1_item_flags_i2d(ASN1_VALUE *val, unsigned char **out, > const ASN1_ITEM *it, int flags) > { > if (out && !*out) { This condition cannot be true, as nginx uses preallocated buffer for i2d_SSL_SESSION(). (Moreover, using a preallocated buffer is this is the only approach documented in the i2d_SSL_SESSION() manual page, and the only one actually available before OpenSSL 1.1.0.) [...] -- Maxim Dounin http://nginx.org/ ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
[PATCH] HTTP/2: add debug logging of control frames
# HG changeset patch # User Piotr Sikora# Date 1490516711 25200 # Sun Mar 26 01:25:11 2017 -0700 # Node ID 1f1549823fba355a0dd1af49108be4b4898bf331 # Parent d1816a2696de8c2faa1cd913a151e5f62a8620f3 HTTP/2: add debug logging of control frames. Signed-off-by: Piotr Sikora diff -r d1816a2696de -r 1f1549823fba src/http/v2/ngx_http_v2.c --- a/src/http/v2/ngx_http_v2.c +++ b/src/http/v2/ngx_http_v2.c @@ -41,9 +41,11 @@ /* settings fields */ #define NGX_HTTP_V2_HEADER_TABLE_SIZE_SETTING0x1 +#define NGX_HTTP_V2_ENABLE_PUSH_SETTING 0x2 #define NGX_HTTP_V2_MAX_STREAMS_SETTING 0x3 #define NGX_HTTP_V2_INIT_WINDOW_SIZE_SETTING 0x4 #define NGX_HTTP_V2_MAX_FRAME_SIZE_SETTING 0x5 +#define NGX_HTTP_V2_HEADER_LIST_SIZE_SETTING 0x6 #define NGX_HTTP_V2_FRAME_BUFFER_SIZE24 @@ -1946,6 +1948,9 @@ ngx_http_v2_state_settings(ngx_http_v2_c return ngx_http_v2_connection_error(h2c, NGX_HTTP_V2_SIZE_ERROR); } +ngx_log_debug0(NGX_LOG_DEBUG_HTTP, h2c->connection->log, 0, + "http2 SETTINGS frame ack:1"); + h2c->settings_ack = 1; return ngx_http_v2_state_complete(h2c, pos, end); @@ -1959,6 +1964,10 @@ ngx_http_v2_state_settings(ngx_http_v2_c return ngx_http_v2_connection_error(h2c, NGX_HTTP_V2_SIZE_ERROR); } +ngx_log_debug1(NGX_LOG_DEBUG_HTTP, h2c->connection->log, 0, + "http2 SETTINGS frame params:%uz", + h2c->state.length / NGX_HTTP_V2_SETTINGS_PARAM_SIZE); + return ngx_http_v2_state_settings_params(h2c, pos, end); } @@ -1986,6 +1995,27 @@ ngx_http_v2_state_settings_params(ngx_ht switch (id) { +case NGX_HTTP_V2_HEADER_TABLE_SIZE_SETTING: + +ngx_log_debug1(NGX_LOG_DEBUG_HTTP, h2c->connection->log, 0, + "http2 SETTINGS param HEADER_TABLE_SIZE:%ui " + "(ignored)", value); +break; + +case NGX_HTTP_V2_ENABLE_PUSH_SETTING: + +ngx_log_debug1(NGX_LOG_DEBUG_HTTP, h2c->connection->log, 0, + "http2 SETTINGS param ENABLE_PUSH:%ui " + "(ignored)", value); +break; + +case NGX_HTTP_V2_MAX_STREAMS_SETTING: + +ngx_log_debug1(NGX_LOG_DEBUG_HTTP, h2c->connection->log, 0, + "http2 SETTINGS param MAX_CONCURRENT_STREAMS:%ui " + "(ignored)", value); +break; + case NGX_HTTP_V2_INIT_WINDOW_SIZE_SETTING: if (value > NGX_HTTP_V2_MAX_WINDOW) { @@ -1997,6 +2027,10 @@ ngx_http_v2_state_settings_params(ngx_ht NGX_HTTP_V2_FLOW_CTRL_ERROR); } +ngx_log_debug1(NGX_LOG_DEBUG_HTTP, h2c->connection->log, 0, + "http2 SETTINGS param INITIAL_WINDOW_SIZE:%ui", + value); + window_delta = value - h2c->init_window; h2c->init_window = value; @@ -2015,16 +2049,34 @@ ngx_http_v2_state_settings_params(ngx_ht NGX_HTTP_V2_PROTOCOL_ERROR); } +ngx_log_debug1(NGX_LOG_DEBUG_HTTP, h2c->connection->log, 0, + "http2 SETTINGS param MAX_FRAME_SIZE:%ui", + value); + h2c->frame_size = value; break; +case NGX_HTTP_V2_HEADER_LIST_SIZE_SETTING: + +ngx_log_debug1(NGX_LOG_DEBUG_HTTP, h2c->connection->log, 0, + "http2 SETTINGS param MAX_HEADER_LIST_SIZE:%ui " + "(ignored)", value); +break; + default: + +ngx_log_debug2(NGX_LOG_DEBUG_HTTP, h2c->connection->log, 0, + "http2 SETTINGS param 0x%Xi:%ui " + "(ignored)", id, value); break; } pos += NGX_HTTP_V2_SETTINGS_PARAM_SIZE; } +ngx_log_debug0(NGX_LOG_DEBUG_HTTP, h2c->connection->log, 0, + "http2 send SETTINGS frame ack:1"); + frame = ngx_http_v2_get_frame(h2c, NGX_HTTP_V2_SETTINGS_ACK_SIZE, NGX_HTTP_V2_SETTINGS_FRAME, NGX_HTTP_V2_ACK_FLAG, 0); @@ -2075,12 +2127,16 @@ ngx_http_v2_state_ping(ngx_http_v2_conne } ngx_log_debug1(NGX_LOG_DEBUG_HTTP, h2c->connection->log, 0, - "http2 PING frame, flags: %ud", h2c->state.flags); + "http2 PING frame ack:%ud", + h2c->state.flags & NGX_HTTP_V2_ACK_FLAG ? 1 : 0); if (h2c->state.flags & NGX_HTTP_V2_ACK_FLAG) { return ngx_http_v2_state_skip(h2c, pos, end); } +ngx_log_debug0(NGX_LOG_DEBUG_HTTP, h2c->connection->log, 0, + "http2 send PING frame ack:1"); + frame =
Re: [PATCH] HTTP/2: add debug logging of control frames
Hey Valentin, > Ok, I've already resigned myself to multiline output, but don't let it > look like an another SETTINGS frame. > > IMHO, something like that will be good enough: > >http2 send SETTINGS frame >http2 SETTINGS param MAX_CONCURRENT_STREAMS: 100 >http2 SETTINGS param INITIAL_WINDOW_SIZE: 65536 >http2 SETTINGS param MAX_FRAME_SIZE: 16777215 Done, with retained "send " prefix to differentiate params that we send and receive. I've also re-added params counter to the "send SETTINGS frame", by setting "len" value a bit sooner, so that the number of params is calculated and cannot be forgotten in subsequent commits. Best regards, Piotr Sikora ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: [PATCH] Avoid using the result of i2d_SSL_SESSION when the session is invalid
According to the man-page of i2d_SSL_SESSION the result can be NULL or 0, but case the actual result can also be -1 in case of a failed CRYPTO_malloc. The call trace for this function is: Call chain: i2d_SSL_SESSION i2d_SSL_SESSION_ASN1 ASN1_item_i2d asn1_item_flags_i2d The preprocessor output generates the following code: static int asn1_item_flags_i2d(ASN1_VALUE *val, unsigned char **out, const ASN1_ITEM *it, int flags) { if (out && !*out) { unsigned char *p, *buf; int len; len = ASN1_item_ex_i2d(, # 60 "crypto/asn1/tasn_enc.c" 3 4 ((void *)0) # 60 "crypto/asn1/tasn_enc.c" , it, -1, flags); if (len <= 0) return len; buf = CRYPTO_malloc(len, "crypto/asn1/tasn_enc.c", 63); if (buf == # 64 "crypto/asn1/tasn_enc.c" 3 4 ((void *)0) # 64 "crypto/asn1/tasn_enc.c" ) return -1; p = buf; ASN1_item_ex_i2d(, , it, -1, flags); *out = buf; return len; } return ASN1_item_ex_i2d(, out, it, -1, flags); } As you can see around line 65 the -1 is returned once the allocation fails, which is then directly returned in all intermediate calls up to i2d_SSL_SESSION. So the -1 case is missed once the current "if statement" is kept without the "len < 1" or-statement. Regards, B. On 2017-06-19 12:10, Ruslan Ermilov wrote: On Mon, Jun 19, 2017 at 08:08:38AM +0200, Bart Warmerdam wrote: # HG changeset patch # User Bart Warmerdam# Date 1497852211 -7200 # Mon Jun 19 08:03:31 2017 +0200 # Branch i2d_ssl_session_length # Node ID 079afb2cb4be3ef06d07e96d1a54cc359b971631 # Parent d1816a2696de8c2faa1cd913a151e5f62a8620f3 Make sure to also take into account the 'return 0' response of i2d_SSL_SESSION, which is possible when the session is not valid A case of invalid session is already caught by checking the return value of SSL_get0_session() just prior to calling i2d_SSL_SESSION() in ngx_http_upstream_save_round_robin_peer_session() and ngx_stream_upstream_save_round_robin_peer_session(). The ngx_ssl_new_session() function is passed as an argument to SSL_CTX_sess_set_new_cb() that "sets the callback function, which is automatically called whenever a new session was negotiated." Do you think that it's possible for a session to be invalid in either of these cases? diff -r d1816a2696de -r 079afb2cb4be src/event/ngx_event_openssl.c --- a/src/event/ngx_event_openssl.c Fri Jun 16 18:15:58 2017 +0300 +++ b/src/event/ngx_event_openssl.c Mon Jun 19 08:03:31 2017 +0200 @@ -2458,9 +2458,9 @@ len = i2d_SSL_SESSION(sess, NULL); -/* do not cache too big session */ - -if (len > (int) NGX_SSL_MAX_SESSION_SIZE) { +/* do not cache too big or invalid session */ + +if (len > (int) NGX_SSL_MAX_SESSION_SIZE || len < 1) { return 0; } diff -r d1816a2696de -r 079afb2cb4be src/http/ngx_http_upstream_round_robin.c --- a/src/http/ngx_http_upstream_round_robin.c Fri Jun 16 18:15:58 2017 +0300 +++ b/src/http/ngx_http_upstream_round_robin.c Mon Jun 19 08:03:31 2017 +0200 @@ -755,9 +755,9 @@ len = i2d_SSL_SESSION(ssl_session, NULL); -/* do not cache too big session */ +/* do not cache too big or invalid session */ -if (len > NGX_SSL_MAX_SESSION_SIZE) { + if (len > NGX_SSL_MAX_SESSION_SIZE || len < 1) { return; } diff -r d1816a2696de -r 079afb2cb4be src/stream/ngx_stream_upstream_round_robin.c --- a/src/stream/ngx_stream_upstream_round_robin.c Fri Jun 16 18:15:58 2017 +0300 +++ b/src/stream/ngx_stream_upstream_round_robin.c Mon Jun 19 08:03:31 2017 +0200 @@ -787,9 +787,9 @@ len = i2d_SSL_SESSION(ssl_session, NULL); -/* do not cache too big session */ +/* do not cache too big or invalid session */ -if (len > NGX_SSL_MAX_SESSION_SIZE) { +if (len > NGX_SSL_MAX_SESSION_SIZE || len < 1) { return; } ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: [PATCH 4 of 4] HTTP/2: reject HTTP/2 requests with connection-specific headers
Hello! On Sat, Jun 17, 2017 at 01:57:38PM -0700, Piotr Sikora via nginx-devel wrote: [...] > > Unless there are practical reasons for these changes, I would > > rather reject the series. > > The practical reason is that other implementations (e.g. nghttp2) > reject requests with those headers, which leads to a weird behavior > where NGINX accepts requests and proxies them to a HTTP/2 upstream > which rejects them because they contain one of those headers. > > We could clear those headers in proxy module (I'm already doing that > for most of the headers, anyway), but it feels like a workaround for > broken clients. We anyway have to remove hop-by-hop headers from HTTP/1.x connections. I don't see how HTTP/2 can be different, specially if one side uses HTTP/1.x and another one uses HTTP/2. Accordingly, if an upstream server rejects a request, there are two possible reasons: - we've forgot to remove something we have to (that is, there is a bug in nginx); - a client sent something it shouldn't (that is, there is a bug in the client). In either case returing the error to the client, as it will naturally happen, looks fine to me. > Having said that, I'm fine with dropping the whole patchset. Yes, please. -- Maxim Dounin http://nginx.org/ ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
[njs] Object.preventExtensions() method.
details: http://hg.nginx.org/njs/rev/4d5a5d618fca branches: changeset: 367:4d5a5d618fca user: Dmitry Volyntsevdate: Mon Jun 19 14:40:14 2017 +0300 description: Object.preventExtensions() method. diffstat: njs/njs_object.c | 25 + njs/test/njs_unit_test.c | 17 + 2 files changed, 42 insertions(+), 0 deletions(-) diffs (69 lines): diff -r 824fbb7fcd35 -r 4d5a5d618fca njs/njs_object.c --- a/njs/njs_object.c Mon Jun 19 14:39:56 2017 +0300 +++ b/njs/njs_object.c Mon Jun 19 14:40:14 2017 +0300 @@ -754,6 +754,23 @@ njs_object_freeze(njs_vm_t *vm, njs_valu } +static njs_ret_t +njs_object_prevent_extensions(njs_vm_t *vm, njs_value_t *args, nxt_uint_t nargs, +njs_index_t unused) +{ +if (nargs < 2 || !njs_is_object([1])) { +vm->exception = _exception_type_error; +return NXT_ERROR; +} + +args[1].data.u.object->extensible = 0; + +vm->retval = args[1]; + +return NXT_OK; +} + + /* * The __proto__ property of booleans, numbers and strings primitives, * of objects created by Boolean(), Number(), and String() constructors, @@ -939,6 +956,14 @@ static const njs_object_prop_t njs_obje .value = njs_native_function(njs_object_freeze, 0, NJS_SKIP_ARG, NJS_OBJECT_ARG), }, + +/* Object.preventExtensions(). */ +{ +.type = NJS_METHOD, +.name = njs_long_string("preventExtensions"), +.value = njs_native_function(njs_object_prevent_extensions, 0, + NJS_SKIP_ARG, NJS_OBJECT_ARG), +}, }; diff -r 824fbb7fcd35 -r 4d5a5d618fca njs/test/njs_unit_test.c --- a/njs/test/njs_unit_test.c Mon Jun 19 14:39:56 2017 +0300 +++ b/njs/test/njs_unit_test.c Mon Jun 19 14:40:14 2017 +0300 @@ -6164,6 +6164,23 @@ static njs_unit_test_t njs_test[] = { nxt_string("var r = Object.freeze(new RegExp('')); r.a = 1; r.a"), nxt_string("undefined") }, +{ nxt_string("var o = Object.preventExtensions({a:1});" + "Object.defineProperty(o, 'b', {value:1})"), + nxt_string("TypeError") }, + +{ nxt_string("var o = Object.preventExtensions({a:1});" + "Object.defineProperties(o, {b:{value:1}})"), + nxt_string("TypeError") }, + +{ nxt_string("var o = Object.preventExtensions({a:1}); o.a = 2; o.a"), + nxt_string("2") }, + +{ nxt_string("var o = Object.preventExtensions({a:1}); delete o.a; o.a"), + nxt_string("undefined") }, + +{ nxt_string("var o = Object.preventExtensions({a:1}); o.b = 1; o.b"), + nxt_string("undefined") }, + { nxt_string("var d = new Date(''); d +' '+ d.getTime()"), nxt_string("Invalid Date NaN") }, ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
[njs] Object.seal() method.
details: http://hg.nginx.org/njs/rev/cc5ab912d455 branches: changeset: 370:cc5ab912d455 user: Dmitry Volyntsevdate: Mon Jun 19 14:46:39 2017 +0300 description: Object.seal() method. diffstat: njs/njs_object.c | 45 + njs/test/njs_unit_test.c | 21 + 2 files changed, 66 insertions(+), 0 deletions(-) diffs (93 lines): diff -r bd6c05f66ea9 -r cc5ab912d455 njs/njs_object.c --- a/njs/njs_object.c Mon Jun 19 14:46:34 2017 +0300 +++ b/njs/njs_object.c Mon Jun 19 14:46:39 2017 +0300 @@ -803,6 +803,43 @@ done: static njs_ret_t +njs_object_seal(njs_vm_t *vm, njs_value_t *args, nxt_uint_t nargs, +njs_index_t unused) +{ +nxt_lvlhsh_t *hash; +njs_object_t *object; +njs_object_prop_t *prop; +nxt_lvlhsh_each_t lhe; + +if (nargs < 2 || !njs_is_object([1])) { +vm->exception = _exception_type_error; +return NXT_ERROR; +} + +object = args[1].data.u.object; +object->extensible = 0; + +nxt_lvlhsh_each_init(, _object_hash_proto); + +hash = >hash; + +for ( ;; ) { +prop = nxt_lvlhsh_each(hash, ); + +if (prop == NULL) { +break; +} + +prop->configurable = 0; +} + +vm->retval = args[1]; + +return NXT_OK; +} + + +static njs_ret_t njs_object_prevent_extensions(njs_vm_t *vm, njs_value_t *args, nxt_uint_t nargs, njs_index_t unused) { @@ -1033,6 +1070,14 @@ static const njs_object_prop_t njs_obje NJS_SKIP_ARG, NJS_OBJECT_ARG), }, +/* Object.seal(). */ +{ +.type = NJS_METHOD, +.name = njs_string("seal"), +.value = njs_native_function(njs_object_seal, 0, + NJS_SKIP_ARG, NJS_OBJECT_ARG), +}, + /* Object.preventExtensions(). */ { .type = NJS_METHOD, diff -r bd6c05f66ea9 -r cc5ab912d455 njs/test/njs_unit_test.c --- a/njs/test/njs_unit_test.c Mon Jun 19 14:46:34 2017 +0300 +++ b/njs/test/njs_unit_test.c Mon Jun 19 14:46:39 2017 +0300 @@ -6215,6 +6215,27 @@ static njs_unit_test_t njs_test[] = { nxt_string("var o = Object.freeze({a:1}); Object.isFrozen(o)"), nxt_string("true") }, +{ nxt_string("var o = Object.seal({a:1}); o.a = 2; o.a"), + nxt_string("2") }, + +{ nxt_string("var o = Object.seal({a:1}); delete o.a; o.a"), + nxt_string("1") }, + +{ nxt_string("var o = Object.seal({a:1}); o.b = 1; o.b"), + nxt_string("undefined") }, + +{ nxt_string("var o = Object.seal(Object.create({a:1})); o.a = 2; o.a"), + nxt_string("1") }, + +{ nxt_string("var o = Object.seal({a:{b:1}}); o.a.b = 2; o.a.b"), + nxt_string("2") }, + +{ nxt_string("Object.seal(1)"), + nxt_string("TypeError") }, + +{ nxt_string("Object.seal('')"), + nxt_string("TypeError") }, + { nxt_string("var o = Object.preventExtensions({a:1});" "Object.defineProperty(o, 'b', {value:1})"), nxt_string("TypeError") }, ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
[njs] Object.isExtensible() method.
details: http://hg.nginx.org/njs/rev/bd6c05f66ea9 branches: changeset: 369:bd6c05f66ea9 user: Dmitry Volyntsevdate: Mon Jun 19 14:46:34 2017 +0300 description: Object.isExtensible() method. diffstat: njs/njs_object.c | 28 njs/test/njs_unit_test.c | 33 + 2 files changed, 61 insertions(+), 0 deletions(-) diffs (88 lines): diff -r b2ccd7673a5e -r bd6c05f66ea9 njs/njs_object.c --- a/njs/njs_object.c Mon Jun 19 14:41:03 2017 +0300 +++ b/njs/njs_object.c Mon Jun 19 14:46:34 2017 +0300 @@ -819,6 +819,26 @@ njs_object_prevent_extensions(njs_vm_t * } +static njs_ret_t +njs_object_is_extensible(njs_vm_t *vm, njs_value_t *args, nxt_uint_t nargs, +njs_index_t unused) +{ +const njs_value_t *retval; + +if (nargs < 2 || !njs_is_object([1])) { +vm->exception = _exception_type_error; +return NXT_ERROR; +} + +retval = args[1].data.u.object->extensible ? _string_true : + _string_false; + +vm->retval = *retval; + +return NXT_OK; +} + + /* * The __proto__ property of booleans, numbers and strings primitives, * of objects created by Boolean(), Number(), and String() constructors, @@ -1020,6 +1040,14 @@ static const njs_object_prop_t njs_obje .value = njs_native_function(njs_object_prevent_extensions, 0, NJS_SKIP_ARG, NJS_OBJECT_ARG), }, + +/* Object.isExtensible(). */ +{ +.type = NJS_METHOD, +.name = njs_string("isExtensible"), +.value = njs_native_function(njs_object_is_extensible, 0, + NJS_SKIP_ARG, NJS_OBJECT_ARG), +}, }; diff -r b2ccd7673a5e -r bd6c05f66ea9 njs/test/njs_unit_test.c --- a/njs/test/njs_unit_test.c Mon Jun 19 14:41:03 2017 +0300 +++ b/njs/test/njs_unit_test.c Mon Jun 19 14:46:34 2017 +0300 @@ -6232,6 +6232,39 @@ static njs_unit_test_t njs_test[] = { nxt_string("var o = Object.preventExtensions({a:1}); o.b = 1; o.b"), nxt_string("undefined") }, +{ nxt_string("Object.isExtensible({})"), + nxt_string("true") }, + +{ nxt_string("Object.isExtensible([])"), + nxt_string("true") }, + +{ nxt_string("Object.isExtensible(function() {})"), + nxt_string("true") }, + +{ nxt_string("Object.isExtensible(new Date(''))"), + nxt_string("true") }, + +{ nxt_string("Object.isExtensible(new RegExp(''))"), + nxt_string("true") }, + +{ nxt_string("Object.isExtensible(1)"), + nxt_string("TypeError") }, + +{ nxt_string("Object.isExtensible('')"), + nxt_string("TypeError") }, + +{ nxt_string("Object.isExtensible(Object.preventExtensions({}))"), + nxt_string("false") }, + +{ nxt_string("Object.isExtensible(Object.preventExtensions([]))"), + nxt_string("false") }, + +{ nxt_string("Object.isExtensible(Object.freeze({}))"), + nxt_string("false") }, + +{ nxt_string("Object.isExtensible(Object.freeze([]))"), + nxt_string("false") }, + { nxt_string("var d = new Date(''); d +' '+ d.getTime()"), nxt_string("Invalid Date NaN") }, ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
[njs] Object.isSealed() method.
details: http://hg.nginx.org/njs/rev/bcd7a7256805 branches: changeset: 371:bcd7a7256805 user: Dmitry Volyntsevdate: Mon Jun 19 14:46:46 2017 +0300 description: Object.isSealed() method. diffstat: njs/njs_object.c | 56 njs/test/njs_unit_test.c | 51 +++ 2 files changed, 107 insertions(+), 0 deletions(-) diffs (134 lines): diff -r cc5ab912d455 -r bcd7a7256805 njs/njs_object.c --- a/njs/njs_object.c Mon Jun 19 14:46:39 2017 +0300 +++ b/njs/njs_object.c Mon Jun 19 14:46:46 2017 +0300 @@ -840,6 +840,54 @@ njs_object_seal(njs_vm_t *vm, njs_value_ static njs_ret_t +njs_object_is_sealed(njs_vm_t *vm, njs_value_t *args, nxt_uint_t nargs, +njs_index_t unused) +{ +nxt_lvlhsh_t *hash; +njs_object_t *object; +njs_object_prop_t *prop; +nxt_lvlhsh_each_t lhe; +const njs_value_t *retval; + +if (nargs < 2 || !njs_is_object([1])) { +vm->exception = _exception_type_error; +return NXT_ERROR; +} + +retval = _string_false; + +object = args[1].data.u.object; +nxt_lvlhsh_each_init(, _object_hash_proto); + +hash = >hash; + +if (object->extensible) { +goto done; +} + +for ( ;; ) { +prop = nxt_lvlhsh_each(hash, ); + +if (prop == NULL) { +break; +} + +if (prop->writable) { +goto done; +} +} + +retval = _string_true; + +done: + +vm->retval = *retval; + +return NXT_OK; +} + + +static njs_ret_t njs_object_prevent_extensions(njs_vm_t *vm, njs_value_t *args, nxt_uint_t nargs, njs_index_t unused) { @@ -1078,6 +1126,14 @@ static const njs_object_prop_t njs_obje NJS_SKIP_ARG, NJS_OBJECT_ARG), }, +/* Object.isSealed(). */ +{ +.type = NJS_METHOD, +.name = njs_string("isSealed"), +.value = njs_native_function(njs_object_is_sealed, 0, + NJS_SKIP_ARG, NJS_OBJECT_ARG), +}, + /* Object.preventExtensions(). */ { .type = NJS_METHOD, diff -r cc5ab912d455 -r bcd7a7256805 njs/test/njs_unit_test.c --- a/njs/test/njs_unit_test.c Mon Jun 19 14:46:39 2017 +0300 +++ b/njs/test/njs_unit_test.c Mon Jun 19 14:46:46 2017 +0300 @@ -6236,6 +6236,57 @@ static njs_unit_test_t njs_test[] = { nxt_string("Object.seal('')"), nxt_string("TypeError") }, +{ nxt_string("Object.isSealed({a:1})"), + nxt_string("false") }, + +{ nxt_string("Object.isSealed([1,2])"), + nxt_string("false") }, + +{ nxt_string("Object.isSealed(function() {})"), + nxt_string("false") }, + +{ nxt_string("Object.isSealed(new Date(''))"), + nxt_string("false") }, + +{ nxt_string("Object.isSealed(new RegExp(''))"), + nxt_string("false") }, + +{ nxt_string("Object.isSealed(1)"), + nxt_string("TypeError") }, + +{ nxt_string("Object.isSealed('')"), + nxt_string("TypeError") }, + +{ nxt_string("Object.isSealed(Object.defineProperties({}, {a:{value:1}}))"), + nxt_string("false") }, + +{ nxt_string("var o = Object.defineProperties({}, {a:{}, b:{}});" + "o = Object.preventExtensions(o);" + "Object.isSealed(o)"), + nxt_string("true") }, + +{ nxt_string("var o = Object.defineProperties({}, {a:{}, b:{writable:1}});" + "o = Object.preventExtensions(o);" + "Object.isSealed(o)"), + nxt_string("false") }, + +{ nxt_string("var o = Object.defineProperties({}, {a:{writable:1}});" + "o = Object.preventExtensions(o);" + "Object.isSealed(o)"), + nxt_string("false") }, + +{ nxt_string("var o = Object.defineProperties({}, {a:{configurable:1}});" + "o = Object.preventExtensions(o);" + "Object.isSealed(o)"), + nxt_string("true") }, + +{ nxt_string("var o = Object.preventExtensions({a:1});" + "Object.isFrozen(o)"), + nxt_string("false") }, + +{ nxt_string("var o = Object.freeze({a:1}); Object.isFrozen(o)"), + nxt_string("true") }, + { nxt_string("var o = Object.preventExtensions({a:1});" "Object.defineProperty(o, 'b', {value:1})"), nxt_string("TypeError") }, ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
[njs] Object.isFrozen() method.
details: http://hg.nginx.org/njs/rev/b2ccd7673a5e branches: changeset: 368:b2ccd7673a5e user: Dmitry Volyntsevdate: Mon Jun 19 14:41:03 2017 +0300 description: Object.isFrozen() method. diffstat: njs/njs_object.c | 56 njs/test/njs_unit_test.c | 51 +++ 2 files changed, 107 insertions(+), 0 deletions(-) diffs (134 lines): diff -r 4d5a5d618fca -r b2ccd7673a5e njs/njs_object.c --- a/njs/njs_object.c Mon Jun 19 14:40:14 2017 +0300 +++ b/njs/njs_object.c Mon Jun 19 14:41:03 2017 +0300 @@ -755,6 +755,54 @@ njs_object_freeze(njs_vm_t *vm, njs_valu static njs_ret_t +njs_object_is_frozen(njs_vm_t *vm, njs_value_t *args, nxt_uint_t nargs, +njs_index_t unused) +{ +nxt_lvlhsh_t *hash; +njs_object_t *object; +njs_object_prop_t *prop; +nxt_lvlhsh_each_t lhe; +const njs_value_t *retval; + +if (nargs < 2 || !njs_is_object([1])) { +vm->exception = _exception_type_error; +return NXT_ERROR; +} + +retval = _string_false; + +object = args[1].data.u.object; +nxt_lvlhsh_each_init(, _object_hash_proto); + +hash = >hash; + +if (object->extensible) { +goto done; +} + +for ( ;; ) { +prop = nxt_lvlhsh_each(hash, ); + +if (prop == NULL) { +break; +} + +if (prop->writable || prop->configurable) { +goto done; +} +} + +retval = _string_true; + +done: + +vm->retval = *retval; + +return NXT_OK; +} + + +static njs_ret_t njs_object_prevent_extensions(njs_vm_t *vm, njs_value_t *args, nxt_uint_t nargs, njs_index_t unused) { @@ -957,6 +1005,14 @@ static const njs_object_prop_t njs_obje NJS_SKIP_ARG, NJS_OBJECT_ARG), }, +/* Object.isFrozen(). */ +{ +.type = NJS_METHOD, +.name = njs_string("isFrozen"), +.value = njs_native_function(njs_object_is_frozen, 0, + NJS_SKIP_ARG, NJS_OBJECT_ARG), +}, + /* Object.preventExtensions(). */ { .type = NJS_METHOD, diff -r 4d5a5d618fca -r b2ccd7673a5e njs/test/njs_unit_test.c --- a/njs/test/njs_unit_test.c Mon Jun 19 14:40:14 2017 +0300 +++ b/njs/test/njs_unit_test.c Mon Jun 19 14:41:03 2017 +0300 @@ -6164,6 +6164,57 @@ static njs_unit_test_t njs_test[] = { nxt_string("var r = Object.freeze(new RegExp('')); r.a = 1; r.a"), nxt_string("undefined") }, +{ nxt_string("Object.isFrozen({a:1})"), + nxt_string("false") }, + +{ nxt_string("Object.isFrozen([1,2])"), + nxt_string("false") }, + +{ nxt_string("Object.isFrozen(function() {})"), + nxt_string("false") }, + +{ nxt_string("Object.isFrozen(new Date(''))"), + nxt_string("false") }, + +{ nxt_string("Object.isFrozen(new RegExp(''))"), + nxt_string("false") }, + +{ nxt_string("Object.isFrozen(1)"), + nxt_string("TypeError") }, + +{ nxt_string("Object.isFrozen('')"), + nxt_string("TypeError") }, + +{ nxt_string("Object.isFrozen(Object.defineProperties({}, {a:{value:1}}))"), + nxt_string("false") }, + +{ nxt_string("var o = Object.defineProperties({}, {a:{}, b:{}});" + "o = Object.preventExtensions(o);" + "Object.isFrozen(o)"), + nxt_string("true") }, + +{ nxt_string("var o = Object.defineProperties({}, {a:{}, b:{writable:1}});" + "o = Object.preventExtensions(o);" + "Object.isFrozen(o)"), + nxt_string("false") }, + +{ nxt_string("var o = Object.defineProperties({}, {a:{writable:1}});" + "o = Object.preventExtensions(o);" + "Object.isFrozen(o)"), + nxt_string("false") }, + +{ nxt_string("var o = Object.defineProperties({}, {a:{configurable:1}});" + "o = Object.preventExtensions(o);" + "Object.isFrozen(o)"), + nxt_string("false") }, + +{ nxt_string("var o = Object.preventExtensions({a:1});" + "Object.isFrozen(o)"), + nxt_string("false") }, + +{ nxt_string("var o = Object.freeze({a:1}); Object.isFrozen(o)"), + nxt_string("true") }, + { nxt_string("var o = Object.preventExtensions({a:1});" "Object.defineProperty(o, 'b', {value:1})"), nxt_string("TypeError") }, ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
[njs] Object.freeze() method.
details: http://hg.nginx.org/njs/rev/824fbb7fcd35 branches: changeset: 366:824fbb7fcd35 user: Dmitry Volyntsevdate: Mon Jun 19 14:39:56 2017 +0300 description: Object.freeze() method. diffstat: njs/njs_array.c | 3 +- njs/njs_builtin.c| 2 + njs/njs_date.c | 1 + njs/njs_function.c | 2 + njs/njs_object.c | 58 + njs/njs_regexp.c | 1 + njs/njs_vm.c | 5 ++ njs/njs_vm.h | 7 ++- njs/test/njs_unit_test.c | 93 9 files changed, 169 insertions(+), 3 deletions(-) diffs (332 lines): diff -r 7ed74a2e4c50 -r 824fbb7fcd35 njs/njs_array.c --- a/njs/njs_array.c Wed Jun 14 17:58:10 2017 +0300 +++ b/njs/njs_array.c Mon Jun 19 14:39:56 2017 +0300 @@ -149,6 +149,7 @@ njs_array_alloc(njs_vm_t *vm, uint32_t l array->object.__proto__ = >prototypes[NJS_PROTOTYPE_ARRAY].object; array->object.type = NJS_ARRAY; array->object.shared = 0; +array->object.extensible = 1; array->size = size; array->length = length; @@ -1941,7 +1942,7 @@ njs_array_string_sort(njs_vm_t *vm, njs_ static const njs_function_t njs_array_string_sort_function = { -.object.shared = 1, +.object = { .type = NJS_FUNCTION, .shared = 1, .extensible = 1 }, .native = 1, .continuation_size = NJS_CONTINUATION_SIZE, .args_types = { NJS_SKIP_ARG, NJS_STRING_ARG, NJS_STRING_ARG }, diff -r 7ed74a2e4c50 -r 824fbb7fcd35 njs/njs_builtin.c --- a/njs/njs_builtin.c Wed Jun 14 17:58:10 2017 +0300 +++ b/njs/njs_builtin.c Mon Jun 19 14:39:56 2017 +0300 @@ -206,6 +206,7 @@ njs_builtin_objects_create(njs_vm_t *vm) } functions[i].object.shared = 1; +functions[i].object.extensible = 1; functions[i].native = 1; functions[i].args_offset = 1; functions[i].u.native = native_functions[i].native; @@ -236,6 +237,7 @@ njs_builtin_objects_create(njs_vm_t *vm) for (i = NJS_CONSTRUCTOR_OBJECT; i < NJS_CONSTRUCTOR_MAX; i++) { constructors[i].object.shared = 0; +constructors[i].object.extensible = 1; constructors[i].native = 1; constructors[i].ctor = 1; constructors[i].args_offset = 1; diff -r 7ed74a2e4c50 -r 824fbb7fcd35 njs/njs_date.c --- a/njs/njs_date.cWed Jun 14 17:58:10 2017 +0300 +++ b/njs/njs_date.cMon Jun 19 14:39:56 2017 +0300 @@ -154,6 +154,7 @@ njs_date_constructor(njs_vm_t *vm, njs_v nxt_lvlhsh_init(>object.shared_hash); date->object.type = NJS_DATE; date->object.shared = 0; +date->object.extensible = 1; date->object.__proto__ = >prototypes[NJS_PROTOTYPE_DATE].object; date->time = time; diff -r 7ed74a2e4c50 -r 824fbb7fcd35 njs/njs_function.c --- a/njs/njs_function.cWed Jun 14 17:58:10 2017 +0300 +++ b/njs/njs_function.cMon Jun 19 14:39:56 2017 +0300 @@ -44,6 +44,7 @@ njs_function_alloc(njs_vm_t *vm) function->object.shared_hash = vm->shared->function_prototype_hash; function->object.type = NJS_FUNCTION; function->object.shared = 1; +function->object.extensible = 1; function->args_offset = 1; function->u.lambda = nxt_mem_cache_zalloc(vm->mem_cache_pool, @@ -635,6 +636,7 @@ njs_function_prototype_bind(njs_vm_t *vm function->object.__proto__ = >prototypes[NJS_PROTOTYPE_FUNCTION].object; function->object.shared = 0; +function->object.extensible = 1; if (nargs == 1) { args = (njs_value_t *) _value_void; diff -r 7ed74a2e4c50 -r 824fbb7fcd35 njs/njs_object.c --- a/njs/njs_object.c Wed Jun 14 17:58:10 2017 +0300 +++ b/njs/njs_object.c Mon Jun 19 14:39:56 2017 +0300 @@ -43,6 +43,7 @@ njs_object_alloc(njs_vm_t *vm) object->__proto__ = >prototypes[NJS_PROTOTYPE_OBJECT].object; object->type = NJS_OBJECT; object->shared = 0; +object->extensible = 1; } return object; @@ -86,6 +87,7 @@ njs_object_value_alloc(njs_vm_t *vm, con nxt_lvlhsh_init(>object.shared_hash); ov->object.type = njs_object_value_type(type); ov->object.shared = 0; +ov->object.extensible = 1; index = njs_primitive_prototype_index(type); ov->object.__proto__ = >prototypes[index].object; @@ -416,6 +418,11 @@ njs_object_define_property(njs_vm_t *vm, return NXT_ERROR; } +if (!args[1].data.u.object->extensible) { +vm->exception = _exception_type_error; +return NXT_ERROR; +} + ret = njs_define_property(vm, args[1].data.u.object, [2], args[3].data.u.object); @@ -444,6 +451,11 @@ njs_object_define_properties(njs_vm_t *v return NXT_ERROR; } +if (!args[1].data.u.object->extensible) { +vm->exception = _exception_type_error; +return NXT_ERROR; +} + nxt_lvlhsh_each_init(,
[njs] Using njs_string_get() where appropriate.
details: http://hg.nginx.org/njs/rev/7ed74a2e4c50 branches: changeset: 365:7ed74a2e4c50 user: Dmitry Volyntsevdate: Wed Jun 14 17:58:10 2017 +0300 description: Using njs_string_get() where appropriate. diffstat: njs/njs_object.c | 11 +-- njs/njs_vm.c | 11 +-- njs/njs_vm.h | 4 ++-- 3 files changed, 4 insertions(+), 22 deletions(-) diffs (59 lines): diff -r cf6b4a543eea -r 7ed74a2e4c50 njs/njs_object.c --- a/njs/njs_object.c Tue Jun 13 17:52:11 2017 +0300 +++ b/njs/njs_object.c Wed Jun 14 17:58:10 2017 +0300 @@ -109,16 +109,7 @@ njs_object_hash_create(njs_vm_t *vm, nxt lhq.pool = vm->mem_cache_pool; do { -lhq.key.length = prop->name.short_string.size; - -if (lhq.key.length != NJS_STRING_LONG) { -lhq.key.start = (u_char *) prop->name.short_string.start; - -} else { -lhq.key.length = prop->name.data.string_size; -lhq.key.start = prop->name.data.u.string->start; -} - +njs_string_get(>name, ); lhq.key_hash = nxt_djb_hash(lhq.key.start, lhq.key.length); lhq.value = (void *) prop; diff -r cf6b4a543eea -r 7ed74a2e4c50 njs/njs_vm.c --- a/njs/njs_vm.c Tue Jun 13 17:52:11 2017 +0300 +++ b/njs/njs_vm.c Wed Jun 14 17:58:10 2017 +0300 @@ -1028,16 +1028,7 @@ njs_property_query(njs_vm_t *vm, njs_pro if (nxt_fast_path(ret == NXT_OK)) { -pq->lhq.key.length = pq->value.short_string.size; - -if (pq->lhq.key.length != NJS_STRING_LONG) { -pq->lhq.key.start = pq->value.short_string.start; - -} else { -pq->lhq.key.length = pq->value.data.string_size; -pq->lhq.key.start = pq->value.data.u.string->start; -} - +njs_string_get(>value, >lhq.key); pq->lhq.key_hash = hash(pq->lhq.key.start, pq->lhq.key.length); if (obj == NULL) { diff -r cf6b4a543eea -r 7ed74a2e4c50 njs/njs_vm.h --- a/njs/njs_vm.h Tue Jun 13 17:52:11 2017 +0300 +++ b/njs/njs_vm.h Wed Jun 14 17:58:10 2017 +0300 @@ -412,11 +412,11 @@ typedef njs_ret_t (*njs_vmcode_operation do { \ if ((value)->short_string.size != NJS_STRING_LONG) { \ (str)->length = (value)->short_string.size; \ -(str)->start = (value)->short_string.start; \ +(str)->start = (u_char *) (value)->short_string.start;\ \ } else { \ (str)->length = (value)->data.string_size;\ -(str)->start = (value)->data.u.string->start; \ +(str)->start = (u_char *) (value)->data.u.string->start; \ } \ } while (0) ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: [PATCH] Avoid memory leak on failure to allocate name during resolving
Hello Bart, On Mon, Jun 19, 2017 at 08:00:37AM +0200, Bart Warmerdam wrote: > > # HG changeset patch > # User Bart Warmerdam> # Date 1497851445 -7200 > # Mon Jun 19 07:50:45 2017 +0200 > # Branch memleak_resolve_name > # Node ID dd8c5ef0483cf0abe6f9f88b4bb9ba681aec7be4 > # Parent d1816a2696de8c2faa1cd913a151e5f62a8620f3 > Avoid leak on error allocating name > > diff -r d1816a2696de -r dd8c5ef0483c src/core/ngx_resolver.c > --- a/src/core/ngx_resolver.c Fri Jun 16 18:15:58 2017 +0300 > +++ b/src/core/ngx_resolver.c Mon Jun 19 07:50:45 2017 +0200 > @@ -443,7 +443,7 @@ > > name.data = ngx_resolver_alloc(r, name.len); > if (name.data == NULL) { > -return NGX_ERROR; > +goto resolve_error; > } > > if (slen == ctx->service.len) { > @@ -481,6 +481,7 @@ > ngx_resolver_free(r, ctx->event); > } > > +resolve_error: > ngx_resolver_free(r, ctx); > > return NGX_ERROR; Thanks! Committed with minor changes. -- Roman Arutyunyan ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
[nginx] Resolver: fixed allocation error handling while resolving SRV.
details: http://hg.nginx.org/nginx/rev/a39bc74873fa branches: changeset: 7039:a39bc74873fa user: Bart Warmerdamdate: Mon Jun 19 14:25:42 2017 +0300 description: Resolver: fixed allocation error handling while resolving SRV. diffstat: src/core/ngx_resolver.c | 4 +++- 1 files changed, 3 insertions(+), 1 deletions(-) diffs (21 lines): diff -r d1816a2696de -r a39bc74873fa src/core/ngx_resolver.c --- a/src/core/ngx_resolver.c Fri Jun 16 18:15:58 2017 +0300 +++ b/src/core/ngx_resolver.c Mon Jun 19 14:25:42 2017 +0300 @@ -443,7 +443,7 @@ ngx_resolve_name(ngx_resolver_ctx_t *ctx name.data = ngx_resolver_alloc(r, name.len); if (name.data == NULL) { -return NGX_ERROR; +goto failed; } if (slen == ctx->service.len) { @@ -481,6 +481,8 @@ ngx_resolve_name(ngx_resolver_ctx_t *ctx ngx_resolver_free(r, ctx->event); } +failed: + ngx_resolver_free(r, ctx); return NGX_ERROR; ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: [PATCH] Avoid using the result of i2d_SSL_SESSION when the session is invalid
On Mon, Jun 19, 2017 at 08:08:38AM +0200, Bart Warmerdam wrote: > # HG changeset patch > # User Bart Warmerdam> # Date 1497852211 -7200 > # Mon Jun 19 08:03:31 2017 +0200 > # Branch i2d_ssl_session_length > # Node ID 079afb2cb4be3ef06d07e96d1a54cc359b971631 > # Parent d1816a2696de8c2faa1cd913a151e5f62a8620f3 > Make sure to also take into account the 'return 0' response of > i2d_SSL_SESSION, which is possible when the session is not valid A case of invalid session is already caught by checking the return value of SSL_get0_session() just prior to calling i2d_SSL_SESSION() in ngx_http_upstream_save_round_robin_peer_session() and ngx_stream_upstream_save_round_robin_peer_session(). The ngx_ssl_new_session() function is passed as an argument to SSL_CTX_sess_set_new_cb() that "sets the callback function, which is automatically called whenever a new session was negotiated." Do you think that it's possible for a session to be invalid in either of these cases? > diff -r d1816a2696de -r 079afb2cb4be src/event/ngx_event_openssl.c > --- a/src/event/ngx_event_openssl.c Fri Jun 16 18:15:58 2017 +0300 > +++ b/src/event/ngx_event_openssl.c Mon Jun 19 08:03:31 2017 +0200 > @@ -2458,9 +2458,9 @@ > > len = i2d_SSL_SESSION(sess, NULL); > > -/* do not cache too big session */ > - > -if (len > (int) NGX_SSL_MAX_SESSION_SIZE) { > +/* do not cache too big or invalid session */ > + > +if (len > (int) NGX_SSL_MAX_SESSION_SIZE || len < 1) { > return 0; > } > > diff -r d1816a2696de -r 079afb2cb4be > src/http/ngx_http_upstream_round_robin.c > --- a/src/http/ngx_http_upstream_round_robin.c Fri Jun 16 18:15:58 2017 > +0300 > +++ b/src/http/ngx_http_upstream_round_robin.c Mon Jun 19 08:03:31 2017 > +0200 > @@ -755,9 +755,9 @@ > > len = i2d_SSL_SESSION(ssl_session, NULL); > > -/* do not cache too big session */ > +/* do not cache too big or invalid session */ > > -if (len > NGX_SSL_MAX_SESSION_SIZE) { > + if (len > NGX_SSL_MAX_SESSION_SIZE || len < 1) { > return; > } > > diff -r d1816a2696de -r 079afb2cb4be > src/stream/ngx_stream_upstream_round_robin.c > --- a/src/stream/ngx_stream_upstream_round_robin.c Fri Jun 16 > 18:15:58 2017 +0300 > +++ b/src/stream/ngx_stream_upstream_round_robin.c Mon Jun 19 > 08:03:31 2017 +0200 > @@ -787,9 +787,9 @@ > > len = i2d_SSL_SESSION(ssl_session, NULL); > > -/* do not cache too big session */ > +/* do not cache too big or invalid session */ > > -if (len > NGX_SSL_MAX_SESSION_SIZE) { > +if (len > NGX_SSL_MAX_SESSION_SIZE || len < 1) { > return; > } > ___ > nginx-devel mailing list > nginx-devel@nginx.org > http://mailman.nginx.org/mailman/listinfo/nginx-devel > -- Ruslan Ermilov Join us at nginx.conf, Sep. 6-8, Portland, OR https://www.nginx.com/nginxconf ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
[PATCH] Avoid using the result of i2d_SSL_SESSION when the session is invalid
# HG changeset patch # User Bart Warmerdam# Date 1497852211 -7200 # Mon Jun 19 08:03:31 2017 +0200 # Branch i2d_ssl_session_length # Node ID 079afb2cb4be3ef06d07e96d1a54cc359b971631 # Parent d1816a2696de8c2faa1cd913a151e5f62a8620f3 Make sure to also take into account the 'return 0' response of i2d_SSL_SESSION, which is possible when the session is not valid diff -r d1816a2696de -r 079afb2cb4be src/event/ngx_event_openssl.c --- a/src/event/ngx_event_openssl.c Fri Jun 16 18:15:58 2017 +0300 +++ b/src/event/ngx_event_openssl.c Mon Jun 19 08:03:31 2017 +0200 @@ -2458,9 +2458,9 @@ len = i2d_SSL_SESSION(sess, NULL); -/* do not cache too big session */ - -if (len > (int) NGX_SSL_MAX_SESSION_SIZE) { +/* do not cache too big or invalid session */ + +if (len > (int) NGX_SSL_MAX_SESSION_SIZE || len < 1) { return 0; } diff -r d1816a2696de -r 079afb2cb4be src/http/ngx_http_upstream_round_robin.c --- a/src/http/ngx_http_upstream_round_robin.c Fri Jun 16 18:15:58 2017 +0300 +++ b/src/http/ngx_http_upstream_round_robin.c Mon Jun 19 08:03:31 2017 +0200 @@ -755,9 +755,9 @@ len = i2d_SSL_SESSION(ssl_session, NULL); -/* do not cache too big session */ +/* do not cache too big or invalid session */ -if (len > NGX_SSL_MAX_SESSION_SIZE) { + if (len > NGX_SSL_MAX_SESSION_SIZE || len < 1) { return; } diff -r d1816a2696de -r 079afb2cb4be src/stream/ngx_stream_upstream_round_robin.c --- a/src/stream/ngx_stream_upstream_round_robin.c Fri Jun 16 18:15:58 2017 +0300 +++ b/src/stream/ngx_stream_upstream_round_robin.c Mon Jun 19 08:03:31 2017 +0200 @@ -787,9 +787,9 @@ len = i2d_SSL_SESSION(ssl_session, NULL); -/* do not cache too big session */ +/* do not cache too big or invalid session */ -if (len > NGX_SSL_MAX_SESSION_SIZE) { +if (len > NGX_SSL_MAX_SESSION_SIZE || len < 1) { return; } ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
[PATCH] Avoid memory leak on failure to allocate name during resolving
# HG changeset patch # User Bart Warmerdam# Date 1497851445 -7200 # Mon Jun 19 07:50:45 2017 +0200 # Branch memleak_resolve_name # Node ID dd8c5ef0483cf0abe6f9f88b4bb9ba681aec7be4 # Parent d1816a2696de8c2faa1cd913a151e5f62a8620f3 Avoid leak on error allocating name diff -r d1816a2696de -r dd8c5ef0483c src/core/ngx_resolver.c --- a/src/core/ngx_resolver.c Fri Jun 16 18:15:58 2017 +0300 +++ b/src/core/ngx_resolver.c Mon Jun 19 07:50:45 2017 +0200 @@ -443,7 +443,7 @@ name.data = ngx_resolver_alloc(r, name.len); if (name.data == NULL) { -return NGX_ERROR; +goto resolve_error; } if (slen == ctx->service.len) { @@ -481,6 +481,7 @@ ngx_resolver_free(r, ctx->event); } +resolve_error: ngx_resolver_free(r, ctx); return NGX_ERROR; ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel