classification
Title: Trailing slash in sys.path cause import failure
Type: behavior Stage:
Components: Interpreter Core Versions: Python 3.0, Python 2.6, Python 2.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: brett.cannon Nosy List: brett.cannon, christian.heimes, facundobatista, georg.brandl, guillaumegirard, gvanrossum
Priority: low Keywords: patch

Created on 2007-10-18 12:47 by guillaumegirard, last changed 2007-11-07 17:46 by gvanrossum. This issue is now closed.

Files
File name Uploaded Description Edit
trailing_slash.patch christian.heimes, 2007-11-01 21:12
trailing_slash2.patch christian.heimes, 2007-11-01 21:53
trailing_slash3.patch christian.heimes, 2007-11-06 21:49
Messages (15)
msg56523 - (view) Author: Guillaume Girard (guillaumegirard) Date: 2007-10-18 12:47
On win32, the following code:

import sys
sys.path.append('../bar/')
import bar

where the file bar.py is present in ../bar/ will return an import error
"No module named bar". Remove the trailing slash and the bar.py is
imported correctly. The problem is identical with backslash. This code
works in Python 2.4.

Not a very serious bug, but it breaks programs working with Python 2.4.
msg57025 - (view) Author: Facundo Batista (facundobatista) * (Python committer) Date: 2007-11-01 18:36
Only in win32, in Linux it behaves ok (I put a /tmp/w.py that prints 'w'):

Python 2.5.1 (r251:54863, May  2 2007, 16:56:35) 
[GCC 4.1.2 (Ubuntu 4.1.2-0ubuntu4)] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import sys
>>> sys.path.append("/tmp/")
>>> import w
w
>>>
msg57030 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2007-11-01 19:49
I was able to reproduce the bug on Windows with Python 2.6 and 3.0. I've
added an unit test to both versions.
msg57038 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2007-11-01 21:12
Here is a patch that solves the problem. However the patch is against
the py3k sources and I like somebody to review and test it. I don't have
enough disk space in my VMWare box to test it against the trunk or 2.5.

Reason for the problem: Windows' stat doesn't like trailing slashes.
msg57039 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2007-11-01 21:53
Fixed patch. Georg pointed out that PyArg_ParseTuple("s") returns a
reference to the internal data of the PyString object. The new version
copies the path to a fixed width buffer before it mangles the trailing
slashes.

The new patch applies against the trunk.

Brett, you are the import master. Can you review the patch, please?
msg57044 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2007-11-02 01:45
I will have a look when I can.
msg57177 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2007-11-06 19:10
I see several problems with this, but there's light at the end of the
tunnel.

(1) Don't use s#; it will allow null bytes in the string, which we don't
want.

(2) Put the entire trailing slash removal inside #ifdef MS_WINDOWS.

(3) Careful!  It seems the code you wrote would transform "C:/" into
"C:" which isn't the same thing (the latter refers to the current
directory on the C drive, while the former is the root of the C drive).
msg57178 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2007-11-06 20:07
Guido van Rossum wrote:
> (1) Don't use s#; it will allow null bytes in the string, which we don't
> want.
> 
> (2) Put the entire trailing slash removal inside #ifdef MS_WINDOWS.

Done aand done

> (3) Careful!  It seems the code you wrote would transform "C:/" into
> "C:" which isn't the same thing (the latter refers to the current
> directory on the C drive, while the former is the root of the C drive).

Oh, good catch! I haven't thought of C:/

How do you like
#ifdef MS_WINDOWS
		/* Remove trailing / and \ - Windows' stat doesn't like them - but
		 * keep the trailing slash of C:/
		 */

		Py_ssize_t i;
		char mangled[MAXPATHLEN+1];

		if (pathlen > MAXPATHLEN) {
			PyErr_SetString(PyExc_OverflowError,
					"path is too long");
			return -1;
		}
		strcpy(mangled, path);

		for (i = pathlen-1; i > 3; i--) {
			if (mangled[i] != '/' && mangled[i] != '\\') {
				break;
			}
			mangled[i] = '\0';
		}
		rv = stat(mangled, &statbuf);
#else
		rv = stat(path, &statbuf);
#endif

i > 3 should take care of C:/ and C:\

Christian
msg57179 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2007-11-06 20:19
> How do you like
> #ifdef MS_WINDOWS
>                 /* Remove trailing / and \ - Windows' stat doesn't like them - but
>                  * keep the trailing slash of C:/
>                  */
>
>                 Py_ssize_t i;
>                 char mangled[MAXPATHLEN+1];
>
>                 if (pathlen > MAXPATHLEN) {
>                         PyErr_SetString(PyExc_OverflowError,
>                                         "path is too long");
>                         return -1;
>                 }
>                 strcpy(mangled, path);
>
>                 for (i = pathlen-1; i > 3; i--) {
>                         if (mangled[i] != '/' && mangled[i] != '\\') {
>                                 break;
>                         }
>                         mangled[i] = '\0';
>                 }
>                 rv = stat(mangled, &statbuf);
> #else
>                 rv = stat(path, &statbuf);
> #endif
>
> i > 3 should take care of C:/ and C:\

