Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Merge pull request #4079 from drogus/http_digest_issue
Fix http digest authentication when url ends with `/` or `?`
- Loading branch information
Showing
7 changed files
with
137 additions
and
8 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
require "abstract_unit" | ||
|
||
module ApplicationTests | ||
class BuildOriginalPathTest < Test::Unit::TestCase | ||
def test_include_original_PATH_info_in_ORIGINAL_FULLPATH | ||
env = { 'PATH_INFO' => '/foo/' } | ||
assert_equal "/foo/", Rails.application.send(:build_original_fullpath, env) | ||
end | ||
|
||
def test_include_SCRIPT_NAME | ||
env = { | ||
'SCRIPT_NAME' => '/foo', | ||
'PATH_INFO' => '/bar' | ||
} | ||
|
||
assert_equal "/foo/bar", Rails.application.send(:build_original_fullpath, env) | ||
end | ||
|
||
def test_include_QUERY_STRING | ||
env = { | ||
'PATH_INFO' => '/foo', | ||
'QUERY_STRING' => 'bar', | ||
} | ||
assert_equal "/foo?bar", Rails.application.send(:build_original_fullpath, env) | ||
end | ||
end | ||
end |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
618cb44
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will this be merged into 3.2.0?
not seeing it in 3.2.0rc2
618cb44
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@josevalim @tenderlove @spastorino can we still backport it to 3-2-stable or is it too late?
618cb44
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is a bugfix, so it seems fine to backport to 3-2-stable. Should we also backport to 3-1-stable and 3-0-stable?
618cb44
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tenderlove technically this bugfix also needs adding a feature (keeping original path for the request somewhere), if that's ok I can backport it to 3-2-stable. 3-1-stable and 3-0-stable also seems good
618cb44
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@drogus that's fine with me!
618cb44
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tenderlove cool, I will backport it today
618cb44
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I backported it to 3-1 and 3-2, if anyone needs it for 3-0-stable, please ping me, it will need a bit more changes.
618cb44
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@drogus thanks for working on this!
618cb44
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@trevor you're welcome :)