Permalink
Browse files

Merge pull request #4731 from ndbroadbent/revert_build_original_fullpath

Reverted build_original_fullpath method refactor
  • Loading branch information...
2 parents c0dafb6 + 2d62334 commit fd3211ef677efe9531f38db58919a8c90d65892a @spastorino spastorino committed Jan 28, 2012
Showing with 9 additions and 1 deletion.
  1. +9 −1 railties/lib/rails/application.rb
@@ -290,7 +290,15 @@ def initialize_console #:nodoc:
end
def build_original_fullpath(env)
- ["#{env["SCRIPT_NAME"]}#{env["PATH_INFO"]}", env["QUERY_STRING"]].reject(&:blank?).join("?")
+ path_info = env["PATH_INFO"]
+ query_string = env["QUERY_STRING"]
pacoguzman
pacoguzman Mar 2, 2012 Contributor

I'm getting to this after some research while upgrading a rails application that use http digest. And my question is why use env["QUERY_STRING"]? This env variable is not the original version send from the client is a escaped version of that.

drogus
drogus Mar 2, 2012 Member

This was added to fix the error on http digest - it tried to reauthenticate when questionmark was added to the query and also it was trying to reauthenticate on params change. Can you provide an example of failing request?

pacoguzman
pacoguzman Mar 3, 2012 Contributor

I'm trying to setup a test case in the action_pack suite, but I think the following request could not be properly authenticated

GET '/http_digest_authentication_test/dummy_digest?expando=dummy,dumb'

The tricky part is the parameter with commas

drogus
drogus Mar 3, 2012 Member

I couldn't find any misbehaviors by passing such query strings, so please just open a new issue if you find any.

pacoguzman
pacoguzman Mar 4, 2012 Contributor

Ok, I'll do, the misbehaviors are when you use a different credential[:uri] on the server side, due to Rack manipulations

+ script_name = env["SCRIPT_NAME"]
+
+ if query_string.present?
+ "#{script_name}#{path_info}?#{query_string}"
+ else
+ "#{script_name}#{path_info}"
+ end
end
end
end

0 comments on commit fd3211e

Please sign in to comment.