r8496 Breaks "Environment needs to be upgraded" Message

[John Hampton]

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

@@ -431 +431 @@
         env_error = e
 
     req = Request(environ, start_response)
-    translation.make_activable(lambda: req.locale, env.path)
+    translation.make_activable(lambda: req.locale, env and env.path or None)
     try:
         return _dispatch_request(req, env, env_error)
     finally:
@@ -489 +489 @@
             message = e.detail
         else:
             message = to_unicode(e.detail)
-        data = {'title': title, 'type': 'TracError', 'message': message,
+        data = {'title': title, 'type': 'TracError', 'message': str(message),
                 'frames': [], 'traceback': None}
         if e.code == 403 and req.authname == 'anonymous':
             # 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?

[Remy Blank]

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

@@ -400 +400 @@
         except: # failed to render
             data = get_last_traceback()
             content_type = 'text/plain'
+
+        if isinstance(data, unicode):
+            data = data.encode('utf-8')
 
         self.send_response(status)
         self._outheaders = []

[Christian Boos]

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 ;-) )

[John Hampton]

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 ;)