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.

Sign up for GitHub

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

Jump to bottom

[rack-protection] Consider renaming origin_whitelist to something else #1620

Closed
rhymes opened this issue Jun 8, 2020 · 2 comments · Fixed by #1625
Closed

[rack-protection] Consider renaming origin_whitelist to something else #1620

rhymes opened this issue Jun 8, 2020 · 2 comments · Fixed by #1625

Comments

@rhymes
Copy link
Contributor

rhymes commented Jun 8, 2020

Following the example of rails/rails#33677 and forem/forem#7147 I'm proposing to take into consideration the replacement of the parameter origin_whitelist in Rack::Protection::HttpOrigin with something else.

Possible alternatives: allow, valid, permitted.

According to a quick search - https://github.com/sinatra/sinatra/search?q=whitelist&unscoped_q=whitelist - in the entire Sinatra code base, that's the only instance.

A possible solution would be to add an alternative parameter and a deprecation notice for the existing one and then remove the latter a couple of releases down the line or something like that.

I can work on the initial PR if agreed.

Thank you!
@jkowens
Copy link
Member

jkowens commented Jul 11, 2020

@rhymes sounds like a good plan. My initial thought is that permitted is a good replacement. Your help would be greatly appreciated!

@rhymes rhymes mentioned this issue Jul 11, 2020
Merged
@rhymes
Copy link
Contributor Author

rhymes commented Jul 11, 2020

@jkowens thanks for the availability. I sent a PR in #1625 but it failed in most of the builds and I think it's unrelated to the code change but as I'm unfamiliar with sinatra and rack-protection it might be entirely my fault. I tried reading the contributing guide but found nothing that could related.

Could you take a look? Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Replace origin_whitelist with permitted_origins
2 participants
@rhymes @jkowens