Ticket #10226 (closed defect: fixed)

Opened 10 months ago

Last modified 3 weeks ago

Uses wrongly `re.match` for `[bookmark] paths` settings

Reported by: jun66j5 Assigned to: saigon
Priority: normal Component: BookmarkPlugin
Severity: normal Keywords:
Cc: jun66j5, rjollos, hasienda Trac Release: 0.12

Description

It seems [bookmark] paths supports shell wildcards, not regular expression. 

    bookmarkable_paths = ListOption('bookmark', 'paths', '/*',
        doc='List of URL paths to allow bookmarking on. Globs are supported.')

But the plugin uses re.match for each value with no changes. 

    def post_process_request(self, req, template, data, content_type):
        # Show bookmarks context menu except when on the bookmark page
        if 'BOOKMARK_VIEW' in req.perm and not self.match_request(req):
            for path in self.bookmarkable_paths:
                if re.match(path, req.path_info):
                    self.render_bookmarker(req)
                    break
        return template, data, content_type

I think it should fix to be able to use shell wildcards.

Attachments

Change History

10/19/12 10:24:36 changed by rjollos

Yeah, that makes sense. +1 from me.

03/27/13 01:49:41 changed by rjollos

  • cc changed from jun66j5,rjollos to jun66j5, rjollos, hasienda.

Same issue was noted with the VotePlugin in comment:5:ticket:10942.

04/25/13 23:31:24 changed by rjollos

It looks like fnmatch will accomplish what we want. How does the following patch look? 

Index: tracbookmark/__init__.py
===================================================================
--- tracbookmark/__init__.py	(revision 13007)
+++ tracbookmark/__init__.py	(working copy)
@@ -9,6 +9,7 @@
 # you should have received as part of this distribution.
 
 import re
+from fnmatch import fnmatch
 
 from genshi.builder import tag
 from trac.config import ListOption
@@ -168,7 +169,7 @@
         # Show bookmarks context menu except when on the bookmark page
         if self._authorize(req) and not self.match_request(req):
             for path in self.bookmarkable_paths:
-                if re.match(path, req.path_info):
+                if fnmatch(req.path_info, path):
                     self.render_bookmarker(req)
                     break
         return template, data, content_type

(follow-up: ↓ 5 ) 04/26/13 06:57:24 changed by rjollos

On second look, we might want to use fnmatchcase.

(in reply to: ↑ 4 ) 04/26/13 17:11:22 changed by jun66j5

Replying to rjollos:

On second look, we might want to use fnmatchcase.

Looks good! Yes, we should use fnmatchcase because the URLs in Trac is case-sensitive. Please commit it!

04/26/13 21:59:47 changed by rjollos

  • status changed from new to closed.
  • resolution set to fixed.

(In [13011]) Fixes #10226:

BookmarkPlugin: Perform Unix shell-style wildcard matching (globs), as is documented, for the paths option. Previously, a regular expression was used to match paths.

04/26/13 22:18:09 changed by rjollos

(In [13012]) Fixes #11037, Refs #10226:

VotePlugin: Perform Unix shell-style wildcard matching (globs), as is documented, for the paths option. Previously, a regular expression was used to match paths.

Add/Change #10226 (Uses wrongly `re.match` for `[bookmark] paths` settings)




Change Properties
Action