diff --git a/CHANGELOG.md b/CHANGELOG.md index ce9cd11fa..7cbf377b5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ * [#2532](https://github.com/ruby-grape/grape/pull/2532): Update RuboCop 1.71.2 - [@ericproulx](https://github.com/ericproulx). * [#2535](https://github.com/ruby-grape/grape/pull/2535): Delegates calls to inner objects - [@ericproulx](https://github.com/ericproulx). * [#2537](https://github.com/ruby-grape/grape/pull/2537): Use activesupport `try` pattern - [@ericproulx](https://github.com/ericproulx). +* [#2536](https://github.com/ruby-grape/grape/pull/2536): Update normalize_path like Rails - [@ericproulx](https://github.com/ericproulx). * Your contribution here. #### Fixes diff --git a/lib/grape/router.rb b/lib/grape/router.rb index 1b79324ae..99065b6e4 100644 --- a/lib/grape/router.rb +++ b/lib/grape/router.rb @@ -4,12 +4,30 @@ module Grape class Router attr_reader :map, :compiled + # Taken from Rails + # normalize_path("/foo") # => "/foo" + # normalize_path("/foo/") # => "/foo" + # normalize_path("foo") # => "/foo" + # normalize_path("") # => "/" + # normalize_path("/%ab") # => "/%AB" + # https://github.com/rails/rails/blob/00cc4ff0259c0185fe08baadaa40e63ea2534f6e/actionpack/lib/action_dispatch/journey/router/utils.rb#L19 def self.normalize_path(path) + return +'/' unless path + + # Fast path for the overwhelming majority of paths that don't need to be normalized + return path.dup if path == '/' || (path.start_with?('/') && !(path.end_with?('/') || path.match?(%r{%|//}))) + + # Slow path + encoding = path.encoding path = +"/#{path}" path.squeeze!('/') - path.sub!(%r{/+\Z}, '') - path = '/' if path == '' - path + + unless path == '/' + path.delete_suffix!('/') + path.gsub!(/(%[a-f0-9]{2})/) { ::Regexp.last_match(1).upcase } + end + + path.force_encoding(encoding) end def initialize diff --git a/spec/grape/router_spec.rb b/spec/grape/router_spec.rb new file mode 100644 index 000000000..afa05d461 --- /dev/null +++ b/spec/grape/router_spec.rb @@ -0,0 +1,57 @@ +# frozen_string_literal: true + +describe Grape::Router do + describe '.normalize_path' do + subject { described_class.normalize_path(path) } + + context 'when no leading slash' do + let(:path) { 'foo%20bar%20baz' } + + it { is_expected.to eq '/foo%20bar%20baz' } + end + + context 'when path ends with slash' do + let(:path) { '/foo%20bar%20baz/' } + + it { is_expected.to eq '/foo%20bar%20baz' } + end + + context 'when path has recurring slashes' do + let(:path) { '////foo%20bar%20baz' } + + it { is_expected.to eq '/foo%20bar%20baz' } + end + + context 'when not greedy' do + let(:path) { '/foo%20bar%20baz' } + + it { is_expected.to eq '/foo%20bar%20baz' } + end + + context 'when encoded string in lowercase' do + let(:path) { '/foo%aabar%aabaz' } + + it { is_expected.to eq '/foo%AAbar%AAbaz' } + end + + context 'when nil' do + let(:path) { nil } + + it { is_expected.to eq '/' } + end + + context 'when empty string' do + let(:path) { '' } + + it { is_expected.to eq '/' } + end + + context 'when encoding is different' do + subject { described_class.normalize_path(path).encoding } + + let(:path) { '/foo%AAbar%AAbaz'.b } + + it { is_expected.to eq(Encoding::BINARY) } + end + end +end