Edgewall Software
Modify

Opened 7 years ago

Closed 6 years ago

#11528 closed defect (fixed)

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

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

Attachments (0)

Change History (6)

comment:1 by Peter Suter, 7 years ago

fnmatch.fnmatchcase should be ok, right?

comment:2 by Jun Omae, 7 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, 7 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, 7 years ago

Cc: Ryan J Ollos added

comment:5 by Jun Omae, 6 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, 6 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].

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Jun Omae.
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from Jun Omae to the specified user.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.