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

Fix raw params method to not raise an exception #41280

Merged

Conversation

kentakag
Copy link
Contributor

@kentakag kentakag commented Jan 30, 2021

Summary

#41279

Starting from 6.1.1, ActionController::HttpAuthentication::Token::ControllerMethods#authenticate_with_http_token can raise an exception.
It happens when the http authorization request header has only a type and no credential.

token = nil # or token = ''
request.headers['Authorization'] = "Bearer #{token}"
request.get
authenticate_with_http_token do |token, _options|
  # 
end
NoMethodError:
       undefined method `start_with?' for nil:NilClass

Under 6.1.1, nil is returned in such a case.
As a specification, I think it is correct to return nil instead of an exception when the token is invalid.
Therefore, I fixed ActionController::HttpAuthentication::Token#raw_params which raises an exception.

Other Information

In addition, I fixed the typo.
61a9c13

@kamipo
Copy link
Member

kamipo commented Feb 1, 2021

Can you squash commits into one?

https://guides.rubyonrails.org/contributing_to_ruby_on_rails.html#squashing-commits

Prevent the raw_params method from throwing an exception
if the argument auth is blank.

Add tests for the raw_params method

Fix typo

Fix rubocop offenses
@kentakag kentakag force-pushed the fix-raw-params-method-to-not-raise-an-exception branch from eeddee2 to d7516f4 Compare February 1, 2021 13:15
@kamipo kamipo merged commit e8d7181 into rails:main Feb 1, 2021
kamipo added a commit that referenced this pull request Feb 1, 2021
…raise-an-exception

Fix raw params method to not raise an exception

Fixes #41279.
@kentakag kentakag deleted the fix-raw-params-method-to-not-raise-an-exception branch February 1, 2021 14:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

#authenticate_with_http_token raises an exception from 6.1.1 If token.blank?
2 participants