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 ​http://trac.edgewall.org/changeset/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].