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

Add base64 dependency #1937

Closed
dentarg opened this issue Aug 15, 2023 · 3 comments · Fixed by #1946
Closed

Add base64 dependency #1937

dentarg opened this issue Aug 15, 2023 · 3 comments · Fixed by #1946
Labels
dependencies help wanted

Comments

@dentarg
Copy link
Member

dentarg commented Aug 15, 2023 •
edited
Loading

base64 needs to be a explicit dependency in Ruby 3.4.0

Similar PRs are rails/rails#48907 and rack/rack#2109

It is possible to get away with pack/unpack methods: rack/rack#2110 but Sinatra (authenticity_token middleware in rack-protection) do use the two urlsafe methods:

sinatra/rack-protection/lib/rack/protection/authenticity_token.rb

Lines 227 to 233 in 35bc85b

def encode_token(token)
Base64.urlsafe_encode64(token)
end
def decode_token(token)
Base64.urlsafe_decode64(token)
end

@dentarg dentarg changed the title Add base64 dependency Add (or remove) base64 dependency Aug 15, 2023
@dentarg dentarg changed the title Add (or remove) base64 dependency Add base64 dependency Aug 15, 2023
@zzak
Copy link
Member

zzak commented Aug 17, 2023

Why not just add it as a dependency like any other gem we need?

@dentarg
Copy link
Member Author

dentarg commented Aug 17, 2023

I didn't say we shouldn't, just pointed out it was a possibility – in the case of Rack (rack/rack#2110) it was very simple.

I do not advocate for Sinatra to maintain this code: https://github.com/ruby/ruby/blob/efd611ceb661a230f121797079376cdb4e831951/lib/base64.rb#L80-L113

@jkowens
Copy link
Member

jkowens commented Aug 22, 2023

I suspect some may be using rack-protection in applications that try to be interoperable with Rails csrf protection (mounted apps). Staying in-line with Rails may not be something we want to aim for, but just wanted to throw that out there. That's the only reason I can think of for sticking with Base64.

@mzishtiaq mzishtiaq mentioned this issue Sep 11, 2023
Merged
dentarg pushed a commit that referenced this issue Sep 16, 2023
@mzishtiaq
Add base64 to rack-protection runtime dependencies (#1946)
e130771 
This PR adds `base64` to rack-protection.gemspec because `base64` will
not be part of the default gems in Ruby 3.4.

Resolves #1937
@andreynering andreynering mentioned this issue Jan 15, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
dependencies help wanted
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add base64 to rack-protection runtime dependencies mzishtiaq/sinatra
3 participants
@dentarg @zzak @jkowens