Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make SessionId#to_s be an alias of #public_id #1462

Merged
merged 1 commit into from
Jan 10, 2020

Conversation

jeremyevans
Copy link
Contributor

This restores backwards compatibility with code that uses the
session id.

No justification was given for making to_s raise in the commit
that changed the behavior (cc1d162).

If there is a reason to keep raising an exception, a specific
exception class and explicit error message should be used.

This restores backwards compatibility with code that uses the
session id.

No justification was given for making to_s raise in the commit
that changed the behavior (cc1d162).

If there is a reason to keep raising an exception, a specific
exception class and explicit error message should be used.
@ioquatix
Copy link
Member

@jeremyevans I assume you want to backport this to 2.0.x?

@jeremyevans
Copy link
Contributor Author

Yes, since 2.0.8 is affected.

@ioquatix
Copy link
Member

@tenderlove I'm in agreement with @jeremyevans, so can you merge this, backport it, release 2.0.9 and 2.1.0 tomorrow? Thanks.

Copy link
Collaborator

@rafaelfranca rafaelfranca left a comment

Choose a reason for hiding this comment

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

I'm ok with this change. We thought about doing it, but I think we decided to make it raise so third-party session adapters could upgrade their code.

@tenderlove tenderlove merged commit 7b860ce into rack:master Jan 10, 2020
tenderlove added a commit that referenced this pull request Jan 10, 2020
Make SessionId#to_s be an alias of #public_id
tenderlove added a commit that referenced this pull request Jan 10, 2020
Make SessionId#to_s be an alias of #public_id
tenderlove added a commit that referenced this pull request Jan 10, 2020
Make SessionId#to_s be an alias of #public_id
@tenderlove
Copy link
Member

We thought about doing it, but I think we decided to make it raise so third-party session adapters could upgrade their code.

Yes, this is the justification. As I said in another thread I should have raised a more specific exception and I really apologize.

@tanelsuurhans
Copy link

It looks like the 2.0.9 release was missed with this one.

@jeremyevans
Copy link
Contributor Author

2.0.9 hasn't been released yet. When it is released, it will include this fix. See https://github.com/rack/rack/commits/2-0-stable

@jbcrail
Copy link

jbcrail commented Feb 7, 2020

Can this be backported to 1.6.x? I upgraded recently to address a CVE, but was affected by this issue. Unfortunately, upgrading to 2.x isn't feasible in the short term.

@jeremyevans
Copy link
Contributor Author

@jbcrail This was already backported to 1.6 at 698a060, there just hasn't been another release of 1.6 yet.

This was referenced Mar 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants