File paths containing ':' messes up loader

Bug #405687 reported by Valentin Lab
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
ZConfig
Fix Released
Undecided
Unassigned

Bug Description

method "isPath" of BaseLoader raised an Exception:

..
File "/home/user/hg/my-app-10:30:00/eggs/ZConfig/loader.py", line 138, in isPath
TypeError: object of type 'NoneType' has no len()

The key factor to trigger the bug is having character ":" in an url that should be checked through 'isPath' method.

You should be able to check this by renaming your zope application path to add a ":" character in it.

I have joined a simple fix.

Revision history for this message
Valentin Lab (vaab) wrote :
Revision history for this message
Valentin Lab (vaab) wrote :

I had a little more time, so there's a new fix that could be ready for integration. I provided two patches, each one resolves the issue with a different solution. The second patch is in the following comment.

Review is needed. The issue is not simple :

    >>> assert_(isPath("/ab:c"))

triggered an exception. Which can be pretty annoying if you had a ":" anywhere in your path.

What's happening is that behind the curtains 'isPath' is using 'urllib.splittype()' which purpose
is to split the "scheme" part from the "resource" part.

It try to find any string at the beginning of the string that does not contain "/" nor ":" and which is separated
by a ":" from the remaining part of the string.

This is a pretty loose check, and raises some concerns (in isPath point of view) about what to return in some cases:

   >>> isPath("ab:c")

Should it return "True" it is a relative Path of a directory named "ab:c" OR
Should it return "False" it is NOT a Path as it has a "scheme" identifier which is "ab".

This could lead to the conclusion that isPath purpose and use is not fullfillable this way and a more general refactor could resolve this. Or 'urllib.splittype' might not be the solution and a simple regexp on "://" could be better.

I provided two patch, the first one tackles the problem for absolute paths containing ":" and chooses to return "False" on "ab:c". Next one is in the following comment.

Revision history for this message
Valentin Lab (vaab) wrote :

The second patch avoid using "urllib.splittype" but uses "^([^/:]+)://" regexp.

Both of these patch removes the nasty exception.

Revision history for this message
Fred Drake (fdrake) wrote :

Testing for "://" isn't correct, since not all URIs contain "//"; this is only needed for URI schemes that contain a hierarchical path portion.

So long as ":" is allowed in paths and relative paths may be mixed with full URIs, "ab:c" will be ambiguous if we don't want to impose restrictions on scheme identifiers beyond those specified by the relevant RFCs. This suggests that some separate bit is needed to identify absolute or relative path information unambiguously.

See details on URI syntax in RFC 3986:

    http://www.faqs.org/rfcs/rfc3986.html

Fix committed in revision 105068.

Changed in zconfig:
status: New → Fix Committed
Revision history for this message
Fred Drake (fdrake) wrote :

The changes have now been tested with Python 2.4.5 and 2.6.1 (both using setuptools 0.6c9) and the Python trunk (to become 2.7.0 someday; using the development version of the distribute bootstrap).

Revision history for this message
Tres Seaver (tseaver) wrote :

Fix released with ZConfig 2.8.0:

  http://pypi.python.org/pypi/ZConfig/2.8.0

Changed in zconfig:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.