Edgewall Software
Modify

Opened 13 years ago

Closed 13 years ago

#8730 closed defect (fixed)

r8496 Breaks "Environment needs to be upgraded" Message

Reported by: John Hampton Owned by: John Hampton
Priority: high Milestone: 0.12
Component: general Version: devel
Severity: major Keywords:
Cc: Branch:
Release Notes:
API Changes:
Internal Changes:

Description

r8496 breaks Trac when accessing a site whose environment needs to be upgraded.

The problem is due to line 434 in source:trunk/trac/web/main.py.

If an environment needs an upgrade, then env will be None and thus the access to env.path is invalid.

A simple solution would be:

  • trac/web/main.py

     
    431431        env_error = e
    432432
    433433    req = Request(environ, start_response)
    434     translation.make_activable(lambda: req.locale, env.path)
     434    translation.make_activable(lambda: req.locale, env and env.path or None)
    435435    try:
    436436        return _dispatch_request(req, env, env_error)
    437437    finally:
     
    489489            message = e.detail
    490490        else:
    491491            message = to_unicode(e.detail)
    492         data = {'title': title, 'type': 'TracError', 'message': message,
     492        data = {'title': title, 'type': 'TracError', 'message': str(message),
    493493                'frames': [], 'traceback': None}
    494494        if e.code == 403 and req.authname == 'anonymous':
    495495            # TRANSLATOR: ... not logged in, you may want to 'do so' now (link)

Note, the second line changed where wrapping the message in sstr() was done because otherwise the exception in Request.write() is triggered. I'm not sure that this is actually correct, but don't understand where better to put it.

Suggestions, anyone?

Attachments (0)

Change History (3)

comment:1 by Remy Blank, 13 years ago

For the second part of the patch, rather than converting to string at that location, we should fix Request.send_error() and ensure data is always a UTF-8 string before calling write(). Something like (untested):

  • trac/web/api.py

    diff --git a/trac/web/api.py b/trac/web/api.py
    a b  
    400400        except: # failed to render
    401401            data = get_last_traceback()
    402402            content_type = 'text/plain'
     403
     404        if isinstance(data, unicode):
     405            data = data.encode('utf-8')
    403406
    404407        self.send_response(status)
    405408        self._outheaders = []

comment:2 by Christian Boos, 13 years ago

Severity: criticalmajor

Right, I already had the first chunk in a patch, but was unsure about the exact circumstance triggering the problem.

Instead of the second chunk in your patch, we should do what Remy proposed.

John, can you commit the two changes or should one of us take care of it?

(and the problem is not that critical ;-) )

in reply to:  2 comment:3 by John Hampton, 13 years ago

Resolution: fixed
Status: newclosed

Replying to cboos:

Instead of the second chunk in your patch, we should do what Remy proposed.

Yeah, I definitely like Remy's patch better. Mine was just a quick hack to prove the first line.

John, can you commit the two changes or should one of us take care of it?

Fixed in [8658]

(and the problem is not that critical ;-) )

Perhaps, though breaking every upgrade from 0.11 and any installation of any plugin that requires an env upgrade seems pretty critical to me ;)

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain John Hampton.
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from John Hampton 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.