Edgewall Software

Opened 10 years ago

Closed 10 years ago

#11528 closed defect (fixed)

Should not use fnmatch which is case-insensitive and platform dependent — at Version 6

Reported by: Jun Omae Owned by: Jun Omae
Priority: normal Milestone: 1.0.2
Component: general Version:
Severity: normal Keywords: authzpolicy windows
Cc: Ryan J Ollos Branch:
Release Notes:

Case-sensitive matching resource names in authz policy on Windows.

API Changes:
Internal Changes:

Description

Trac core uses fnmatch.fnmatch method. The method internally use os.path.normcase and is depended on platform. I don't think we should use the methods for wildcard-globbing.

We should use fnmatch.translate instead.

Windows:

Python 2.4.4 (#71, Oct 18 2006, 08:34:43) [MSC v.1310 32 bit (Intel)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> from fnmatch import fnmatch
>>> fnmatch('abc/def', r'a*/def')
True
>>> fnmatch('abc/def', r'a*\def')
True
>>> fnmatch('abc/def', r'a*\DEF')
True

Linux:

Python 2.4.3 (#1, Jan  9 2013, 06:49:54)
[GCC 4.1.2 20080704 (Red Hat 4.1.2-54)] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> from fnmatch import fnmatch
>>> fnmatch('abc/def', 'a*/def')
True
>>> fnmatch('abc/def', r'a*\def')
False
$ grep -wr --include='*.py' fnmatch trac tracopt
trac/versioncontrol/web_ui/browser.py:from fnmatch import fnmatchcase
trac/wiki/macros.py:from fnmatch import fnmatchcase
trac/web/main.py:import fnmatch
trac/web/main.py:                 and not any(fnmatch.fnmatch(path[:-1], pattern)
tracopt/perm/authz_policy.py:from fnmatch import fnmatch
tracopt/perm/authz_policy.py:            if fnmatch(resource_key, resource_glob):

Change History (6)

comment:1 by Peter Suter, 10 years ago

fnmatch.fnmatchcase should be ok, right?

comment:2 by Jun Omae, 10 years ago

Yes. The fnmatchcase method doesn't use os.path.normcase. I've confirmed fnmatch.py source and actually verified.

>>> import os
>>> os.name
'nt'
>>> from fnmatch import fnmatchcase
>>> fnmatchcase('abc/def', r'a*/def')
True
>>> fnmatchcase('abc/def', r'a*\def')
False
>>> fnmatchcase('abc/def', r'a*/DEF')
False

comment:3 by Jun Omae, 10 years ago

Summary: Should not use fnmatch which is platform dependentShould not use fnmatch which is case-insensitive and platform dependent

Especially, in the case of AuthzPolicy, name of resource is defined using wildcards in section of its configuration file.

[wiki:WikiStart*]
...

I don't think that a user expects the matching is case-insensitive.

comment:4 by Ryan J Ollos, 10 years ago

Cc: Ryan J Ollos added

comment:5 by Jun Omae, 10 years ago

Milestone: next-stable-1.0.x1.0.2
Owner: set to Jun Omae
Status: newassigned

Proposed changes in jomae.git@t11528.

comment:6 by Jun Omae, 10 years ago

Keywords: windows added
Release Notes: modified (diff)
Resolution: fixed
Status: assignedclosed

I confirmed on Windows. Committed in [12845] and merged to trunk in [12846].

Note: See TracTickets for help on using tickets.