Skip to content

Commit

Permalink
Fix empty SCRIPT_NAME with partial match route bug
Browse files Browse the repository at this point in the history
Why you made the change:

I made this change so that SCRIPT_NAME and PATH_INFO would be set
appropriately in the scenario where a partial match route is used.

How the change addresses the need:

There is an issue with the current version of rewrite_partial_path_info
in HttpRouter where it does not set SCRIPT_NAME appropriately. When the
request environments PATH_INFO is set, it causes the
request.rack_request.path_info helper return the updated version, not
the original. This is requst.rack_request.path_info eventually just
reads the PATH_INFO value out of the request environment. As a side
effect of this, the original code would always set the SCRIPT_NAME to ""
because it would be slicing from request.rack_request.path_info[0, 0] in
actuality.

This change resolves this issue by first temporarily storing a dup of
the original path_info and using that to determine the end index for the
SCRIPT_NAME. This change also handles setting SCRIPT_NAME correctly for
the somewhat unique but common case of matching the partial match
exactly.

This change also includes a fix to handle maintaining URI encoded values
in PATH_INFO and SCRIPT_INFO. Currently, there is a problem where
because the method uses the request.path helper HttpRouter provides,
which URI.decodes the string, it screws up both the PATH_INFO and
SCRIPT_NAME. Therefore, I URI.encode it back appropriately before any
offsetting, or setting is done.

I have also added tests for each of these cases as I couldn't find any
tests covering this before.

This fixes issue #87.
  • Loading branch information
drewdeponte committed Feb 9, 2016
1 parent add3706 commit 17a2d57
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 0 deletions.
12 changes: 12 additions & 0 deletions lib/hanami/routing/http_router.rb
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,18 @@ def rewrite_path_info(env, request)
env[SCRIPT_NAME] = @prefix.join(env[SCRIPT_NAME])
end

# @api private
def rewrite_partial_path_info(env, request)
path_info_before = request.rack_request.path_info.dup
if request.path.empty?
env['PATH_INFO'] = "/"
env['SCRIPT_NAME'] += path_info_before
else
env['PATH_INFO'] = "/#{URI.encode(request.path.join('/'))}"
env['SCRIPT_NAME'] += path_info_before[0, path_info_before.size - env['PATH_INFO'].size]
end
end

private

def _rescue_url_recognition
Expand Down
56 changes: 56 additions & 0 deletions test/routing/http_router_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,60 @@ def rack_request
env['SCRIPT_NAME'].to_s.must_equal '/post'
end
end

describe '#rewrite_partial_path_info' do
describe 'when from partial match' do
it 'sets PATH_INFO correctly' do
request_env = nil
router = Hanami::Routing::HttpRouter.new
router.add("/sidekiq*").to { |env| request_env = env; [200, {}, []] }
router.call(Rack::MockRequest.env_for("/sidekiq/queues"))
request_env['PATH_INFO'].to_s.must_equal '/queues'
end

it 'sets SCRIPT_NAME correctly' do
request_env = nil
router = Hanami::Routing::HttpRouter.new
router.add("/sidekiq*").to { |env| request_env = env; [200, {}, []] }
router.call(Rack::MockRequest.env_for("/sidekiq/queues"))
request_env['SCRIPT_NAME'].to_s.must_equal '/sidekiq'
end
end

describe 'when from partial match of single' do
it 'sets PATH_INFO correctly' do
request_env = nil
router = Hanami::Routing::HttpRouter.new
router.add("/sidekiq*").to { |env| request_env = env; [200, {}, []] }
router.call(Rack::MockRequest.env_for("/sidekiq"))
request_env['PATH_INFO'].to_s.must_equal '/'
end

it 'sets SCRIPT_NAME correctly' do
request_env = nil
router = Hanami::Routing::HttpRouter.new
router.add("/sidekiq*").to { |env| request_env = env; [200, {}, []] }
router.call(Rack::MockRequest.env_for("/sidekiq"))
request_env['SCRIPT_NAME'].to_s.must_equal '/sidekiq'
end
end

describe 'when from encoded path' do
it 'sets PATH_INFO correctly' do
request_env = nil
router = Hanami::Routing::HttpRouter.new
router.add("/sidekiq*").to { |env| request_env = env; [200, {}, []] }
router.call(Rack::MockRequest.env_for("/sidekiq/queues/some%20path"))
request_env['PATH_INFO'].to_s.must_equal '/queues/some%20path'
end

it 'sets SCRIPT_NAME correctly' do
request_env = nil
router = Hanami::Routing::HttpRouter.new
router.add("/sidekiq*").to { |env| request_env = env; [200, {}, []] }
router.call(Rack::MockRequest.env_for("/sidekiq/queues/some%20path"))
request_env['SCRIPT_NAME'].to_s.must_equal '/sidekiq'
end
end
end
end

0 comments on commit 17a2d57

Please sign in to comment.