Skip to content

Commit

Permalink
Fix session id collisions (#15170)
Browse files Browse the repository at this point in the history
  • Loading branch information
GrahamCampbell authored and taylorotwell committed Aug 31, 2016
1 parent ba7a37f commit 0135851
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion src/Illuminate/Session/Store.php
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ public function isValidId($id)
*/
protected function generateSessionId()
{
return sha1(uniqid('', true).Str::random(25).microtime(true));
return Str::random(40);
}

/**
Expand Down

10 comments on commit 0135851

@afraca
Copy link
Contributor

@afraca afraca commented on 0135851 Sep 1, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With this you broke the isValid method, making all session id's that are generated invalid, breaking all logins.

@GrahamCampbell
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What? How could this change do that. Please explain.

@GrahamCampbell
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I see.

@GrahamCampbell
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FFS.

@KennedyTedesco
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@GrahamCampbell What about this for the next version?

@GrahamCampbell
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeh, I've already PRed this to 5.3. Not to late to break stuff there tbh.

@GrahamCampbell
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is only a small change.

@KennedyTedesco
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed.

@dakshhmehta
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@GrahamCampbell - Dude you wasted our whole day in company at @Romin Interactive.

@GrahamCampbell
Copy link
Member Author

@GrahamCampbell GrahamCampbell commented on 0135851 Sep 1, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@GrahamCampbell - Dude you wasted our whole day in company at @Romin Interactive.

@dakshhmehta That's gratitude right there! Don't use code we haven't release yet!

Please sign in to comment.