classification
Title: CGIHTTPServer module discard continuous '/' letters from params given by GET method.
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.6, Python 3.4, Python 3.5, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: martin.panter Nosy List: berker.peksag, martin.panter, python-dev, takayuki, xiang.zhang
Priority: normal Keywords: patch

Created on 2015-07-18 02:49 by takayuki, last changed 2015-10-03 07:39 by martin.panter. This issue is now closed.

Files
File name Uploaded Description Edit
test.py martin.panter, 2015-09-22 02:04 Dump CGI environment
cgihandler.diff xiang.zhang, 2015-09-25 15:26 Fix CGIRequestHandler's uncorrect behavior of query component. review
cgihander.patch xiang.zhang, 2015-09-26 03:47 Add a testcase and use partition review
Messages (12)
msg246877 - (view) Author: (takayuki) Date: 2015-07-18 02:49
I executed CGIHTTPServer and requested the following URI,
"http://localhost:8000/cgi-bin/test.py?k=aa%2F%2Fbb"
to pass "aa//bb" as argument "k",
but test.py received "aa/bb".

I looked in CGIHTTPServer.py and found _url_collapse_path function
discards continuous slash letters even they are in the given parameters.
msg251222 - (view) Author: (takayuki) Date: 2015-09-21 12:54
This bug seems to remain in Python 3.5.0.

How to reproduce:

1. Save the attached cgitest.py into cgi-bin directory and changed it to executable file by "chmod +x cgitest.py"

2. Run CGIHTTPRequestHandler
[GCC 5.1.1 20150618 (Red Hat 5.1.1-4)] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import http.server
>>> http.server.test(HandlerClass=http.server.CGIHTTPRequestHandler)

3. Visit http://localhost:8000/cgi-bin/cgitest.py by any browser.

4. Input "a/b/c//d//e///f///g" to form named "p".

5. The continuous slash letters are trimed and "a/b/c/d/e/f/g" is given to cgitest.py.
msg251283 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015-09-22 02:04
Yes it also seems to apply to Python 3.

Perhaps you forgot your test script, so I made my own. After running

python3 -m http.server --cgi

The response from the following URL has no double slashes to be seen:

http://localhost:8000/cgi-bin/test.py//x//y//?k=aa%2F%2Fbb&//q//p//=//a//b//

I am not a CGI expert, but I suspect the query string bits should have double slashes, but maybe the PATH_INFO is right not to (see RFC 3875).
msg251479 - (view) Author: Xiang Zhang (xiang.zhang) * (Python committer) Date: 2015-09-24 03:03
I think this is a bug. 

According to the rfcs, "/" is a reserved character in query component and continuous "/" in query component may be invalid and how to deal with it depends on the server. But encoded "/", %2F, acts as data and should be preserved. And from rfc3875, QUERY_STRING must be passed encoded.

I tested in apache2.4 with martin's script, query string is:

('QUERY_STRING', 'k=aa%2F%2Fbb&//q//p//=//a//b//')

In python's CGI server, it is:

('QUERY_STRING', 'k=aa/bb&/q/p/=/a/b/'),
msg251584 - (view) Author: Xiang Zhang (xiang.zhang) * (Python committer) Date: 2015-09-25 15:26
The path with query component are unquoted entirely and then pass into
_url_collapse_path.
I think this behaviour is wrong and according to rfc3875 query component
should be left encoded in QUERY_STRING.
This patch seems to solve the problem. It passes the tests and with
martin's script, it gets:

('QUERY_STRING', 'k=aa%2F%2Fbb&//q//p//=//a//b//')

has the same behaviour with apache.
msg251618 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015-09-25 22:10
It would be good to have a regression test case for this one too.
msg251635 - (view) Author: Xiang Zhang (xiang.zhang) * (Python committer) Date: 2015-09-26 03:47
Add the testcase and use str.partition.
msg252082 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015-10-02 03:42
The patch looks like it will fix this particular bug without much negative impact. However there are plenty of other problems with this module’s URL handling, see Issue 14567. I think the translate_path(), _url_collapse_path(), is_cgi(), run_cgi(), etc functions all need a good rewrite.

Anyway it might be worth going ahead and committing this straight away, whether or not anyone is motivated to fix the wider issue later on.
msg252112 - (view) Author: Xiang Zhang (xiang.zhang) * (Python committer) Date: 2015-10-02 12:30
Yes, there seems to still exist some defects not conforming to the
specification. I would like to investigate it. Maybe I can propose
a patch for it.
msg252196 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2015-10-03 06:44
New changeset 634fe6a90e0c by Martin Panter in branch '3.4':
Issue #24657: Prevent CGIRequestHandler from collapsing the URL query
https://hg.python.org/cpython/rev/634fe6a90e0c

New changeset ba1e3c112e42 by Martin Panter in branch '3.5':
Issues #25232, #24657: Merge two CGI server fixes from 3.4 into 3.5
https://hg.python.org/cpython/rev/ba1e3c112e42

New changeset 88918f2a54df by Martin Panter in branch '3.5':
Issues #25232, #24657: Use new enum status to match rest of tests
https://hg.python.org/cpython/rev/88918f2a54df

New changeset 0f03023d4318 by Martin Panter in branch 'default':
Issues #25232, #24657: Merge two CGI server fixes from 3.5
https://hg.python.org/cpython/rev/0f03023d4318

New changeset 3c006ee38287 by Martin Panter in branch 'default':
Issues #25232, #24657: Add NEWS to 3.6.0a1 section
https://hg.python.org/cpython/rev/3c006ee38287
msg252198 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2015-10-03 07:27
New changeset a4302005f9a2 by Martin Panter in branch '2.7':
Issue #24657: Prevent CGIRequestHandler from collapsing the URL query
https://hg.python.org/cpython/rev/a4302005f9a2
msg252199 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015-10-03 07:39
Thanks everyone for the reports and patches. There were a couple of subtle compatibility tweaks needed for the 3.4 and 2.7 branches, but I think I got them all.
History
Date User Action Args
2015-11-11 05:48:59martin.panterlinkissue24661 superseder
2015-10-03 07:39:53martin.pantersetstage: commit review -> resolved
2015-10-03 07:39:42martin.pantersetstatus: open -> closed
resolution: fixed
messages: + msg252199
2015-10-03 07:27:40python-devsetmessages: + msg252198
2015-10-03 06:44:36python-devsetnosy: + python-dev
messages: + msg252196
2015-10-03 05:27:55martin.pantersetassignee: martin.panter

nosy: + berker.peksag
stage: patch review -> commit review
2015-10-02 12:30:59xiang.zhangsetmessages: + msg252112
2015-10-02 03:42:41martin.pantersetmessages: + msg252082
2015-09-26 03:47:52xiang.zhangsetfiles: + cgihander.patch

messages: + msg251635
2015-09-25 22:10:18martin.pantersetmessages: + msg251618
stage: needs patch -> patch review
2015-09-25 15:26:40xiang.zhangsetfiles: + cgihandler.diff
keywords: + patch
messages: + msg251584
2015-09-24 03:03:06xiang.zhangsetnosy: + xiang.zhang
messages: + msg251479
2015-09-22 02:04:16martin.pantersetfiles: + test.py

type: behavior
versions: + Python 3.4, Python 3.5, Python 3.6
nosy: + martin.panter

messages: + msg251283
stage: needs patch
2015-09-21 12:54:27takayukisetmessages: + msg251222
2015-07-18 02:49:36takayukicreate