Modify

Opened 2 years ago

Closed 17 months ago

Last modified 17 months ago

#10226 closed defect (fixed)

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

Reported by: jun66j5 Owned by: 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 (0)

Change History (7)

comment:1 Changed 2 years ago by rjollos

Yeah, that makes sense. +1 from me.

comment:2 Changed 18 months ago by rjollos

  • Cc hasienda added

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

comment:3 Changed 17 months ago by rjollos

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

  • tracbookmark/__init__.py

     
    99# you should have received as part of this distribution. 
    1010 
    1111import re 
     12from fnmatch import fnmatch 
    1213 
    1314from genshi.builder import tag 
    1415from trac.config import ListOption 
     
    168169        # Show bookmarks context menu except when on the bookmark page 
    169170        if self._authorize(req) and not self.match_request(req): 
    170171            for path in self.bookmarkable_paths: 
    171                 if re.match(path, req.path_info): 
     172                if fnmatch(req.path_info, path): 
    172173                    self.render_bookmarker(req) 
    173174                    break 
    174175        return template, data, content_type 

comment:4 follow-up: Changed 17 months ago by rjollos

On second look, we might want to use fnmatchcase.

comment:5 in reply to: ↑ 4 Changed 17 months ago 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!

comment:6 Changed 17 months ago by rjollos

  • Resolution set to fixed
  • Status changed from new to closed

(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.

comment:7 Changed 17 months ago 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 Comment

Modify Ticket

Action
as closed .
The resolution will be deleted. Next status will be 'reopened'.
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.