Lib/http/server.py: Return HTTPStatus.NOT_FOUND if path.endswith(/) and not a directory #78892
Comments
|
Going back to bpo-17234 - there has been a test to check that a URL with a trailing slash reports 404 status. On AIX a trailing-slash is ignored - if the rest of the path is a valid filename. At a very low-level, in Modules/_io/fileio.c the code could be adjusted to always look for a trailing slash, and if AIX considers the name to be a file (S_ISREG(mode)) then this could be promoted to an error. just as it does for "posix" opens directories with trailing slashes (and fileio.c deals with that in a Python way). A quick fix is to skip the test bpo-17324 introduced; In short, the simple resolution - without impacting core in any way is:
diff --git a/Lib/test/test_httpservers.py b/Lib/test/test_httpservers.py
index cc829a522b..31be9b4479 100644
--- a/Lib/test/test_httpservers.py
+++ b/Lib/test/test_httpservers.py
@@ -13,6 +13,7 @@ import sys
import re
import base64
import ntpath
+import platform
import shutil
import email.message
import email.utils
@@ -29,6 +30,7 @@ from io import BytesIO
import unittest
from test import support+AIX = platform.system() == 'AIX' class NoLogRequestHandler:
def log_message(self, *args):
@@ -417,9 +419,11 @@ class SimpleHTTPServerTestCase(BaseTestCase):
#constructs the path relative to the root directory of the HTTPServer
response = self.request(self.base_url + '/test')
self.check_status_and_reason(response, HTTPStatus.OK, data=self.data)
- # check for trailing "/" which should return 404. See Issue17324
- response = self.request(self.base_url + '/test/')
- self.check_status_and_reason(response, HTTPStatus.NOT_FOUND)
+ # AIX open() will open a filename with a trailing slash - as a file
+ if not AIX:
+ # check for trailing "/" which should return 404. See Issue17324
+ response = self.request(self.base_url + '/test/')
+ self.check_status_and_reason(response, HTTPStatus.NOT_FOUND)
response = self.request(self.base_url + '/')
self.check_status_and_reason(response, HTTPStatus.OK)
response = self.request(self.base_url)
@@ -519,8 +523,9 @@ class SimpleHTTPServerTestCase(BaseTestCase):
def test_path_without_leading_slash(self):
response = self.request(self.tempdir_name + '/test')
self.check_status_and_reason(response, HTTPStatus.OK, data=self.data)
- response = self.request(self.tempdir_name + '/test/')
- self.check_status_and_reason(response, HTTPStatus.NOT_FOUND)
+ if not AIX:
+ response = self.request(self.tempdir_name + '/test/')
+ self.check_status_and_reason(response, HTTPStatus.NOT_FOUND)
response = self.request(self.tempdir_name + '/')
self.check_status_and_reason(response, HTTPStatus.OK)
response = self.request(self.tempdir_name)Comments! |
aixtools
added
tests
|
This is also an issue on Windows when the target path resides within a junction, paths outside of a junction respond (err, fail) as expected. |
|
On 17/09/2018 12:47, Jeremy Kloth wrote:
[ENOTDIR] from http://pubs.opengroup.org/onlinepubs/9699919799/functions/open.html
|
aixtools
changed the title
|
OK - bpo-17324 (not 1 7 2 3 4) And, as jkloth reported that Windows also has an issue - sometimes, found a way to do this in Modules/_io/fileio.c diff --git a/Modules/_io/fileio.c b/Modules/_io/fileio.c
index c0e43e0ae4..3623ff16ea 100644
--- a/Modules/_io/fileio.c
+++ b/Modules/_io/fileio.c
@@ -228,6 +228,7 @@ _io_FileIO___init___impl(fileio *self, PyObject *nameobj, const char *mode,
#endif
PyObject *stringobj = NULL;
const char *s;
+ char *rcpt;
int ret = 0;
int rwa = 0, plus = 0;
int flags = 0;
@@ -447,6 +448,23 @@ _io_FileIO___init___impl(fileio *self, PyObject *nameobj, const char *mode,
}
}
else {
+#if defined(S_ISREG) && defined(ENOTDIR)
+ /* On AIX and Windows, open may succeed for files with a trailing slash.
+ The Open Group specifies filenames ending with a trailing slash should
+ be an error - ENOTDIR */
+ if (S_ISREG(fdfstat.st_mode)) {
+#ifdef MS_WINDOWS
+ rcpt= strrch(widename, '\');
+#else
+ rcpt = strrchr(name, '/');
+#endif
+ if ((rcpt != NULL) && (strlen(rcpt) == 1)) {
+ errno = ENOTDIR;
+ PyErr_SetFromErrnoWithFilenameObject(PyExc_OSError, nameobj);
+ goto error;
+ }
+ }
+#endif
#if defined(S_ISDIR) && defined(EISDIR)
/* On Unix, open will succeed for directories.
In Python, there should be no file objects referring toAnd, now for the PR tests... |
|
On 17/09/2018 16:00, Michael Felt wrote:
Had a review - many thanks, but before I press the resolve button - ./python -E -S -m sysconfig --generate-posix-vars ;\ I have no clue how my relatively small change can have this kind of a |
|
Jeremy Kloth: "This is also an issue on Windows when the target path resides within a junction, paths outside of a junction respond (err, fail) as expected." I don't know the behavior on Windows. I tried to create a file name "a\" (U+0061, U+005C): I get an OSError "invalid argument" (error 22). I confirm that in a junction point, I am able to:
On the PR, I wrote:
Ok, let me give some examples of function which directly open a file:
Ok... But there are other functions to access files... stat()/fstat() functions:
To start to have a better ideas of how many functions accept filenames, open also Lib/shutil.py. shutil.copyfile() uses os.stat(), but then it uses os.symlink() and open()... So what about os.symlink()? Ok, here I only listen a *few* examples of functions which are "controlled" by Python. But there are *many* wrappers to 3rd party libraries which accept a filename. Examples:
Where is the limit? How many functions must be patched in Python? How do we patch OpenSSL and SQLite libraries? Python is designed as a thin wrapper to the operating system. IMHO Python must not validate the filename itself. --
IMHO you must fix a single place: the SimpleHTTPServer, not all code handling the filesytem. Same remark for AIX and Windows junctions. I suggest to reject this issue. |
You replied with an encrypted message which isn't understood by the bug tracker. |
|
Final attempt to send as plain text On 10/2/2018 1:07 AM, Benjamin Peterson wrote:
At the surface, it appears Python is using PY_LDFLAGS (with A reason for a separate variable is that this particular option is only root@x066:[/data/prj/python/git/cpython-master]grep LDFLAGS *.in The ONLY line that needs $LDMAXDATA is: Makefile.pre.in: or set
On 10/2/2018 1:02 PM, Michael Felt wrote:
|
|
On 10/2/2018 10:36 AM, STINNER Victor wrote:
IMHO: the "statement" of the test is that the name "test/" MUST be # check for trailing "/" which should return 404. See bpo-17324 Title <http://docs.python.org/devguide/triaging.html#title\>: I can assume the test was adjusted to catch the 404 return code, and let As to your comment about "other" things that can happen - the message in In short, this PR took the road where Python says .../filename/ is wrong
So, what should be done: code in HTTPServer that checks for trailing I am not "core" enough to make this choice. I can attempt to code any
|
|
Was not my intent. Firewall issues. After 4 more attempts gave up until now. On 10/2/2018 3:17 PM, STINNER Victor wrote:
|
|
On 10/2/2018 7:36 PM, Michael Felt wrote:
Assumption: if Python is not validating filenames, then a Module should |
|
Changed the title to reflect the requested change is only in Tests. |
aixtools
changed the title
This path-parsing bug that strips a trailing slash occurs when a traversed directory is a reparse point, such as a mount-point (junction) or directory symbolic link. It may be limited to just the NTFS and ReFS file-system drivers, which is all I tested. Or it may be a bug in the I/O manager itself, which actually implements the name grafting for Microsoft's mount-point (0xA0000003) and symlink (0xA000000C) reparse tags. I thought the suggested patch was ok because it followed the pattern of existing code that prevents opening directories on Unix. But I can see the distinction in that the latter is implementing behavior policy rather than working around a platform bug. Also, while _Py_open and _Py_fopen do allow opening a directory on a Linux box, in apparent violation of this policy, these are private, internal functions, and there's no use that I'm aware of in the CPython implementation for directory FDs or FILE streams opened this way. |
|
Hi Michael, I agree with Victor that the best place to fix the problem is in the HTTP server module. In other words, the “medium fix” you mentioned in your original post. Your recent proposal to just skip the test means that AIX will continue to suffer from the original bug. |
|
Closed "test" version. made new PR that makes server.py conform to bpo-17234 demands |
aixtools
added
stdlib
aixtools
changed the title
|
Could this also be backported to Version 3.7 and 3.6? |
|
Hi Michael, I think no, because 3.6 is in security mode. |
|
On 11/03/2019 09:42, Stéphane Wirtel wrote:
|
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:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: