twisted.web.proxy incorrectly mangles URIs

[exarkun]

ReverseProxyResource.render parses the request URI it receives. It then passes on the path and the query portions to the target of the proxying.

It does not pass on the fragment.

It unquotes the URI and does not re-quote it when sending it on.

[exarkun]

Huh, actually, I have no idea what the correct behavior WRT the fragment is. The quoting issue is real and serious, though.

[exarkun]

(In [22401]) Branching to 'proxy-urlquote-3013'

[danderson]

The changes on the branch look good.

As for the fragment question, I don't think that a normal HTTP client would ever send a fragment to a server, since it's just a local referencing system, none of the server's business. At least Firefox and wget filter out the fragment part of the URL before sending the request to the server. Telnetting to an Apache server and GETing an URL with a fragment returns the same resource as the URL without fragment, but the fragment is made available in logs, and to scripts (eg. php).

Given all that, it seems that the issue is moot for most applications, since they don't actually send a fragment. But, to make the proxy as transparent as posible, I think the fragment should be passed on. Let the destination server decide what to do with it.

[exarkun]

Thanks for the review.

Passing on the fragment makes sense to me, but it's going to be pretty difficult to implement.

ReverseProxyResource learns about the URI through the getChild API. The segment passed to getChild is already decoded. This means there's no way to differentiate between the # which delimits the fragment and a # which was really a %23 in the request. So, while ideally I'd change this code to pass on fragments, I'm not actually excited about changing the whole proxying implementation in order to support this non-standard feature.

[therve]

Why did you mangle the quote import as urlquote?

Otherwise, looks good.

[exarkun]

15:06 < exarkun> therve: "quote" seems too vague to me
15:07 < therve> exarkun: alright
15:07 < therve> exarkun: you can merge then

[exarkun]

(In [22431]) Merge proxy-urlquote-3013

Author: exarkun Reviewer: danderson, therve Fixes #3013

Change twisted.web.proxy.ReverseProxyResource so that it correctly encodes the request URI it sends on to the server for which it is a proxy.