Edgewall Software
Modify

Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#11283 closed defect (fixed)

InterTrac link to non-existent changeset results in inaccurate error message

Reported by: Ryan J Ollos Owned by: Ryan J Ollos
Priority: normal Milestone: 1.0.2
Component: wiki system Version: 1.0-stable
Severity: normal Keywords: intertrac, changeset, traclinks
Cc:
Release Notes:

Improved error message for an intertrac link to a non-existent or forbidden changeset.

API Changes:

Description

For example, changeset/1000000 leads to a page displaying:

Error: Invalid Changeset Number

No changeset 1000000 in the repository

TracGuide — The Trac User and Administration Guide

However, [trac 1000000] leads to a page displaying:

Error: Forbidden

Can't view changeset:"1000000": privileges are required to perform this operation. You don't have the required permissions.

TracGuide — The Trac User and Administration Guide

Attachments (0)

Change History (8)

comment:1 Changed 5 years ago by Ryan J Ollos

Keywords: intertrac traclink changeset added
Milestone: next-stable-1.0.x
Owner: set to Ryan J Ollos
Status: newassigned

comment:2 Changed 5 years ago by Ryan J Ollos

The error message looks strange because the "message" is being passed as the first argument and is being assigned to the parameter action. We should at least be using a keyword argument for the msg parameter.

We could try to check if the resource exists, and raise different exception depending on whether the issue is resource existence or authorization. That seems like a bit of work though, and for now I'd just propose the following:

  • trac/wiki/intertrac.py

    diff --git a/trac/wiki/intertrac.py b/trac/wiki/intertrac.py
    index 58e00c2..03c23d6 100644
    a b class InterTracDispatcher(Component):  
    9191        if isinstance(link_frag, (Element, Fragment)):
    9292            elt = find_element(link_frag, 'href')
    9393            if elt is None: # most probably no permissions to view
    94                 raise PermissionError(_("Can't view %(link)s:", link=link))
     94                raise PermissionError(
     95                    msg=_("Can't view %(link)s. Resource doesn't exist or "
     96                          "you don't have the required permission.",
     97                          link=link))
    9598            href = elt.attrib.get('href')
    9699        else:
    97100            href = req.href(link.rstrip(':'))

This change results in:

Error: Forbidden

Can't view changeset:"1000000". Resource doesn't exist or you don't have the required permission.

TracGuide — The Trac User and Administration Guide

Maybe it is more appropriate to raise a TracError rather than a PermissionError.

  • trac/wiki/intertrac.py

    diff --git a/trac/wiki/intertrac.py b/trac/wiki/intertrac.py
    index 58e00c2..16ca381 100644
    a b from genshi.builder import Element, Fragment, tag  
    2020
    2121from trac.config import ConfigSection
    2222from trac.core import *
    23 from trac.perm import PermissionError
    2423from trac.util.html import find_element
    2524from trac.util.translation import _, N_
    2625from trac.web.api import IRequestHandler
    class InterTracDispatcher(Component):  
    9089        link_frag = extract_link(self.env, web_context(req), link)
    9190        if isinstance(link_frag, (Element, Fragment)):
    9291            elt = find_element(link_frag, 'href')
    93             if elt is None: # most probably no permissions to view
    94                 raise PermissionError(_("Can't view %(link)s:", link=link))
     92            if elt is None:
     93                raise TracError(
     94                    _("Can't view %(link)s. Resource doesn't exist or "
     95                      "you don't have the required permission.", link=link))
    9596            href = elt.attrib.get('href')
    9697        else:
    9798            href = req.href(link.rstrip(':'))

Trac Error

Can't view changeset:"1000000". Resource doesn't exist or you don't have the required permission.

TracGuide — The Trac User and Administration Guide

comment:3 Changed 5 years ago by Ryan J Ollos

Milestone: next-stable-1.0.x1.0.2

comment:4 Changed 5 years ago by Ryan J Ollos

Should I commit this to trunk rather than 1.0-stable since it changes a translatable string and it's not a fix for a major functional issue?

comment:5 Changed 5 years ago by Jun Omae

I think that pushing to 1.0-stable is okay because the changesets after 1.0.1, e.g. r11777, have been changed the translation messages.

comment:6 Changed 5 years ago by Ryan J Ollos

Release Notes: modified (diff)
Resolution: fixed
Status: assignedclosed

Committed to 1.0-stable in [12048]. Merged to trunk in [12049].

comment:7 Changed 5 years ago by Ryan J Ollos

Keywords:

comment:8 Changed 5 years ago by Ryan J Ollos

Keywords: traclinks added; traclink removed

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Ryan J Ollos.
The resolution will be deleted.
to The owner will be changed from Ryan J Ollos 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.