CGIHTTPServer module discard continuous '/' letters from params given by GET method.

[takayuki]

BPO	24657
Nosy	@berkerpeksag, @vadmium, @zhangyangyu
Files	test.py: Dump CGI environment cgihandler.diff: Fix CGIRequestHandler's uncorrect behavior of query component. cgihander.patch: Add a testcase and use partition

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/vadmium'
closed_at = <Date 2015-10-03.07:39:42.164>
created_at = <Date 2015-07-18.02:49:36.328>
labels = ['type-bug', 'library']
title = "CGIHTTPServer module discard continuous '/' letters from params given by GET method."
updated_at = <Date 2015-10-03.07:39:53.794>
user = 'https://bugs.python.org/takayuki'

bugs.python.org fields:

activity = <Date 2015-10-03.07:39:53.794>
actor = 'martin.panter'
assignee = 'martin.panter'
closed = True
closed_date = <Date 2015-10-03.07:39:42.164>
closer = 'martin.panter'
components = ['Library (Lib)']
creation = <Date 2015-07-18.02:49:36.328>
creator = 'takayuki'
dependencies = []
files = ['40541', '40573', '40585']
hgrepos = []
issue_num = 24657
keywords = ['patch']
message_count = 12.0
messages = ['246877', '251222', '251283', '251479', '251584', '251618', '251635', '252082', '252112', '252196', '252198', '252199']
nosy_count = 5.0
nosy_names = ['takayuki', 'python-dev', 'berker.peksag', 'martin.panter', 'xiang.zhang']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue24657'
versions = ['Python 2.7', 'Python 3.4', 'Python 3.5', 'Python 3.6']

[takayuki]

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.

[takayuki]

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.

[vadmium]

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).

[zhangyangyu]

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/'),

[zhangyangyu]

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.

[vadmium]

It would be good to have a regression test case for this one too.

[zhangyangyu]

Add the testcase and use str.partition.

[vadmium]

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 bpo-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.

[zhangyangyu]

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.

[python-dev]

New changeset 634fe6a90e0c by Martin Panter in branch '3.4':
Issue bpo-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 bpo-25232, bpo-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 bpo-25232, bpo-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 bpo-25232, bpo-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 bpo-25232, bpo-24657: Add NEWS to 3.6.0a1 section
https://hg.python.org/cpython/rev/3c006ee38287

[python-dev]

New changeset a4302005f9a2 by Martin Panter in branch '2.7':
Issue bpo-24657: Prevent CGIRequestHandler from collapsing the URL query
https://hg.python.org/cpython/rev/a4302005f9a2

[vadmium]

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.