From bcb9461557e51653682c3e7ac8dfea89b79c7a96 Mon Sep 17 00:00:00 2001 From: Eric Proulx Date: Sun, 16 Feb 2025 14:15:23 +0100 Subject: [PATCH 1/4] Update normalize_path like rails --- lib/grape/router.rb | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/lib/grape/router.rb b/lib/grape/router.rb index 6889b4213..a9c2aacbc 100644 --- a/lib/grape/router.rb +++ b/lib/grape/router.rb @@ -5,11 +5,22 @@ class Router attr_reader :map, :compiled def self.normalize_path(path) + return '/'.dup 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 From a026c187206584157254668994baea8e90127706 Mon Sep 17 00:00:00 2001 From: Eric Proulx Date: Sun, 16 Feb 2025 14:22:54 +0100 Subject: [PATCH 2/4] Fix cop --- lib/grape/router.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/grape/router.rb b/lib/grape/router.rb index a9c2aacbc..b244107db 100644 --- a/lib/grape/router.rb +++ b/lib/grape/router.rb @@ -5,7 +5,7 @@ class Router attr_reader :map, :compiled def self.normalize_path(path) - return '/'.dup unless 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{%|//})) From e444fefcb5fb04033acd2fae4095268a4e655d1f Mon Sep 17 00:00:00 2001 From: Eric Proulx Date: Sun, 16 Feb 2025 14:24:13 +0100 Subject: [PATCH 3/4] Add CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index d5614ce06..7d7ed3b71 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ #### Features * [#2532](https://github.com/ruby-grape/grape/pull/2532): Update RuboCop 1.71.2 - [@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 From 6432ce7d640dd6e472d1939e2b530081187a0efe Mon Sep 17 00:00:00 2001 From: Eric Proulx Date: Tue, 18 Feb 2025 11:28:40 +0100 Subject: [PATCH 4/4] Add specs and comment --- lib/grape/router.rb | 9 ++++++- spec/grape/router_spec.rb | 57 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 1 deletion(-) create mode 100644 spec/grape/router_spec.rb diff --git a/lib/grape/router.rb b/lib/grape/router.rb index 25984d249..99065b6e4 100644 --- a/lib/grape/router.rb +++ b/lib/grape/router.rb @@ -4,11 +4,18 @@ 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{%|//})) + return path.dup if path == '/' || (path.start_with?('/') && !(path.end_with?('/') || path.match?(%r{%|//}))) # Slow path encoding = path.encoding 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