search

jeremyevans / rodauth

Ruby's Most Advanced Authentication Framework
http://rodauth.jeremyevans.net
MIT License
1.65k stars 95 forks source link

Replaced Array#pack with Base64.urlsafe_encode64 in specs for generating authorization header #330

Closed igor-alexandrov closed 1 year ago

igor-alexandrov commented 1 year ago

Current implementation of Authorization header in specs may produce not URL-safe. I found this in rodauth-oauth, but this also applies to Rodauth.

jeremyevans commented 1 year ago

Thanks for submitting a pull request. Can you explain why we should make this change? This is in the specs and not in the library itself, so it should have no effect on Rodauth usage.

Even if this was in the library, the Authorization header is not designed to be used in URLs. Is there a reason it matters that the header values are not URL safe? If you want to use header values in URLs, you should URL encode them.

HoneyryderChuck commented 1 year ago

@jeremyevans while I agree that this is test code and correctness enforcement is not a must, the current implementation is wrong, as it allows newlines, which break the regex used in the production code to parse the token. This may confuse contributors who may accidentally see their tests fail). I agree that making is urlsafe is probably too much (encode64 would have sufficed), it should at least be changed to"pack('m0')" in order to be correct.

jeremyevans commented 1 year ago

@HoneyryderChuck Thanks for explaining that. I'll switch to m0.