Opened 11 years ago
Closed 11 years ago
#3013 defect closed fixed (fixed)
twisted.web.proxy incorrectly mangles URIs
Reported by: | Jean-Paul Calderone | Owned by: | |
---|---|---|---|
Priority: | highest | Milestone: | |
Component: | web | Keywords: | |
Cc: | Branch: |
branches/proxy-urlquote-3013
branch-diff, diff-cov, branch-cov, buildbot |
|
Author: | exarkun |
Description
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.
Change History (10)
comment:1 Changed 11 years ago by
comment:2 Changed 11 years ago by
author: | → exarkun |
---|---|
Branch: | → branches/proxy-urlquote-3013 |
(In [22401]) Branching to 'proxy-urlquote-3013'
comment:3 Changed 11 years ago by
Keywords: | review added |
---|---|
Owner: | jknight deleted |
Priority: | high → highest |
comment:4 Changed 11 years ago by
Keywords: | review removed |
---|
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.
comment:5 Changed 11 years ago by
Owner: | set to Jean-Paul Calderone |
---|
comment:6 Changed 11 years ago by
Keywords: | review added |
---|---|
Owner: | Jean-Paul Calderone deleted |
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.
comment:7 Changed 11 years ago by
Keywords: | review removed |
---|---|
Owner: | set to Jean-Paul Calderone |
Why did you mangle the quote import as urlquote?
Otherwise, looks good.
comment:8 Changed 11 years ago by
15:06 < exarkun> therve: "quote" seems too vague to me 15:07 < therve> exarkun: alright 15:07 < therve> exarkun: you can merge then
comment:9 Changed 11 years ago by
Resolution: | → fixed |
---|---|
Status: | new → closed |
comment:10 Changed 8 years ago by
Owner: | Jean-Paul Calderone deleted |
---|
Huh, actually, I have no idea what the correct behavior WRT the fragment is. The quoting issue is real and serious, though.