I've having a problem with http://drupal.org/project/persona

If someone visits the site for the first time they cannot sign in. They need to view a second page before it will work. This is because the first time a browser visits the site session_api_available() returns FALSE.

I am not clear on why it needs to be checked in session_api_get_sid(), but commenting it out solves the problem.

CommentFileSizeAuthor
#1 1925962.patch698 bytesjbrown

Comments

jbrown’s picture

jbrown commented
Status: Active » Needs review
StatusFileSize
new 1925962.patch698 bytes

Here's the patch.

jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
commented

Patch looks reasonable, but I'd like some other folks to test this out prior to committing.

jbrown’s picture

jbrown commented

The Persona project is no longer using Session API, but I think the patch is important.

It isn't possible to determine if cookies are enabled on the first request and I'm not clear on why it matters that it is being checked for.

Maybe this patch could be commited and then left in the dev release to see if it causes any problems?

a.ross’s picture

a.ross commented
Status: Needs review » Needs work

This patch will cause test breakage, as session_api_get_sid() must return FALSE when session_api_available() returns FALSE.

a.ross’s picture

a.ross commented
Status: Needs work » Closed (won't fix)

I'm not opposed to the documentation line you added, as developers should be informed about this issue. But the code change cannot be right, as that means the module will start returning Session IDs even when it cannot be guaranteed that clients will be identified by them (IE cookies are disabled).

If it turns out that cookies are disabled, you will want to drop the bogus Session ID and let modules using Session API know it's invalidated. But how will you figure out a subsequent request belongs to the person you just tried and failed to create a Session ID for?

a.ross’s picture

a.ross commented
Issue summary: View changes

Removed trailing semi-colon from URL which would make it not link properly.