get_session fails for usernames with . (dot, period)

[hauke.muentinga@…]

In my Trac instance I use the naming scheme firstname.lastname. With the upgrade to 1.0.7, after login users were greeted with a Traceback:

2015-07-19 13:54:31,017 Trac[main] ERROR: Internal Server Error: 
Traceback (most recent call last):
  File "/usr/local/lib/python2.7/dist-packages/trac/web/main.py", line 550, in 
_dispatch_request
    dispatcher.dispatch(req)
  File "/usr/local/lib/python2.7/dist-packages/trac/web/main.py", line 208, in 
dispatch
    self._pre_process_request(req, chosen_handler)
  File "/usr/local/lib/python2.7/dist-packages/trac/web/main.py", line 371, in 
_pre_process_request
    chosen_handler = filter_.pre_process_request(req, chosen_handler)
  File "build/bdist.linux-x86_64/egg/acct_mgr/register.py", line 479, in pre_pr
ocess_request
    if not req.session.authenticated:
AttributeError: 'FakeSession' object has no attribute 'authenticated'

This is due to the validation of sid in trac.web.session.Session.get_session() with _valid_sid_re = re.compile(r'[_A-Za-z0-9\.]+\Z') introduced in [14120].

[hauke.muentinga@…]

Patch

[anonymous]

Replying to hauke.muentinga@…:

This is due to the validation of sid in trac.web.session.Session.get_session() with _valid_sid_re = re.compile(r'[_A-Za-z0-9\.]+\Z') […]

That is actually my fixed regex, the broken one is: _valid_sid_re = re.compile(r'[_A-Za-z0-9]+\Z').

[Jun Omae]

That's pretty bad. We shouldn't restrict sid for authenticated users because the user's name is used for sid. (e.g. ldap.user.name@REALM.EXAMPLE.COM, unicode username in #6318, …)

Log when trying to log in with jun.omae:

2015-07-21 15:30:39,523 Trac[util] DEBUG: args: ('3617a426ab2c1a6c0adc962ceec51dae', u'jun.omae', '192.168.11.46', 1437460239)
2015-07-21 15:30:39,523 Trac[util] DEBUG: prefetch: 0 rows
2015-07-21 15:30:39,524 Trac[util] DEBUG: SQL: SELECT authenticated FROM session WHERE sid=%s OR sid=%s
2015-07-21 15:30:39,524 Trac[util] DEBUG: args: ('0f944398718e7fe8ff58457f', u'jun.omae')
2015-07-21 15:30:39,524 Trac[util] DEBUG: prefetch: 0 rows
2015-07-21 15:30:39,524 Trac[util] DEBUG: SQL: INSERT INTO session (sid, last_visit, authenticated)
                          VALUES (%s, %s, 1)

2015-07-21 15:30:39,524 Trac[util] DEBUG: args: (u'jun.omae', 1437460239)
2015-07-21 15:30:39,529 Trac[util] DEBUG: prefetch: 0 rows
2015-07-21 15:30:39,529 Trac[util] DEBUG: SQL: SELECT authenticated FROM session WHERE sid=%s OR sid=%s
2015-07-21 15:30:39,529 Trac[util] DEBUG: args: ('0f944398718e7fe8ff58457f', u'jun.omae')
2015-07-21 15:30:39,529 Trac[util] DEBUG: prefetch: 1 rows
2015-07-21 15:30:39,529 Trac[main] ERROR: can't retrieve session: TracError: Session ID must be alphanumeric.
2015-07-21 15:30:39,531 Trac[main] DEBUG: Negotiated locale: None -> en_US
2015-07-21 15:30:39,536 Trac[main] ERROR: can't retrieve session: TracError: Session ID must be alphanumeric.
2015-07-21 15:30:39,757 Trac[main] DEBUG: Dispatching <RequestWithSession "GET '/'">

[Ryan J Ollos]

There were a few instances of this traceback in the t-h.o logs and issue reproduced with user rjollos-test. We should probably generate a milestone:1.0.8 release fairly soon to fix the issue.

[Jun Omae]

Proposed changes in jomae.git@t12129.

We should probably generate a milestone:1.0.8 release fairly soon to fix the issue.

I agree.

[Ryan J Ollos]

Changes work well for me. I can do the release on Thursday.

There are some test case failures though because multiple tests are run in the same case without reseting the database. Here is the log for one failure:

======================================================================
FAIL: test_new_session_promotion (trac.web.tests.session.SessionTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/user/Workspace/t11944/teo-rjollos.git/trac/web/tests/session.py", line 176, in test_new_session_promotion
    self._test_new_session_promotion('j.smith')
  File "/home/user/Workspace/t11944/teo-rjollos.git/trac/web/tests/session.py", line 172, in _test_new_session_promotion
    "SELECT sid, authenticated FROM session"))
AssertionError: Lists differ: [('j.smith', 1)] != [(u'j.smith', 1), (u'john', 1)...

Second list contains 1 additional elements.
First extra element 1:
(u'john', 1)

- [('j.smith', 1)]
+ [(u'j.smith', 1), (u'john', 1)]

[Jun Omae]

Thanks for the reviewing. I didn't run tests, stupidly. Updated jomae.git@t12129 branch.

[Ryan J Ollos]

Looks good and tests passing for me. I'll deploy the fix to trac-hacks.org soon after you commit the change.

[Jun Omae]

Committed in [14191] and merged to trunk in [14192].

[Ryan J Ollos]

I've found some errors in the logs days after having upgraded. Any ideas?

2015-07-27 04:09:41,414 Trac[main] ERROR: Exception caught while post-processing request:
Traceback (most recent call last):
  File "build/bdist.linux-i686/egg/trac/web/main.py", line 274, in dispatch
    self._post_process_request(req)
  File "build/bdist.linux-i686/egg/trac/web/main.py", line 387, in _post_process_request
    f.post_process_request(req, *(None,)*extra_arg_count)
  File "/srv/trac-hacks.org/pve/lib/python2.6/site-packages/TracAccountManager-0.4.3-py2.6.egg/acct_mgr/register.py", line 509, in post_process_request
    if not req.session.authenticated:
AttributeError: 'FakeSession' object has no attribute 'authenticated'
2015-07-27 04:09:41,415 Trac[main] ERROR: Internal Server Error:
Traceback (most recent call last):
  File "build/bdist.linux-i686/egg/trac/web/main.py", line 551, in _dispatch_request
    dispatcher.dispatch(req)
  File "build/bdist.linux-i686/egg/trac/web/main.py", line 209, in dispatch
    self._pre_process_request(req, chosen_handler)
  File "build/bdist.linux-i686/egg/trac/web/main.py", line 372, in _pre_process_request
    chosen_handler = filter_.pre_process_request(req, chosen_handler)
  File "/srv/trac-hacks.org/pve/lib/python2.6/site-packages/TracAccountManager-0.4.3-py2.6.egg/acct_mgr/register.py", line 479, in pre_process_request
    if not req.session.authenticated:
AttributeError: 'FakeSession' object has no attribute 'authenticated'

[Jun Omae]

Similar to #12062. How about adding same attributes of DetachedSession to the FakeSession?

trac/web/main.py

@@ -62 +62 @@
 
 class FakeSession(dict):
     sid = None
+    authenticated = False
+    last_visit = 0
     def save(self):
         pass
 

[Ryan J Ollos]

Seems like a good idea. If committed soon we can test on trac-hacks.org leading up to the 1.0.9 release to be sure it resolves all issues.

[Ryan J Ollos]

#12172 created to deal with issue in comment:11.