Re: [PATCH] HTTP/2: added support for setting custom push request headers
On Tue, Feb 13, 2018 at 11:58:40AM +, Alessandro Ghedini wrote:
> If it's of any help, I merged your patch and mine into one, which copies the
> headers (excluding Accept) into PUSH_PROMISE and r->headers_in like my
> original
> patch did, as well as HPACK encode them into PUSH_PROMISE instead of writing
> them as literal strings, as your patch did. This fixes the problems I
> mentioned
> in my previous email.
To keep you updated, we're cultivating these patches:
# HG changeset patch
# User Ruslan Ermilov
# Date 1518609549 -10800
# Wed Feb 14 14:59:09 2018 +0300
# Node ID 9cd08f096c366771ffbebd2872883be04903d425
# Parent 8b0553239592f5d0fd419e5116b9d343838685cf
Expose more headers with NGX_HTTP_HEADERS.
diff --git a/src/http/ngx_http_request.c b/src/http/ngx_http_request.c
--- a/src/http/ngx_http_request.c
+++ b/src/http/ngx_http_request.c
@@ -140,7 +140,7 @@ ngx_http_header_t ngx_http_headers_in[]
offsetof(ngx_http_headers_in_t, upgrade),
ngx_http_process_header_line },
-#if (NGX_HTTP_GZIP)
+#if (NGX_HTTP_GZIP || NGX_HTTP_HEADERS)
{ ngx_string("Accept-Encoding"),
offsetof(ngx_http_headers_in_t, accept_encoding),
ngx_http_process_header_line },
diff --git a/src/http/ngx_http_request.h b/src/http/ngx_http_request.h
--- a/src/http/ngx_http_request.h
+++ b/src/http/ngx_http_request.h
@@ -200,7 +200,7 @@ typedef struct {
ngx_table_elt_t *expect;
ngx_table_elt_t *upgrade;
-#if (NGX_HTTP_GZIP)
+#if (NGX_HTTP_GZIP || NGX_HTTP_HEADERS)
ngx_table_elt_t *accept_encoding;
ngx_table_elt_t *via;
#endif
# HG changeset patch
# User Ruslan Ermilov
# Date 1518634812 -10800
# Wed Feb 14 22:00:12 2018 +0300
# Node ID cd54884b375b70ecd358358e5e78f43e70b6163a
# Parent 9cd08f096c366771ffbebd2872883be04903d425
HTTP/2: push additional request headers (closes #1478).
The Accept-Encoding, Accept-Language, and User-Agent header fields
are now copied from the original request to pushed requests.
diff --git a/auto/modules b/auto/modules
--- a/auto/modules
+++ b/auto/modules
@@ -420,6 +420,7 @@ if [ $HTTP = YES ]; then
if [ $HTTP_V2 = YES ]; then
have=NGX_HTTP_V2 . auto/have
+have=NGX_HTTP_HEADERS . auto/have
ngx_module_name=ngx_http_v2_module
ngx_module_incs=src/http/v2
diff --git a/src/http/v2/ngx_http_v2.c b/src/http/v2/ngx_http_v2.c
--- a/src/http/v2/ngx_http_v2.c
+++ b/src/http/v2/ngx_http_v2.c
@@ -11,6 +11,14 @@
#include
+typedef struct {
+ngx_str_t name;
+ngx_uint_t offset;
+ngx_uint_t hash;
+ngx_http_header_t *hh;
+} ngx_http_v2_parse_header_t;
+
+
/* errors */
#define NGX_HTTP_V2_NO_ERROR 0x0
#define NGX_HTTP_V2_PROTOCOL_ERROR 0x1
@@ -156,6 +164,8 @@ static ngx_int_t ngx_http_v2_parse_schem
ngx_str_t *value);
static ngx_int_t ngx_http_v2_parse_authority(ngx_http_request_t *r,
ngx_str_t *value);
+static ngx_int_t ngx_http_v2_parse_header(ngx_http_request_t *r,
+ngx_http_v2_parse_header_t *header, ngx_str_t *value);
static ngx_int_t ngx_http_v2_construct_request_line(ngx_http_request_t *r);
static ngx_int_t ngx_http_v2_cookie(ngx_http_request_t *r,
ngx_http_v2_header_t *header);
@@ -201,6 +211,23 @@ static ngx_http_v2_handler_pt ngx_http_v
(sizeof(ngx_http_v2_frame_states) / sizeof(ngx_http_v2_handler_pt))
+static ngx_http_v2_parse_header_t ngx_http_v2_parse_headers[] = {
+{ ngx_string("host"),
+ offsetof(ngx_http_headers_in_t, host), 0, NULL },
+
+{ ngx_string("accept-encoding"),
+ offsetof(ngx_http_headers_in_t, accept_encoding), 0, NULL },
+
+{ ngx_string("accept-language"),
+ offsetof(ngx_http_headers_in_t, accept_language), 0, NULL },
+
+{ ngx_string("user-agent"),
+ offsetof(ngx_http_headers_in_t, user_agent), 0, NULL },
+
+{ ngx_null_string, 0, 0, NULL }
+};
+
+
void
ngx_http_v2_init(ngx_event_t *rev)
{
@@ -2514,21 +2541,25 @@ ngx_http_v2_parse_int(ngx_http_v2_connec
}
-ngx_int_t
-ngx_http_v2_push_stream(ngx_http_v2_connection_t *h2c, ngx_uint_t depend,
-size_t request_length, ngx_str_t *path, ngx_str_t *authority)
+ngx_http_v2_stream_t *
+ngx_http_v2_push_stream(ngx_http_v2_stream_t *parent, ngx_str_t *path)
{
-ngx_int_t rc;
-ngx_str_t value;
-ngx_connection_t *fc;
-ngx_http_request_t*r;
-ngx_http_v2_node_t*node;
-ngx_http_v2_stream_t *stream;
+ngx_int_t rc;
+ngx_str_t value;
+ngx_table_elt_t **h;
+ngx_connection_t *fc;
+ngx_http_request_t *r;
+ngx_http_v2_node_t *node;
+ngx_http_v2_stream_t *stream;
+ngx_http_v2_connection_t *h2c;
+ngx_http_v2_parse_header_t *header;
+
+h2c = parent->connection;
node = ngx_htt
Re: [PATCH] HTTP/2: added support for setting custom push request headers
On Mon, Feb 12, 2018 at 03:30:21PM +, Alessandro Ghedini wrote:
> On Mon, Feb 12, 2018 at 05:11:55PM +0300, Ruslan Ermilov wrote:
> > On Mon, Feb 12, 2018 at 12:35:13PM +, Alessandro Ghedini wrote:
> > > # HG changeset patch
> > > # User Alessandro Ghedini
> > > # Date 1518438578 0
> > > # Mon Feb 12 12:29:38 2018 +
> > > # Branch http2-push-header
> > > # Node ID 4eb0c9e8da0bc52065578e4ee78df1833617ac35
> > > # Parent a49af443656f2b65ca5de9d8cad5594f44e18ff7
> > > HTTP/2: added support for setting custom push request headers.
> > >
> > > This implementa the http2_push_header configuration directive that makes
> > > it possible to set custom request headers for pushed requests.
> > >
> > > Complex values are evaluated in the context of the original request,
> > > so it's possible to copy its headers into pushed requests.
> > >
> > > Example usage:
> > >
> > > http2_push_header User-Agent $http_user_agent;
> >
> > If I'm not mistaken, both the original patch and this one
> > do not send the copied headers in the PUSH_PROMISE frame.
> >
> > Given the https://trac.nginx.org/nginx/ticket/1478, and some
> > research of how different implementations behave, I prepared
> > the following patch:
>
> Also, your patch doesn't seem to work completely. You don't to copy the
> request
> headers in the request's headers_in, which means they don't get processed by
> NGINX, if the pushed resource is handled by NGINX itself.
If it's of any help, I merged your patch and mine into one, which copies the
headers (excluding Accept) into PUSH_PROMISE and r->headers_in like my original
patch did, as well as HPACK encode them into PUSH_PROMISE instead of writing
them as literal strings, as your patch did. This fixes the problems I mentioned
in my previous email.
# HG changeset patch
# User Alessandro Ghedini
# Date 1518522249 0
# Tue Feb 13 11:44:09 2018 +
# Branch push-headers
# Node ID bfd3019a0f63cf00364d44bab74b7864808abbd7
# Parent 8b0553239592f5d0fd419e5116b9d343838685cf
HTTP/2: push additional headers (ticket #1478).
When pushing requests, copy and push Accept-Encoding, Accept-Language,
and User-Agent header fields from the original request.
Includes changes made by Ruslan Ermilov.
diff -r 8b0553239592 -r bfd3019a0f63 src/http/v2/ngx_http_v2.c
--- a/src/http/v2/ngx_http_v2.c Fri Feb 09 23:20:08 2018 +0300
+++ b/src/http/v2/ngx_http_v2.c Tue Feb 13 11:44:09 2018 +
@@ -156,6 +156,8 @@
ngx_str_t *value);
static ngx_int_t ngx_http_v2_parse_authority(ngx_http_request_t *r,
ngx_str_t *value);
+static ngx_int_t ngx_http_v2_copy_header(ngx_http_request_t *r,
+ngx_table_elt_t *hdr);
static ngx_int_t ngx_http_v2_construct_request_line(ngx_http_request_t *r);
static ngx_int_t ngx_http_v2_cookie(ngx_http_request_t *r,
ngx_http_v2_header_t *header);
@@ -2516,7 +2518,8 @@
ngx_int_t
ngx_http_v2_push_stream(ngx_http_v2_connection_t *h2c, ngx_uint_t depend,
-size_t request_length, ngx_str_t *path, ngx_str_t *authority)
+size_t request_length, ngx_str_t *path, ngx_str_t *authority,
+ngx_http_headers_in_t *headers_in)
{
ngx_int_t rc;
ngx_str_t value;
@@ -2605,6 +2608,28 @@
goto error;
}
+rc = ngx_http_v2_copy_header(r, headers_in->user_agent);
+
+if (rc != NGX_OK) {
+goto error;
+}
+
+#if (NGX_HTTP_GZIP)
+rc = ngx_http_v2_copy_header(r, headers_in->accept_encoding);
+
+if (rc != NGX_OK) {
+goto error;
+}
+#endif
+
+#if (NGX_HTTP_HEADERS)
+rc = ngx_http_v2_copy_header(r, headers_in->accept_language);
+
+if (rc != NGX_OK) {
+goto error;
+}
+#endif
+
fc->write->handler = ngx_http_v2_run_request_handler;
ngx_post_event(fc->write, &ngx_posted_events);
@@ -3479,6 +3504,63 @@
static ngx_int_t
+ngx_http_v2_copy_header(ngx_http_request_t *r, ngx_table_elt_t *hdr)
+{
+ngx_table_elt_t*h;
+ngx_http_header_t *hh;
+ngx_http_core_main_conf_t *cmcf;
+
+if (hdr == NULL) {
+return NGX_OK;
+}
+
+h = ngx_list_push(&r->headers_in.headers);
+if (h == NULL) {
+return NGX_ERROR;
+}
+
+h->hash = hdr->hash;
+
+h->key.len = hdr->key.len;
+
+h->key.data = ngx_pnalloc(r->stream->pool, h->key.len + 1);
+if (h->key.data == NULL) {
+h->hash = 0;
+return NGX_ERROR;
+}
+
+(void) ngx_cpystrn(h->key.data, hdr->key.data, h->key.len + 1);
+
+h->value.len = hdr->value.len;
+
+h->value.data = ngx_pnalloc(r->stream->pool, h->value.len + 1);
+if (h->key.data == NULL) {
+h->hash = 0;
+return NGX_ERROR;
+}
+
+(void) ngx_cpystrn(h->value.data, hdr->value.data, h->value.len + 1);
+
+h->lowcase_key = h->key.data;
+
+cmcf = ngx_http_get_module_main_conf(r, ngx_http_core_module);
+
+hh = ngx_hash_find(&cmcf->headers_in_hash, h->hash,
+ h->lowcase_key, h->key.len);
+
+if (hh == NULL) {
+re
Re: [PATCH] HTTP/2: added support for setting custom push request headers
On Mon, Feb 12, 2018 at 05:11:55PM +0300, Ruslan Ermilov wrote:
> On Mon, Feb 12, 2018 at 12:35:13PM +, Alessandro Ghedini wrote:
> > # HG changeset patch
> > # User Alessandro Ghedini
> > # Date 1518438578 0
> > # Mon Feb 12 12:29:38 2018 +
> > # Branch http2-push-header
> > # Node ID 4eb0c9e8da0bc52065578e4ee78df1833617ac35
> > # Parent a49af443656f2b65ca5de9d8cad5594f44e18ff7
> > HTTP/2: added support for setting custom push request headers.
> >
> > This implementa the http2_push_header configuration directive that makes
> > it possible to set custom request headers for pushed requests.
> >
> > Complex values are evaluated in the context of the original request,
> > so it's possible to copy its headers into pushed requests.
> >
> > Example usage:
> >
> > http2_push_header User-Agent $http_user_agent;
>
> If I'm not mistaken, both the original patch and this one
> do not send the copied headers in the PUSH_PROMISE frame.
>
> Given the https://trac.nginx.org/nginx/ticket/1478, and some
> research of how different implementations behave, I prepared
> the following patch:
Also, your patch doesn't seem to work completely. You don't to copy the request
headers in the request's headers_in, which means they don't get processed by
NGINX, if the pushed resource is handled by NGINX itself.
E.g.
daemon off;
worker_processes 1;
master_process off;
events {
worker_connections 1024;
}
http {
access_log /dev/stdout;
error_log logs/error.log debug;
server {
listen 4433 ssl http2;
server_name localhost;
ssl_certificate cert.crt;
ssl_certificate_key cert.key;
http2_max_concurrent_pushes 200;
http2_push_preloadon;
gzip on;
gzip_types text/plain;
gzip_min_length 1;
location /hello {
http2_push /a;
root html;
}
location /a {
return 200 "a";
}
}
}
% nghttp https://127.0.0.1:4433/hello -vn
[ 0.001] Connected
[WARNING] Certificate verification failed: Hostname mismatch
The negotiated protocol: h2
[ 0.003] send SETTINGS frame
(niv=2)
[SETTINGS_MAX_CONCURRENT_STREAMS(0x03):100]
[SETTINGS_INITIAL_WINDOW_SIZE(0x04):65535]
[ 0.003] send PRIORITY frame
(dep_stream_id=0, weight=201, exclusive=0)
[ 0.003] send PRIORITY frame
(dep_stream_id=0, weight=101, exclusive=0)
[ 0.003] send PRIORITY frame
(dep_stream_id=0, weight=1, exclusive=0)
[ 0.003] send PRIORITY frame
(dep_stream_id=7, weight=1, exclusive=0)
[ 0.003] send PRIORITY frame
(dep_stream_id=3, weight=1, exclusive=0)
[ 0.003] send HEADERS frame
; END_STREAM | END_HEADERS | PRIORITY
(padlen=0, dep_stream_id=11, weight=16, exclusive=0)
; Open new stream
:method: GET
:path: /hello
:scheme: https
:authority: 127.0.0.1:4433
accept: */*
accept-encoding: gzip, deflate
user-agent: nghttp2/1.29.0
[ 0.004] recv SETTINGS frame
(niv=3)
[SETTINGS_MAX_CONCURRENT_STREAMS(0x03):128]
[SETTINGS_INITIAL_WINDOW_SIZE(0x04):65536]
[SETTINGS_MAX_FRAME_SIZE(0x05):16777215]
[ 0.004] recv WINDOW_UPDATE frame
(window_size_increment=2147418112)
[ 0.004] send SETTINGS frame
; ACK
(niv=0)
[ 0.004] recv SETTINGS frame
; ACK
(niv=0)
[ 0.004] recv (stream_id=13) :method: GET
[ 0.004] recv (stream_id=13) :path: /a
[ 0.004] recv (stream_id=13) :authority: 127.0.0.1:4433
[ 0.004] recv (stream_id=13) :scheme: https
[ 0.004] recv (stream_id=13) accept-encoding: gzip, deflate
[ 0.004] recv (stream_id=13) user-agent: nghttp2/1.29.0
[ 0.004] recv PUSH_PROMISE frame
; END_HEADERS
(padlen=0, promised_stream_id=2)
[ 0.004] recv (stream_id=13) :status: 200
[ 0.004] recv (stream_id=13) server: nginx/1.13.9
[ 0.004] recv (stream_id=13) date: Mon, 12 Feb 2018 15:18:39 GMT
[ 0.004] recv (stream_id=13) content-type: text/plain
[ 0.004] recv (stream_id=13) last-modified: Thu, 08 Feb 2018 15:43:05 GMT
[ 0.004] recv (stream_id=13) etag: W/"5a7c7009-1f5"
[ 0.004] recv (stream_id=13) content-encoding: gzip
[ 0.004] recv HEADERS frame
; END_HEADERS
(padlen=0)
; First response header
[ 0.004] recv DATA frame
; END_STREAM
[ 0.004] recv (stream_id=2) :status: 200
[ 0.004] recv (stream_id=2) server: nginx/1.13.9
[ 0.004] recv (stream_id=2) date: Mon, 12 Feb 2018 15:18:39 GMT
[ 0.004] recv (stream_id=2) content-type: text/plain
[ 0.004] recv (stream_id=2) content-length: 1
[ 0.004] recv HEADERS frame
; END_HEADERS
(padlen=0)
; First push response header
[ 0.004] recv DATA frame
; END_STREAM
[ 0.004] send GOAWAY frame
(last_stream_id=2, error_code=NO_ERROR(0x00), opaque_d
Re: [PATCH] HTTP/2: added support for setting custom push request headers
On Mon, Feb 12, 2018 at 05:11:55PM +0300, Ruslan Ermilov wrote: > On Mon, Feb 12, 2018 at 12:35:13PM +, Alessandro Ghedini wrote: > > # HG changeset patch > > # User Alessandro Ghedini > > # Date 1518438578 0 > > # Mon Feb 12 12:29:38 2018 + > > # Branch http2-push-header > > # Node ID 4eb0c9e8da0bc52065578e4ee78df1833617ac35 > > # Parent a49af443656f2b65ca5de9d8cad5594f44e18ff7 > > HTTP/2: added support for setting custom push request headers. > > > > This implementa the http2_push_header configuration directive that makes > > it possible to set custom request headers for pushed requests. > > > > Complex values are evaluated in the context of the original request, > > so it's possible to copy its headers into pushed requests. > > > > Example usage: > > > > http2_push_header User-Agent $http_user_agent; > > If I'm not mistaken, both the original patch and this one > do not send the copied headers in the PUSH_PROMISE frame. They both do actually AFAICT, see the changes in ngx_http_v2_push_resource. Though they don't use HPACK. Cheers ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: [PATCH] HTTP/2: added support for setting custom push request headers
On Mon, Feb 12, 2018 at 12:35:13PM +, Alessandro Ghedini wrote:
> # HG changeset patch
> # User Alessandro Ghedini
> # Date 1518438578 0
> # Mon Feb 12 12:29:38 2018 +
> # Branch http2-push-header
> # Node ID 4eb0c9e8da0bc52065578e4ee78df1833617ac35
> # Parent a49af443656f2b65ca5de9d8cad5594f44e18ff7
> HTTP/2: added support for setting custom push request headers.
>
> This implementa the http2_push_header configuration directive that makes
> it possible to set custom request headers for pushed requests.
>
> Complex values are evaluated in the context of the original request,
> so it's possible to copy its headers into pushed requests.
>
> Example usage:
>
> http2_push_header User-Agent $http_user_agent;
If I'm not mistaken, both the original patch and this one
do not send the copied headers in the PUSH_PROMISE frame.
Given the https://trac.nginx.org/nginx/ticket/1478, and some
research of how different implementations behave, I prepared
the following patch:
# HG changeset patch
# User Ruslan Ermilov
# Date 151803 -10800
# Mon Feb 12 17:06:43 2018 +0300
# Node ID 4e72b3bfdeaa9917bbdc89a7fa75580c6bec41a7
# Parent 8b0553239592f5d0fd419e5116b9d343838685cf
HTTP/2: push additional headers (ticket #1478).
When pushing requests, copy and push Accept-Encoding, Accept-Language,
and User-Agent header fields from the original request.
diff --git a/src/http/v2/ngx_http_v2_filter_module.c
b/src/http/v2/ngx_http_v2_filter_module.c
--- a/src/http/v2/ngx_http_v2_filter_module.c
+++ b/src/http/v2/ngx_http_v2_filter_module.c
@@ -50,20 +50,37 @@
#define NGX_HTTP_V2_STATUS_404_INDEX 13
#define NGX_HTTP_V2_STATUS_500_INDEX 14
+#define NGX_HTTP_V2_ACCEPT_ENCODING_INDEX 16
+#define NGX_HTTP_V2_ACCEPT_LANGUAGE_INDEX 17
#define NGX_HTTP_V2_CONTENT_LENGTH_INDEX 28
#define NGX_HTTP_V2_CONTENT_TYPE_INDEX31
#define NGX_HTTP_V2_DATE_INDEX33
#define NGX_HTTP_V2_LAST_MODIFIED_INDEX 44
#define NGX_HTTP_V2_LOCATION_INDEX46
#define NGX_HTTP_V2_SERVER_INDEX 54
+#define NGX_HTTP_V2_USER_AGENT_INDEX 58
#define NGX_HTTP_V2_VARY_INDEX59
#define NGX_HTTP_V2_NO_TRAILERS (ngx_http_v2_out_frame_t *) -1
+typedef struct {
+ngx_str_t authority;
+#if (NGX_HTTP_GZIP)
+ngx_str_t accept_encoding;
+#endif
+#if (NGX_HTTP_HEADERS)
+ngx_str_t accept_language;
+#endif
+ngx_str_t user_agent;
+} ngx_http_v2_push_ctx_t;
+
+
static ngx_int_t ngx_http_v2_push_resources(ngx_http_request_t *r);
static ngx_int_t ngx_http_v2_push_resource(ngx_http_request_t *r,
-ngx_str_t *path, ngx_str_t *authority);
+ngx_str_t *path, ngx_http_v2_push_ctx_t *ctx);
+static ngx_int_t ngx_http_v2_indexed_header_encode(ngx_http_request_t *r,
+u_char index, ngx_str_t *value, ngx_str_t *header);
static u_char *ngx_http_v2_string_encode(u_char *dst, u_char *src, size_t len,
u_char *tmp, ngx_uint_t lower);
@@ -685,16 +702,17 @@ ngx_http_v2_push_resources(ngx_http_requ
{
u_char *start, *end, *last;
ngx_int_t rc;
-ngx_str_t path, authority;
+ngx_str_t path;
ngx_uint_t i, push;
ngx_table_elt_t **h;
ngx_http_v2_loc_conf_t *h2lcf;
+ngx_http_v2_push_ctx_t ctx;
ngx_http_complex_value_t *pushes;
ngx_log_debug0(NGX_LOG_DEBUG_HTTP, r->connection->log, 0,
"http2 push resources");
-ngx_str_null(&authority);
+ngx_memzero(&ctx, sizeof(ngx_http_v2_push_ctx_t));
h2lcf = ngx_http_get_module_loc_conf(r, ngx_http_v2_module);
@@ -715,7 +733,7 @@ ngx_http_v2_push_resources(ngx_http_requ
continue;
}
-rc = ngx_http_v2_push_resource(r, &path, &authority);
+rc = ngx_http_v2_push_resource(r, &path, &ctx);
if (rc == NGX_ERROR) {
return NGX_ERROR;
@@ -880,7 +898,7 @@ ngx_http_v2_push_resources(ngx_http_requ
if (push && path.len
&& !(path.len > 1 && path.data[0] == '/' && path.data[1] == '/'))
{
-rc = ngx_http_v2_push_resource(r, &path, &authority);
+rc = ngx_http_v2_push_resource(r, &path, &ctx);
if (rc == NGX_ERROR) {
return NGX_ERROR;
@@ -905,11 +923,17 @@ ngx_http_v2_push_resources(ngx_http_requ
static ngx_int_t
ngx_http_v2_push_resource(ngx_http_request_t *r, ngx_str_t *path,
-ngx_str_t *authority)
+ngx_http_v2_push_ctx_t *ctx)
{
u_char*start, *pos, *tmp;
size_t len;
-ngx_table_elt_t *host;
+ngx_table_elt_t *host, *user_agent;
+#if (NGX_HTTP_GZIP)
+ngx_table_elt_t *accept_encoding;
+#endif
+#if (NGX_HTTP_HEADERS)
+ngx_table_elt_t *accept_language;
+#endif
