[issue34711] return ENOTDIR when open() accepts filenames with a trailing slash

[Michael Felt]

Change by Michael Felt :


--
pull_requests: +9063

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue34711] return ENOTDIR when open() accepts filenames with a trailing slash

[Michael Felt]

Michael Felt  added the comment:

On 10/2/2018 7:36 PM, Michael Felt wrote:
> Python is designed as a thin wrapper to the operating system. IMHO Python 
> must not validate the filename itself.
To shorten the discussion, I'll kill the current PR and just modify the
test to skip the trailing slash test.

Assumption: if Python is not validating filenames, then a Module should
also not validate a filename.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue34711] return ENOTDIR when open() accepts filenames with a trailing slash

[Michael Felt]

Michael Felt  added the comment:

Was not my intent. Firewall issues. After 4 more attempts gave up until now.

On 10/2/2018 3:17 PM, STINNER Victor wrote:
> STINNER Victor  added the comment:
>
>> 2018-10-02 11:02:32  Michael.Feltset files: + mime-attachment, 
>> encrypted.asc
> You replied with an encrypted message which isn't understood by the bug 
> tracker.
>
> --
>
> ___
> Python tracker 
> 
> ___
>

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue34711] return ENOTDIR when open() accepts filenames with a trailing slash

[Michael Felt]

Michael Felt  added the comment:

On 10/2/2018 10:36 AM, STINNER Victor wrote:
> STINNER Victor  added the comment:
>
> 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."
> https://developercommunity.visualstudio.com/content/problem/272379/createfile-non-error-on-filename-with-trailing-bac.html
> 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:
>
> * open an existing file with an additional trailing antislash (U+005C): the 
> extra character is simply ignored and I am able to open the file
>
> * create a new file with an additional trailing antislash (U+005C): the 
> filename is created without the trailing antislash
>
> On the PR, I wrote:
>
>
>> There are much more functions which "open a file". Open Python/fileutils.c 
>> for a few mores. What about os.open()? What about all other functions which 
>> accept a filename and then call a third party library which calls open() 
>> directly?
> Ok, let me give some examples of function which directly open a file:
>
> * fileutils.c: _Py_open(), _Py_open_noraise(), _Py_wfopen(), _Py_fopen(), 
> _Py_fopen_obj()
> * os.open()
> * _io.FileIO, _pyio.FileIO (use os.open())
>
> Ok... But there are other functions to access files... stat()/fstat() 
> functions:
>
> * fileutils.c: _Py_fstat_noraise(), _Py_fstat(), _Py_stat()
> * Modules/getpath.c: _Py_wstat()
> * os.stat(), os.lstat(), os.fstat()
>
> 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:
>
> * _ssl.SSLContext.load_cert_chain()
> * sqlite3.connect()
> * etc.
>
> 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.
>
> --
>
>> Going back to issue17234 - there has been a test to check that a URL with a 
>> trailing slash reports 404 status.
> IMHO you must fix a single place: the SimpleHTTPServer, not all code handling 
> the filesytem.
What I did not know: do you want the test to be okay as is, or set
SkipIf(condition) for operating systems that accept filename/ as a
filename when it is not directory.

IMHO: the "statement" of the test is that the name "test/" MUST be
rejected in all cases.

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

Title :
SimpleHTTPServer serves files even if the URL has a trailing slash

I can assume the test was adjusted to catch the 404 return code, and let
that pass on OS that will not open .../test.txt and .../text.txt/ as the
same object - while AIX (and windows does).

As to your comment about "other" things that can happen - the message in
the test implies that Python only wants "filenames" that are opened
without a slash. I guess there is not a PEP on this, so anyones guess is
likely to be wrong for someone.

In short, this PR took the road where Python says .../filename/ is wrong
- per the comment in the test.

So, what should be done: code in HTTPServer that checks for trailing
slash and the test can be left ASIS, or modify the test to skip this one
aspect on an OS that is known to accept ".../filename/" as a valid filename?

I am not "core" enough to make this choice. I can attempt to code any
decision made by "python-dev" aka core-devs.
> Same remark for AIX and Windows junctions.
>

> I suggest to reject this issue.
per above, I would rather not reject the issue - I would still like to
see the test fixed. But need help on what should actually be modified.
Test, Module, or internals.
> --
>
> ___
> Python tracker 
> 
> ___
>

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue34711] return ENOTDIR when open() accepts filenames with a trailing slash