But the C: is optional. You will need to do more parsing. Some more
examples (all with slashes; but backslashes work the same):

//share/  -> //share/
//share/x/ -> //share/x
/x/ -> /x
/ -> /
// -> // (probably illegal but doesn't matter)
C:/ -> C:/
x/ -> x
C:x/ -> C:x

Isn't it easier to handle this at the Python level? There probably
already is code for parsing Windows paths. (Hm, maybe we have it in C
too somewhere?)
msg57182 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2007-11-06 21:49
Another patch that uses os.path.normpath.
msg57183 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2007-11-06 22:23
> Another patch that uses os.path.normpath.

Sorry, I really don't like to be importing os.path in order to be able
to process the path used for an import.
msg57185 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2007-11-06 22:42
On 11/6/07, Guido van Rossum <report@bugs.python.org> wrote:
>
> Guido van Rossum added the comment:
>
> > Another patch that uses os.path.normpath.
>
> Sorry, I really don't like to be importing os.path in order to be able
> to process the path used for an import.

Modules/getpath.c:joinpath() I think does what you want in C.
msg57199 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2007-11-07 14:08
Brett Cannon wrote:
> Modules/getpath.c:joinpath() I think does what you want in C.

I don't see how it is going to help us.

I've a final offer before I declare the bug as SEP (someone else's problem):

pseudo code

#ifdef MS_WINDOWS
    rv = stat(...)
    if (rv == error)
        check for *one* trailign / or \ and remove it
        rv = stat(...)
        if (rv == error)
            give up
#endif

It should fix the case when somebody adds a directory with a trailing /
or \ on Windows. More complex cases are still broken but that's really
not my concern. ;)

Christian
msg57203 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2007-11-07 16:00
Works for me!

On Nov 7, 2007 6:08 AM, Christian Heimes <report@bugs.python.org> wrote:
>
> Christian Heimes added the comment:
>
> Brett Cannon wrote:
> > Modules/getpath.c:joinpath() I think does what you want in C.
>
> I don't see how it is going to help us.
>
> I've a final offer before I declare the bug as SEP (someone else's problem):
>
> pseudo code
>
> #ifdef MS_WINDOWS
>     rv = stat(...)
>     if (rv == error)
>         check for *one* trailign / or \ and remove it
>         rv = stat(...)
>         if (rv == error)
>             give up
> #endif
>
> It should fix the case when somebody adds a directory with a trailing /
> or \ on Windows. More complex cases are still broken but that's really
> not my concern. ;)
>
> Christian
>
>
> __________________________________
> Tracker <report@bugs.python.org>
> <http://bugs.python.org/issue1293>
> __________________________________
>
msg57206 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2007-11-07 17:28
I've checked in a fix in r58903 (py3k branch).
History
Date User Action Args
2007-11-07 17:46:51gvanrossumsetstatus: open -> closed
resolution: fixed
2007-11-07 17:28:32christian.heimessetmessages: + msg57206
2007-11-07 16:00:30gvanrossumsetmessages: + msg57203
2007-11-07 14:08:02christian.heimessetmessages: + msg57199
2007-11-06 22:42:46brett.cannonsetmessages: + msg57185
2007-11-06 22:23:27gvanrossumsetmessages: + msg57183
2007-11-06 21:49:30christian.heimessetfiles: + trailing_slash3.patch
messages: + msg57182
2007-11-06 20:19:38gvanrossumsetmessages: + msg57179
2007-11-06 20:07:55christian.heimessetmessages: + msg57178
2007-11-06 19:10:10gvanrossumsetresolution: accepted -> (no value)
2007-11-06 19:10:04gvanrossumsetnosy: + gvanrossum
messages: + msg57177
2007-11-02 01:45:38brett.cannonsetmessages: + msg57044
2007-11-01 21:53:38christian.heimessetfiles: + trailing_slash2.patch
assignee: brett.cannon
messages: + msg57039
nosy: + brett.cannon
2007-11-01 21:12:36christian.heimessetkeywords: + patch
nosy: + georg.brandl
messages: + msg57038
files: + trailing_slash.patch
2007-11-01 19:49:32christian.heimessetpriority: low
resolution: accepted
messages: + msg57030
nosy: + christian.heimes
versions: + Python 2.6, Python 3.0
2007-11-01 18:36:41facundobatistasetnosy: + facundobatista
messages: + msg57025
2007-10-18 12:47:42guillaumegirardcreate