bpo-38863: Improve is_cgi() in http.server#17312
Conversation
Sorry, something went wrong.
corona10
left a comment
corona10
left a comment
There was a problem hiding this comment.
I now understood what you want to say.
Can you please write the unit test for this?
You can easily provide the test case within inherit CGIHTTPRequestHandler
Sorry, something went wrong.
corona10
left a comment
corona10
left a comment
There was a problem hiding this comment.
Please add a news for changing.
You can follow the below guide.
https://devguide.python.org/committing/#what-s-new-and-news-entries
Thank you for the contribution
Sorry, something went wrong.
|
Dear core developers, @kkangshawn is a first-time contributor to this project. Thanks for your understanding. |
Sorry, something went wrong.
is_cgi() function of http.server library does not currently handle a cgi script if one of the cgi_directories is located at the sub-directory of given path. Since is_cgi() in CGIHTTPRequestHandler class separates given path into (dir, rest) based on the first seen '/', multi-level directories like /sub/dir/cgi-bin/hello.py is divided into head=/sub, rest=dir/cgi-bin/hello.py then check whether '/sub' exists in cgi_directories = [..., '/sub/dir/cgi-bin']. This patch makes the is_cgi() keep expanding dir part to the next '/' then checking if that expanded path exists in the cgi_directories. Signed-off-by: Siwon Kang <kkangshawn@gmail.com>
corona10
left a comment
corona10
left a comment
There was a problem hiding this comment.
I am okay with the current change, but the final decision will be decided by core developers.
In case, you should discuss the change will be applied or not on https://bugs.python.org/issue38863
I add the @ethanfurman as the cgi expert.
Sorry, something went wrong.
|
Thank you @corona10 for everything you have helped me on my first contribution to the python project! I expect to see you next :) |
Sorry, something went wrong.
|
Thanks for your contribution. |
Sorry, something went wrong.
corona10
added
type-feature
A feature request or enhancement
and removed
type-bug
An unexpected behavior, bug, or error
labels
Nov 22, 2019
|
This pull request introduced a reference leak in Reverting commit 91daa9d fixes the problem. I opened #17454 to fix the issue |
Sorry, something went wrong.
is_cgi() function of http.server library does not currently handle a cgi script if one of the cgi_directories is located at the sub-directory of given path. Since is_cgi() in CGIHTTPRequestHandler class separates given path into (dir, rest) based on the first seen '/', multi-level directories like /sub/dir/cgi-bin/hello.py is divided into head=/sub, rest=dir/cgi-bin/hello.py then check whether '/sub' exists in cgi_directories = [..., '/sub/dir/cgi-bin']. This patch makes the is_cgi() keep expanding dir part to the next '/' then checking if that expanded path exists in the cgi_directories. Signed-off-by: Siwon Kang <kkangshawn@gmail.com> https://bugs.python.org/issue38863
Reviewers
7 participants
is_cgi() function of http.server library does not currently handle a
cgi script if one of the cgi_directories is located at the
sub-directory of given path. Since is_cgi() in CGIHTTPRequestHandler
class separates given path into (dir, rest) based on the first seen
'/', multi-level directories like /sub/dir/cgi-bin/hello.py is divided
into head=/sub, rest=dir/cgi-bin/hello.py then check whether '/sub'
exists in cgi_directories = [..., '/sub/dir/cgi-bin'].
This patch makes the is_cgi() keep expanding dir part to the next '/'
then checking if that expanded path exists in the cgi_directories.
Signed-off-by: Siwon Kang kkangshawn@gmail.com
https://bugs.python.org/issue38863
Automerge-Triggered-By: @asvetlov