Hello!
On Tue, Jun 25, 2024 at 11:22:14PM +0100, Kirill A. Korinsky wrote:
> Greetings,
>
> Here is a patch that adds a string to proxy_pass_request_headers as a
> possible value.
>
> Such a string is used as a prefix for headers which is overwritten by Nginx
> when it proxies the request to upstream.
>
> As a nice side effect, this feature preserves the original order of HTTP
> headers, which might be needed sometimes.
>
> It is only implemented on the proxy module because there is no other way to
> deliver the original request without touching it.
Could you please provide some more details about specific use
cases for such a feature?
As far as I see, using appropriate
proxy_set_header X-Original-Foo $http_foo;
directives should be mostly equivalent, except it won't preserve
the headers order and will merge duplicate headers. But these
aren't really guaranteed by HTTP anyway, and merging/reorder can
happen on any intermediate hosts.
>
> diff --git src/http/modules/ngx_http_proxy_module.c
> src/http/modules/ngx_http_proxy_module.c
> index 536482ec5..6a722042b 100644
> --- src/http/modules/ngx_http_proxy_module.c
> +++ src/http/modules/ngx_http_proxy_module.c
> @@ -117,6 +117,8 @@ typedef struct {
> ngx_uint_t headers_hash_max_size;
> ngx_uint_t headers_hash_bucket_size;
>
> + ngx_str_t request_headers_prefix;
> +
> #if (NGX_HTTP_SSL)
> ngx_uint_t ssl;
> ngx_uint_t ssl_protocols;
> @@ -215,6 +217,8 @@ static char *ngx_http_proxy_cookie_flags(ngx_conf_t *cf,
> ngx_command_t *cmd,
> void *conf);
> static char *ngx_http_proxy_store(ngx_conf_t *cf, ngx_command_t *cmd,
> void *conf);
> +static char *ngx_http_proxy_pass_request_headers(ngx_conf_t *cf,
> ngx_command_t *cmd,
> + void *conf);
> #if (NGX_HTTP_CACHE)
> static char *ngx_http_proxy_cache(ngx_conf_t *cf, ngx_command_t *cmd,
> void *conf);
> @@ -445,9 +449,9 @@ static ngx_command_t ngx_http_proxy_commands[] = {
>
> { ngx_string("proxy_pass_request_headers"),
> NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_HTTP_LOC_CONF|NGX_CONF_FLAG,
> - ngx_conf_set_flag_slot,
> + ngx_http_proxy_pass_request_headers,
> NGX_HTTP_LOC_CONF_OFFSET,
> - offsetof(ngx_http_proxy_loc_conf_t, upstream.pass_request_headers),
> + 0,
> NULL },
>
> { ngx_string("proxy_pass_request_body"),
> @@ -1389,7 +1393,10 @@ ngx_http_proxy_create_request(ngx_http_request_t *r)
> if (ngx_hash_find(&headers->hash, header[i].hash,
> header[i].lowcase_key, header[i].key.len))
> {
> - continue;
> + if (plcf->request_headers_prefix.len == 0) {
> + continue;
> + }
> + len += plcf->request_headers_prefix.len;
> }
>
> len += header[i].key.len + sizeof(": ") - 1
> @@ -1525,7 +1532,12 @@ ngx_http_proxy_create_request(ngx_http_request_t *r)
> if (ngx_hash_find(&headers->hash, header[i].hash,
> header[i].lowcase_key, header[i].key.len))
> {
> - continue;
> + if (plcf->request_headers_prefix.len == 0) {
> + continue;
> + }
> + b->last = ngx_copy(b->last,
> + plcf->request_headers_prefix.data,
> + plcf->request_headers_prefix.len);
> }
>
> b->last = ngx_copy(b->last, header[i].key.data,
> header[i].key.len);
Note that as implemented, you won't be able to distinguish
original requests headers from the ones with prefix added. E.g.,
assuming prefix "X-Original-", and original request headers:
Host: foo
X-Original-Host: bar
you'll get
X-Original-Host: foo
X-Original-Host: bar
in the upstream request, and you won't be able to tell which one
is real.
Following the exiting proxy_set_header behaviour, I would rather
suggests that such a feature, if implemented, should drop all the
headers with the configured prefix.
> @@ -3349,6 +3361,7 @@ ngx_http_proxy_create_loc_conf(ngx_conf_t *cf)
> * conf->body_values = NULL;
> * conf->body_source = { 0, NULL };
> * conf->redirects = NULL;
> + * conf->request_headers_prefix = { NULL, 0 };
> * conf->ssl = 0;
> * conf->ssl_protocols = 0;
> * conf->ssl_ciphers = { 0, NULL };
> @@ -3727,6 +3740,9 @@ ngx_http_proxy_merge_loc_conf(ngx_conf_t *cf, void
> *parent, void *child)
> ngx_conf_merge_value(conf->upstream.intercept_errors,
> prev->upstream.intercept_errors, 0);
>
> + ngx_conf_merge_str_value(conf->request_headers_prefix,
> + prev->request_headers_prefix, "");
> +
> #if (NGX_HTTP_SSL)
>
> if (ngx_http_proxy_merge_ssl(cf, conf, prev) != NGX_OK) {
> @@ -4772,6 +4788,36 @@ ngx_http_proxy_store(ngx_conf_t *cf, ngx_command_t
> *cmd, void *conf)
> }
>
>
> +static char *
> +ngx_http_proxy_pass_request_headers(ngx_conf_t *cf, ngx_command_t *cmd, void
> *conf)
> +{
> + ngx_http_proxy_loc_conf_t *plcf = conf;
> +
> + ngx_str_t *value;
Style: wrong indentation of variable name, should be
ngx_str_t *value;
> +
> + if (plcf->upstream.pass_request_headers != NGX_CONF_UNSET) {
> + return "is duplicate";
> + }
> +
> + value = cf->args->elts;
> +
> + if (ngx_strcmp(value[1].data, "off") == 0) {
> + plcf->upstream.pass_request_headers = 0;
> + return NGX_CONF_OK;
> + }
> +
> + plcf->upstream.store = 1;
Seems to be a cut-n-paste leftover.
> +
> + if (ngx_strcmp(value[1].data, "on") == 0) {
> + return NGX_CONF_OK;
> + }
> +
> + plcf->request_headers_prefix = value[1];
Not sure if reusing "proxy_pass_request_headers" for such a
feature is a good idea. From the behaviour of
"proxy_pass_request_headers off;", I would rather assume that
"proxy_pass_request_headers X-Original-;" will pass all the
headers with the specified prefix, and not just the headers which
were modified.
> +
> + return NGX_CONF_OK;
> +}
> +
> +
> #if (NGX_HTTP_CACHE)
>
> static char *
>
> --
> wbr, Kirill
--
Maxim Dounin
http://mdounin.ru/
