Welcome! Log In Create A New Profile

Advanced

Re: [PATCH] HTTP: Add new uri_normalization_percent_decode option

Maxim Dounin
April 01, 2023 04:12PM
Hello!

On Thu, Mar 30, 2023 at 05:19:08PM +0000, Michael Kourlas via nginx-devel wrote:

> Hello,
>
> Thanks again for your comments.
>
> > This implies, basically, that there are 3 forms of the request
> > URI: 1) fully encoded, as in $request_uri, 2) fully decoded, as in
> > $uri now, and 3) "all-except-percent-and-reserved". To implement this
> > correctly, it needs clear definition when each form is used, and
> > it is going to be a non-trivial task to do this safely.
>
> I agree. A simple way to do this would be to make percent-decoding customizable
> on a per-directive basis. The core use case I was hoping to support is
> preserving encoded reserved characters in location matching (basically what was
> proposed in [1]), so that is what I would like to focus on in a reworked
> version of this patch.
>
> I propose the following:
>
> (1) The addition of a new variable called $uri_encoded_percent_and_reserved. As
> discussed, this variable is a special version of the normalized URI ($uri)
> that preserves any percent-encoded "%" or reserved characters.
>
> (2) Every transformation applied to $uri (e.g. from the "rewrite" directive,
> internal redirects, etc.) is automatically applied to
> $uri_encoded_percent_and_reserved as well.
>
> If this raises performance concerns, a new flag could be added to enable or
> disable the availability of $uri_encoded_percent_and_reserved.

You suggest that transformations of $uri are "automatically
applied" to the non-fully-decoded variant. Consider the following
rewrite:

rewrite ^/(.*) /$1 break;

Assuming request to "GET /foo%2fbar/", what
$uri_encoded_percent_and_reserved do you expect after each of
these rewrites? Similarly, consider the following rewrite:

rewrite ^/foo/(.*) /$1 break;

What $uri_encoded_percent_and_reserved is expected after the
rewrite?

> (3) The addition of a new optional parameter to the URI form of "location"
> blocks called "match-source":
>
> location [ = | ~ | ~* | ^~ ] uri [match-source=uri|uri-encoded-percent-and-reserved] {
> ...
> }
>
> For example:
>
> location ~ ^/api/objects/[^/]+/subobjects(/.*)?$ match-source=uri-encoded-percent-and-reserved {
> ...
> }
>
> "match-source=uri" is the default and the current behaviour. When
> "uri-encoded-percent-and-reserved" is used, the location matching for that
> block uses $uri_encoded_percent_and_reserved rather than $uri. Nested location
> blocks are not affected (unless they also use
> "uri-encoded-percent-and-reserved").
>
> In future it would be possible to use a similar pattern with other directives
> that use $uri, such as "proxy_pass", but that can be done as part of a separate
> patch.
>
> If you think this is a sensible approach, I will submit a revised patch
> implementing it.

Consider the following configuration:

location /foo%2fbar/ match-source=uri-encoded-percent-and-reserved {
...
}

location /foo/bar/ match-source=uri {
...
}

The question is: which location is expected to be matched for the
request "GET /foo%2fbar/"?

Other questions include:

- Which location is expected to be matched for the request "GET
/foo%2Fbar/" (note that it is exactly equivalent to "GET
/foo%2fbar/").

- Assuming static handling in the locations, what happens with the
request "GET /foo%2fbar/..%2fbazz"?

Note that the behaviour does not seem to be obvious, and it is an
open question if it can be clarified to be safe.

--
Maxim Dounin
http://mdounin.ru/
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
https://mailman.nginx.org/mailman/listinfo/nginx-devel
Subject Author Views Posted

[PATCH] HTTP: Add new uri_normalization_percent_decode option

Michael Kourlas via nginx-devel 638 February 15, 2023 11:52AM

Re: [PATCH] HTTP: Add new uri_normalization_percent_decode option

Maxim Dounin 129 February 17, 2023 08:00AM

Re: [PATCH] HTTP: Add new uri_normalization_percent_decode option

Michael Kourlas via nginx-devel 97 March 27, 2023 12:20PM

Re: [PATCH] HTTP: Add new uri_normalization_percent_decode option

Maxim Dounin 102 March 28, 2023 10:10AM

RE: [PATCH] HTTP: Add new uri_normalization_percent_decode option

Michael Kourlas via nginx-devel 115 March 30, 2023 01:20PM

Re: [PATCH] HTTP: Add new uri_normalization_percent_decode option

Maxim Dounin 108 April 01, 2023 04:12PM

RE: [PATCH] HTTP: Add new uri_normalization_percent_decode option

Michael Kourlas via nginx-devel 95 April 03, 2023 02:34PM

Re: [PATCH] HTTP: Add new uri_normalization_percent_decode option

Maxim Dounin 111 April 03, 2023 11:28PM

RE: [PATCH] HTTP: Add new uri_normalization_percent_decode option

Michael Kourlas via nginx-devel 116 April 06, 2023 10:28AM



Sorry, you do not have permission to post/reply in this forum.

Online Users

Guests: 273
Record Number of Users: 8 on April 13, 2023
Record Number of Guests: 421 on December 02, 2018
Powered by nginx      Powered by FreeBSD      PHP Powered      Powered by MariaDB      ipv6 ready