[Michael Felt]

Michael Felt  added the comment:

Final attempt to send as plain text

On 10/2/2018 1:07 AM, Benjamin Peterson wrote:
> On Mon, Oct 1, 2018, at 12:12, Michael Felt wrote:
>> Hi all,
>>
>> Before I submit a patch to increase the default MAXDATA setting for AIX
>> when in 32-bit mode - I want to know if I can put this LDFLAG setting in
>> LDLAST, or if I should introduce a new AC_SUBST() variable (e.g.,
>> LDMAXDATA).
> I think you should just put it in LDFLAGS.
I was wanting to avoid that, as LDFLAGS is an environmental variable.

At the surface, it appears Python is using PY_LDFLAGS (with
CONFIGURE_LDFLAGS coming from LDFLAGS during the ./configure moment.

A reason for a separate variable is that this particular option is only
relevant for the python EXE, and not for shared libraries and "other
things". IMHO, a reason for LDMAXDATA is because LDLAST is actually
already too widely used:

root@x066:[/data/prj/python/git/cpython-master]grep LDFLAGS *.in
Makefile.pre.in:CONFIGURE_LDFLAGS=  @LDFLAGS@
Makefile.pre.in:# Avoid assigning CFLAGS, LDFLAGS, etc. so users can use
them on the
Makefile.pre.in:# Both CPPFLAGS and LDFLAGS need to contain the shell's
value for setup.py to
Makefile.pre.in:PY_LDFLAGS= $(CONFIGURE_LDFLAGS) $(LDFLAGS)
Makefile.pre.in:LDSHARED=   @LDSHARED@ $(PY_LDFLAGS)
Makefile.pre.in:BLDSHARED=  @BLDSHARED@ $(PY_LDFLAGS)
Makefile.pre.in:OPENSSL_LDFLAGS=@OPENSSL_LDFLAGS@
Makefile.pre.in:    $(MAKE) @DEF_MAKE_RULE@ CFLAGS_NODIST="$(CFLAGS)
$(PGO_PROF_GEN_FLAG)" LDFLAGS="$(LDFLAGS) $(PGO_PROF_GEN_FLAG)"
LIBS="$(LIBS)"
Makefile.pre.in:    $(MAKE) @DEF_MAKE_RULE@ CFLAGS_NODIST="$(CFLAGS)
$(PGO_PROF_USE_FLAG)" LDFLAGS="$(LDFLAGS)"
Makefile.pre.in:    $(LINKCC) $(PY_LDFLAGS) $(LINKFORSHARED) -o $@
Programs/python.o $(BLDLIBRARY) $(LIBS) $(MODLIBS) $(SYSLIBS) $(LDLAST)
Makefile.pre.in: $(CC) -dynamiclib -Wl,-single_module
$(PY_LDFLAGS) -undefined dynamic_lookup
-Wl,-install_name,$(prefix)/lib/libpython$(LDVERSION).dylib
-Wl,-compatibility_version,$(VERSION) -Wl,-current_version,$(VERSION) -o
$@ $(LIBRARY_OBJS) $(SHLIBS) $(LIBC) $(LIBM) $(LDLAST); \
Makefile.pre.in:    $(CC) -o $(LDLIBRARY) $(PY_LDFLAGS) -dynamiclib \
Makefile.pre.in:    $(LINKCC) $(PY_LDFLAGS) $(LINKFORSHARED) -o $@
Programs/_testembed.o $(BLDLIBRARY) $(LIBS) $(MODLIBS) $(SYSLIBS) $(LDLAST)
Makefile.pre.in:    $(LINKCC) $(PY_LDFLAGS) -o $@
Programs/_freeze_importlib.o $(LIBRARY_OBJS_OMIT_FROZEN) $(LIBS)
$(MODLIBS) $(SYSLIBS) $(LDLAST)
Makefile.pre.in:    $(CC) $(OPT) $(PY_LDFLAGS) $(PGENOBJS)
$(LIBS) -o $(PGEN)

The ONLY line that needs $LDMAXDATA is:

Makefile.pre.in:    $(LINKCC) $(PY_LDFLAGS) -o $@
Programs/_freeze_importlib.o $(LIBRARY_OBJS_OMIT_FROZEN) $(LIBS)
$(MODLIBS) $(SYSLIBS) $(LDLAST) $(LDMAXDATA)

or set $(LDLAST) at the end rather than append $(LDMAXDATA)
>> I have not looked yet, but I was thinking that MAYBE! LDLAST is intended
>> as a last resort variable that can be modified in Makefile.
> LDLAST looks vestigial from OSF/1 support and should probably be removed.

On 10/2/2018 1:02 PM, Michael Felt wrote:
> Change by Michael Felt :
>
>
> Added file: https://bugs.python.org/file47840/mime-attachment
> Added file: https://bugs.python.org/file47841/encrypted.asc
>
> ___
> Python tracker 
> 
> ___
>

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue34711] return ENOTDIR when open() accepts filenames with a trailing slash

