Merge pull request #7312 from krainboltgreene/http-token-parser-bug

Http token parser bug

2 files changed, 56 insertions, 17 deletions

diff --git a/actionpack/lib/action_controller/metal/http_authentication.rb b/actionpack/lib/action_controller/metal/http_authentication.rb
index 283f6413ec..f50394837b 100644
--- a/actionpack/lib/action_controller/metal/http_authentication.rb
+++ b/actionpack/lib/action_controller/metal/http_authentication.rb
@@ -384,6 +384,8 @@ module ActionController
     #
     #   RewriteRule ^(.*)$ dispatch.fcgi [E=X-HTTP_AUTHORIZATION:%{HTTP:Authorization},QSA,L]
     module Token
+      TOKEN_REGEX = /^Token /
+      AUTHN_PAIR_DELIMITERS = /(?:,|;|\t+)/
       extend self
 
       module ControllerMethods
@@ -431,20 +433,34 @@ module ActionController
       # Returns an Array of [String, Hash] if a token is present.
       # Returns nil if no token is found.
       def token_and_options(request)
-        if request.authorization.to_s[/^Token (.*)/]
-          values = Hash[$1.split(',').map do |value|
-            value.strip!                      # remove any spaces between commas and values
-            key, value = value.split(/\=\"?/) # split key=value pairs
-            if value
-              value.chomp!('"')                 # chomp trailing " in value
-              value.gsub!(/\\\"/, '"')          # unescape remaining quotes
-              [key, value]
-            end
-          end.compact]
-          [values.delete("token"), values.with_indifferent_access]
+        authorization_request = request.authorization.to_s
+        if authorization_request[TOKEN_REGEX]
+          params = token_params_from authorization_request
+          [params.shift.last, Hash[params].with_indifferent_access]
         end
       end
 
+      def token_params_from(auth)
+        rewrite_param_values params_array_from raw_params auth
+      end
+
+      # Takes raw_params and turns it into an array of parameters
+      def params_array_from(raw_params)
+        raw_params.map { |param| param.split %r/=(.+)?/ }
+      end
+
+      # This removes the `"` characters wrapping the value.
+      def rewrite_param_values(array_params)
+        array_params.each { |param| param.last.gsub! %r/^"|"$/, '' }
+      end
+
+      # This method takes an authorization body and splits up the key-value
+      # pairs by the standardized `:`, `;`, or `\t` delimiters defined in
+      # `AUTHN_PAIR_DELIMITERS`.
+      def raw_params(auth)
+        auth.sub(TOKEN_REGEX, '').split /"\s*#{AUTHN_PAIR_DELIMITERS}\s*/
+      end
+
       # Encodes the given token and options into an Authorization header value.
       #
       # token   - String token.
diff --git a/actionpack/test/controller/http_token_authentication_test.rb b/actionpack/test/controller/http_token_authentication_test.rb
index 8a409d6ed2..faf923e929 100644
--- a/actionpack/test/controller/http_token_authentication_test.rb
+++ b/actionpack/test/controller/http_token_authentication_test.rb
@@ -104,17 +104,40 @@ class HttpTokenAuthenticationTest < ActionController::TestCase
     assert_equal 'Token realm="SuperSecret"', @response.headers['WWW-Authenticate']
   end
 
-  test "authentication request with valid credential" do
-    @request.env['HTTP_AUTHORIZATION'] = encode_credentials('"quote" pretty', :algorithm => 'test')
-    get :display
+  test "token_and_options returns correct token" do
+    token = "rcHu+HzSFw89Ypyhn/896A=="
+    actual = ActionController::HttpAuthentication::Token.token_and_options(sample_request(token)).first
+    expected = token
+    assert_equal(expected, actual)
+  end
+
+  test "token_and_options returns correct token" do
+    token = 'rcHu+=HzSFw89Ypyhn/896A==f34'
+    actual = ActionController::HttpAuthentication::Token.token_and_options(sample_request(token)).first
+    expected = token
+    assert_equal(expected, actual)
+  end
 
-    assert_response :success
-    assert assigns(:logged_in)
-    assert_equal 'Definitely Maybe', @response.body
+  test "token_and_options returns correct token" do
+    token = 'rcHu+\\\\"/896A'
+    actual = ActionController::HttpAuthentication::Token.token_and_options(sample_request(token)).first
+    expected = token
+    assert_equal(expected, actual)
+  end
+
+  test "token_and_options returns correct token" do
+    token = '\"quote\" pretty'
+    actual = ActionController::HttpAuthentication::Token.token_and_options(sample_request(token)).first
+    expected = token
+    assert_equal(expected, actual)
   end
 
   private
 
+  def sample_request(token)
+    @sample_request ||= OpenStruct.new authorization: %{Token token="#{token}"}
+  end
+
   def encode_credentials(token, options = {})
     ActionController::HttpAuthentication::Token.encode_credentials(token, options)
   end