bpo-38863: Improve is_cgi() in http.server#17312
Conversation
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
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
|
Dear core developers, @kkangshawn is a first-time contributor to this project. Thanks for your understanding. |
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.
|
Thank you @corona10 for everything you have helped me on my first contribution to the python project! I expect to see you next :) |
|
Thanks for your contribution. |
corona10
added
type-feature
|
I get it. Thank you @asvetlov for the review! |
|
This pull request introduced a reference leak in Reverting commit 91daa9d fixes the problem. I opened #17454 to fix the issue |
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
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
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