[STINNER Victor]

STINNER Victor  added the comment:

> 2018-10-02 11:02:32   Michael.Feltset files: + mime-attachment, 
> encrypted.asc

You replied with an encrypted message which isn't understood by the bug tracker.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue34711] return ENOTDIR when open() accepts filenames with a trailing slash

[Michael Felt]

Change by Michael Felt :


Added file: https://bugs.python.org/file47840/mime-attachment
Added file: https://bugs.python.org/file47841/encrypted.asc

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue34711] return ENOTDIR when open() accepts filenames with a trailing slash

[STINNER Victor]

STINNER Victor  added the comment:

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."
https://developercommunity.visualstudio.com/content/problem/272379/createfile-non-error-on-filename-with-trailing-bac.html

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:

* open an existing file with an additional trailing antislash (U+005C): the 
extra character is simply ignored and I am able to open the file

* create a new file with an additional trailing antislash (U+005C): the 
filename is created without the trailing antislash

On the PR, I wrote:


> There are much more functions which "open a file". Open Python/fileutils.c 
> for a few mores. What about os.open()? What about all other functions which 
> accept a filename and then call a third party library which calls open() 
> directly?

Ok, let me give some examples of function which directly open a file:

* fileutils.c: _Py_open(), _Py_open_noraise(), _Py_wfopen(), _Py_fopen(), 
_Py_fopen_obj()
* os.open()
* _io.FileIO, _pyio.FileIO (use os.open())

Ok... But there are other functions to access files... stat()/fstat() functions:

* fileutils.c: _Py_fstat_noraise(), _Py_fstat(), _Py_stat()
* Modules/getpath.c: _Py_wstat()
* os.stat(), os.lstat(), os.fstat()

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:

* _ssl.SSLContext.load_cert_chain()
* sqlite3.connect()
* etc.

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.

--

> Going back to issue17234 - there has been a test to check that a URL with a 
> trailing slash reports 404 status.

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue34711] return ENOTDIR when open() accepts filenames with a trailing slash

[Michael Felt]

Michael Felt  added the comment:

On 17/09/2018 16:00, Michael Felt wrote:
> And, now for the PR tests...

Had a review - many thanks, but before I press the resolve button -
Ihope someone can help me understand why the "Travis" etc, checks are
failing, e.g.,

./python -E -S -m sysconfig --generate-posix-vars ;\
if test $? -ne 0 ; then \
echo "generate-posix-vars failed" ; \
rm -f ./pybuilddir.txt ; \
exit 1 ; \
fi
clang -pthread -L/home/travis/multissl/openssl/1.1.0h/lib
-L/home/travis/multissl/openssl/1.1.0h/lib -Xlinker -export-dynamic -o
Programs/_testembed Programs/_testembed.o libpython3.8dm.a -lpthread
-ldl -lutil -lm -lm
Segmentation fault (core dumped)
generate-posix-vars failed

I have no clue how my relatively small change can have this kind of a
effect and then nothing moves forward. Very frustrating!
Thx,
Michael

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue34711] return ENOTDIR when open() accepts filenames with a trailing slash

[STINNER Victor]

Change by STINNER Victor :


--
nosy: +vstinner

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue34711] return ENOTDIR when open() accepts filenames with a trailing slash

[Michael Felt]

Change by Michael Felt :


--
keywords: +patch
pull_requests: +8782
stage:  -> patch review

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue34711] return ENOTDIR when open() accepts filenames with a trailing slash

[Michael Felt]

Michael Felt  added the comment:

OK - issue17324 (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 to

And, now for the PR tests...

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue34711] return ENOTDIR when open() accepts filenames with a trailing slash

[Michael Felt]

Change by Michael Felt :


--
components: +IO
title: Fix test_httpservers on AIX -> return ENOTDIR when open() accepts 
filenames with a trailing slash

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com