Modify

Opened 5 years ago

Closed 4 years ago

Last modified 4 years ago

#10226 closed defect (fixed)

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

Reported by: Jun Omae Owned by: yosiyuki
Priority: normal Component: BookmarkPlugin
Severity: normal Keywords:
Cc: Jun Omae, Ryan J Ollos, Steffen Hoffmann 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 5 years ago by Ryan J Ollos

Yeah, that makes sense. +1 from me.

comment:2 Changed 4 years ago by Ryan J Ollos

Cc: Ryan J Ollos Steffen Hoffmann added

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

comment:3 Changed 4 years ago by Ryan J Ollos

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 Changed 4 years ago by Ryan J Ollos

On second look, we might want to use fnmatchcase.

comment:5 in reply to:  4 Changed 4 years ago by Jun Omae

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 4 years ago by Ryan J Ollos

Resolution: fixed
Status: newclosed

(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 4 years ago by Ryan J Ollos

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

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain yosiyuki.
The resolution will be deleted. Next status will be 'reopened'.

Add Comment


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

 
Note: See TracTickets for help on using tickets.