[issue10484] http.server.is_cgi fails to handle CGI URLs containing PATH_INFO
Glenn Linderman v+pyt...@g.nevcal.com added the comment: A minor detail of efficiency: _url_collapse_path_split is used only one place, from is_cgi. _url_collapse_path_split splits the path, and is_cgi now puts it back together. This seems inefficient. Would it not be better for _url_collapse_path_split to be renamed to _url_collapse_path and simply return a single value, the path canonicalized (.. removed)? Then is_cgi would not need to put it back together before taking it apart another way. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue10484 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue10484] http.server.is_cgi fails to handle CGI URLs containing PATH_INFO
Glenn Linderman v+pyt...@g.nevcal.com added the comment:
I note that there is no test for tail_part == '.'. I suggest adding a couple,
such as the following which I added to my local copy for testing of the next
item:
'/a/b/.': ('/a/b', ''),
'/a/b/c/../d/e/../../../../f/../.': ('/', ''),
Given your comment that you found bugs in my code, I figured out how to run the
tests against my code, and found the bugs. Here is a slightly shorter, more
efficient (it cuts 40% off the execution time, see time_test.py), and to me
much clearer version of _url_collapse_path_split, and it passes all the tests:
def _url_collapse_path_split(path):
# Similar to os.path.split(os.path.normpath(path)) but specific to URL
# path semantics rather than local operating system semantics.
path_parts = path.split('/')
head_parts = []
for part in path_parts[:-1]:
if part == '..':
head_parts.pop() # IndexError if more '..' than prior parts
elif part and part != '.':
head_parts.append( part )
if path_parts:
tail_part = path_parts.pop()
if tail_part:
if tail_part == '..':
head_parts.pop()
tail_part = ''
elif tail_part == '.':
tail_part = ''
else:
tail_part = ''
return ('/' + '/'.join(head_parts), tail_part)
--
Added file: http://bugs.python.org/file25175/time_test.py
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue10484
___
___
Python-bugs-list mailing list
Unsubscribe:
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue10484] http.server.is_cgi fails to handle CGI URLs containing PATH_INFO
Roundup Robot devn...@psf.upfronthosting.co.za added the comment: New changeset c67efb8ffca4 by Senthil Kumaran in branch '2.7': Issue 10484 - Incorporate improvements to CGI module - Suggested by Glenn Linderman. Refactor code and tests http://hg.python.org/cpython/rev/c67efb8ffca4 New changeset fc001124a3ee by Senthil Kumaran in branch '3.2': 3.2 - Issue 10484 - Incorporate improvements to CGI module - Suggested by Glenn Linderman. Refactor code and tests http://hg.python.org/cpython/rev/fc001124a3ee New changeset 23f648d7053b by Senthil Kumaran in branch 'default': merge to default - Issue 10484 - Incorporate improvements to CGI module - Suggested by Glenn Linderman. Refactor code and tests http://hg.python.org/cpython/rev/23f648d7053b -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue10484 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue10484] http.server.is_cgi fails to handle CGI URLs containing PATH_INFO
Senthil Kumaran sent...@uthcode.com added the comment: Hello Glenn, Your suggestions were valid. I have made those changes in the code. Thanks! I think, we can close this bug now and move over to others. Thanks, Senthil -- status: open - closed ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue10484 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue10484] http.server.is_cgi fails to handle CGI URLs containing PATH_INFO
Roundup Robot devn...@psf.upfronthosting.co.za added the comment: New changeset 89eeaa18700f by Senthil Kumaran in branch '2.7': fix the incorrect changes made for PATH_INFO value - Issue10484 http://hg.python.org/cpython/rev/89eeaa18700f New changeset 827a4062b1d6 by Senthil Kumaran in branch '3.2': 3.2- fix the incorrect changes made for PATH_INFO value - Issue10484 http://hg.python.org/cpython/rev/827a4062b1d6 New changeset 8bcc5768bc05 by Senthil Kumaran in branch 'default': merge - fix the incorrect changes made for PATH_INFO value - Issue10484 http://hg.python.org/cpython/rev/8bcc5768bc05 -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue10484 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue10484] http.server.is_cgi fails to handle CGI URLs containing PATH_INFO
Senthil Kumaran sent...@uthcode.com added the comment: I had to carefully review a lot of changes for this. In addition, I did setup a apache and tested the behaviors too. Few notes - - The test for _url_collapse_path_split is not directly helpful, especially for PATH_INFO. I removed my previous changes I had made as I think, it is a mistake to test PATH_INFO in there. - There is not a test for path_info, it should be added. I could test only on local apache setup. - Coming the changes itself, the previous _url_collapse_path_split was just fine for what it is supposed to do. Return head and tail for certain definitions of parsing (removing . and .. and tail is last part and rest is head). - I incorporated Glenn Linderman logic in is_cgi to have explicit head and tail. Glenn's comment and cgi directories could overridden is important and in that ascept the previous commit had to be changed. I also looked at changes in the patches contained in issue 13893, but I found that those broke behavior or exhibited the security issue again. taking care of all these, I have the made the changes to the cgi module and also verified pratically that PATH_INFO is indeed sent correctly. Ran python -m CGIHttpServer and placed cgi-bin/file1.py and sent some requests to verify PATH_INFO. The values were proper. With this, I think this issue can be closed. The rest of the cgi changes can be handled in their respective issues. A test to PATH_INFO could just be added. -- Added file: http://bugs.python.org/file25171/file1.py ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue10484 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue10484] http.server.is_cgi fails to handle CGI URLs containing PATH_INFO
Glenn Linderman v+pyt...@g.nevcal.com added the comment: Senthil, thanks for your work on this. Regarding: I also looked at changes in the patches contained in issue 13893, but I found that those broke behavior or exhibited the security issue again. I'd be curious to know what problems you discovered, since I had hoped I had fixed them all (and know I fixed enough to work in my environment). I'll review your code in the next while, and attempt to include it in mine, and test it in my environment... I really don't want to maintain my own code, but do need functional code... hopefully once the other issues are fixed I can just use released code. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue10484 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue10484] http.server.is_cgi fails to handle CGI URLs containing PATH_INFO
Senthil Kumaran sent...@uthcode.com added the comment: To note in the tracker, the original 2.6 code was changed in Issue 2254 and the relevant changeset is c6c4398293bd I find, Glenn's suggestion a possibly okay solution for PATH_INFO problem, keeping in tact the security issue the the previous change dealt with. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue10484 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue10484] http.server.is_cgi fails to handle CGI URLs containing PATH_INFO
Senthil Kumaran sent...@uthcode.com added the comment: Actually no. I revert back my previous statement. Meddling with splitpath breaks all the tests. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue10484 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue10484] http.server.is_cgi fails to handle CGI URLs containing PATH_INFO
Glenn Linderman v+pyt...@g.nevcal.com added the comment: I'm not sure what tests break or why, I suppose it is the ones for the security patch. I'm not sure which code you are referring to... the code in msg122259 I tested only cursorily, the (different) code in issue 13893 is what I'm running today, and so has been tested more heavily, but I haven't run the regression tests (haven't figured out how). The latter code fixes the _url_collapse_path_split function, instead of working around it, which the code in msg122259 tries to do. I would be highly interested if it fails any tests, and why, and would be glad to help analyze and fix. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue10484 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue10484] http.server.is_cgi fails to handle CGI URLs containing PATH_INFO
Roundup Robot devn...@psf.upfronthosting.co.za added the comment: New changeset bab9f29c93fd by Senthil Kumaran in branch '2.7': 2.7 - Issue #10484: Fix the CGIHTTPServer's PATH_INFO handling problem http://hg.python.org/cpython/rev/bab9f29c93fd New changeset 88c86869ce92 by Senthil Kumaran in branch '3.2': closes issue10484 - Fix the http.server's cgi PATH_INFO handling problem http://hg.python.org/cpython/rev/88c86869ce92 New changeset 13c44ad094b4 by Senthil Kumaran in branch 'default': closes issue10484 - Fix the http.server's cgi PATH_INFO handling problem http://hg.python.org/cpython/rev/13c44ad094b4 -- nosy: +python-dev resolution: - fixed stage: - committed/rejected status: open - closed ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue10484 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue10484] http.server.is_cgi fails to handle CGI URLs containing PATH_INFO
Senthil Kumaran sent...@uthcode.com added the comment: I treated this as a regression from 2.6. A cgi url with /cgi-bin/hello.py/hello/world would have PATH_INFO as '/hello/world' I saw some apache specs saying that it should be 'hello/world', but I relied on python2.6's behavior with had initial '/'. Glenn, I referred to Lib/test/test_httpservers.py. Thanks for raising this and other issues (and patches)! -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue10484 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue10484] http.server.is_cgi fails to handle CGI URLs containing PATH_INFO
Glenn Linderman v+pyt...@g.nevcal.com added the comment:
In reviewing my code in this area, I also see that in addition to fixing
_url_collapse_path_split, I override the location that uses it, which is the
is_cgi function. Here is my code for the override, which actually creates a
proper PATH_INFO string:
def is_cgi(self):
Test whether self.path corresponds to a CGI script.
Returns True and updates the cgi_info attribute to the tuple
(dir, rest) if self.path requires running a CGI script.
Returns False otherwise.
If any exception is raised, the caller should assume that
self.path was rejected as invalid and act accordingly.
The default implementation tests whether the normalized url
path begins with one of the strings in self.cgi_directories
(and the next character is a '/' or the end of the string).
splitpath = server._url_collapse_path_split(self.path)
# more processing required due to possible PATHINFO parts
# not clear above function really does what is needed here,
# nor just how general it is!
splitpath = '/'.join( splitpath ).split('/', 2 )
head = '/' + splitpath[ 1 ]
tail = splitpath[ 2 ]
if head in self.cgi_directories:
self.cgi_info = head, tail
return True
return False
I have no idea what applications might depend on the improper handling of
PATH_INFO that the current code is performing, so that is why I applied my fix
for that in my overridden code, rather than in the server.py source file.
It may be that the actual fix for this issue is in the overridden code above
(but the fix to _url_collapse_path_split also seemed necessary, there was a
corner case that it did incorrectly, but after 16 months, I couldn't tell you
what that corner case was, any more.
Yes, the biggest issue here was the regression from 2.6, the security fix
seemed to break the PATH_INFO feature.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue10484
___
___
Python-bugs-list mailing list
Unsubscribe:
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue10484] http.server.is_cgi fails to handle CGI URLs containing PATH_INFO
Glenn Linderman v+pyt...@g.nevcal.com added the comment: Senthil, the patch you submitted breaks the encapsulation of cgi_directories -- this should be changeable per particular application via subclassing or assignment, but you've built those directories into the code. Perhaps you should take a look at my code for is_cgi (which I was trying to post while you were posting the patches, sorry I was slow). Perhaps there should be one more test, which actually changes the cgi_directories, and tests a few more things, to prove that the code does not hard code those directories (it shouldn't). -- status: closed - open ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue10484 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue10484] http.server.is_cgi fails to handle CGI URLs containing PATH_INFO
Glenn Linderman v+pyt...@g.nevcal.com added the comment: Another issue with the patch, is that it doesn't do .. and . collapsing on the PATH_INFO part of the path. It is possible for a path like /cgi-bin/script.py/../../plain-file.html to be passed to the server. I guess the question is if it should serve plain-file.html or if it should pass ../../plain-file.html to script.py as its PATH_INFO. I would think the former would be appropriate. I would have to do research to determine if some standard states otherwise. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue10484 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue10484] http.server.is_cgi fails to handle CGI URLs containing PATH_INFO
Glenn Linderman v+pyt...@g.nevcal.com added the comment: I just tested what Apache does with such a path as /cgi-bin/script.py/../../plain-file.html, and it serves the plain-file.html. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue10484 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue10484] http.server.is_cgi fails to handle CGI URLs containing PATH_INFO
Glenn Linderman v+pyt...@g.nevcal.com added the comment:
Senthil, from you patch, I discovered where the test_httpservers.py lives, and
added
'/cgi-bin/file1.py/../../a': ('/', 'a'),
to a local copy, and it worked fine against whatever version of the server and
tests it was running against... but that was in test_url_collapse_path_split
... I haven't figured out the full environment of the testing, or even if it
launches an HTTP SERVER or only unittests particular functions within it, but
it seems that that particular test is just testing the one function. I'm not
clear on where the tests actually test the activities of is_cgi or the creation
of PATH_INFO, if that occurs. What time I've spent working with Python can be
characterized as 99% application, 0.999% Python source to figure out how things
work (or fail), .001% test environment (just now). So I'm pretty ignorant of
the test environment, although I've spent 30+ years programming, only about 4
has been in Python, and that not full time.
It is not clear to me, though, that the test you added:
'/cgi-bin/file1.py/PATH-INFO': ('/cgi-bin', 'file1.py/PATH-INFO'),
should be the expected results of _url_collapse_path_split...
I would expect
'/cgi-bin/file1.py/a/b/c/../../d': ('/cgi-bin/file1.py/a', 'd'),
but I don't think it would, with your present patch. It looks like it would
return ('/cgi-bin', 'file1.py/a/b/c/../../d') which reintroduces the security
problem: the '..' stuff has not been processed. I haven't applied your patch to
actually run it, this is just from inspection, and I may have missed something.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue10484
___
___
Python-bugs-list mailing list
Unsubscribe:
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue10484] http.server.is_cgi fails to handle CGI URLs containing PATH_INFO
Giovanni Funchal gafunc...@gmail.com added the comment: This is still an issue as of python 3.2.2 and is affecting me. -- nosy: +Giovanni.Funchal ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue10484 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue10484] http.server.is_cgi fails to handle CGI URLs containing PATH_INFO
Senthil Kumaran sent...@uthcode.com added the comment: Oh Sorry. I shall fix this by this weekend. -- assignee: - orsenthil ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue10484 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue10484] http.server.is_cgi fails to handle CGI URLs containing PATH_INFO
Glenn Linderman v+pyt...@g.nevcal.com added the comment: issue 13893 contains code that fixes this, and several other open issues. Sadly, I created that code by debugging and rewriting until it worked, and only then teased apart the specific, separable issues that I had debugged and fixed, and created issues. I'm not up to speed on the Python development process, but feel free to borrow any or all of my code in issue 13893, which contains the fixes for all the following issues: issue 10483 issue 10484 (this issue) issue 10485 issue 10486 (creates more standard Environment variables, not sure all) issue 10487 I would certainly like to see all of these issues fixed, rather than continue to maintain my own code in parallel to various Python releases. Whether they are fixed with my code, or with other code, is immaterial to me, but my code has been running for over a year in my environments without discovering additional bugs. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue10484 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue10484] http.server.is_cgi fails to handle CGI URLs containing PATH_INFO
Glenn Linderman v+pyt...@g.nevcal.com added the comment:
Here is a replacement for the body of is_cgi that will work with the current
_url_collapse_path_split function, but it seems to me that it is ineffecient to
do multiple splits and joins of the path between the two functions.
splitpath = server._url_collapse_path_split(self.path)
# more processing required due to possible PATHINFO parts
# not clear above function really does what is needed here,
# nor just how general it is!
splitpath = '/'.join( splitpath ).split('/', 2 )
head = '/' + splitpath[ 1 ]
tail = splitpath[ 2 ]
if head in self.cgi_directories:
self.cgi_info = head, tail
return True
return False
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue10484
___
___
Python-bugs-list mailing list
Unsubscribe:
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue10484] http.server.is_cgi fails to handle CGI URLs containing PATH_INFO
Changes by Antoine Pitrou pit...@free.fr: -- nosy: +facundobatista, fdrake, orsenthil versions: +Python 3.1 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue10484 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue10484] http.server.is_cgi fails to handle CGI URLs containing PATH_INFO
New submission from Glenn Linderman v+pyt...@g.nevcal.com: is_cgi doesn't properly handle PATH_INFO parts of the path. The Python2.x CGIHTTPServer.py had this right, but the introduction and use of _url_collapse_path_split broke it. _url_collapse_path_split splits the URL into a two parts, the second part is guaranteed to be a single path component, and the first part is the rest. However, URLs such as /cgi-bin/foo.exe/this/is/PATH_INFO/parameters can and do want to exist, but the code in is_cgi will never properly detect that /cgi-bin/foo.exe is the appropriate executable, and the rest should be PATH_INFO. This used to work correctly in the precedecessor CGIHTTPServer.py code in Python 2.6, so is a regression. -- components: Library (Lib) messages: 121876 nosy: v+python priority: normal severity: normal status: open title: http.server.is_cgi fails to handle CGI URLs containing PATH_INFO type: behavior versions: Python 3.2 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue10484 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
