Look for a 405 neighbour only on paths the method has no route on

CHANGELOG.md

@@ -12,6 +12,7 @@
 * [#2915](https://github.com/ruby-grape/grape/pull/2915): Update rubocop to 1.90.0 and rubocop-performance to 1.27.0 - [@ericproulx](https://github.com/ericproulx).
 * [#2918](https://github.com/ruby-grape/grape/pull/2918): Skip the dry-types round trip when a value already is the declared type - [@ericproulx](https://github.com/ericproulx).
 * [#2917](https://github.com/ruby-grape/grape/pull/2917): Read path captures out of the router's union match instead of re-running the route's pattern - [@ericproulx](https://github.com/ericproulx).
+* [#2920](https://github.com/ruby-grape/grape/pull/2920): Look for a 405 neighbour only on paths the request's method has no route on - [@ericproulx](https://github.com/ericproulx).
 * Your contribution here.
 
 ### 4.0.0 (2026-09-07)

lib/grape/router.rb

@@ -6,17 +6,19 @@ def initialize
       @neutral_map = []
       @neutral_regexes = []
       # Plain hashes with no auto-vivifying default: a lookup for an HTTP method
-      # that has no routes must not insert a key. `compile!` freezes both maps,
+      # that has no routes must not insert a key. `compile!` freezes the maps,
       # so request-time reads never mutate shared state (see #match? / #rotation).
       @map = {}
       @optimized_map = {}
+      # Per HTTP method, the greedy routes still worth trying once that method's
+      # own routes have missed a path (see #compile_neighbour_map).
+      @neighbour_map = {}
     end
 
     def compile!
       return if @compiled
 
       @union = resolve_capture_groups(Regexp.union(@neutral_regexes), @neutral_map)
-      @neutral_regexes = nil
       # Compiled from the routes actually registered rather than from
       # Grape::HTTP_SUPPORTED_METHODS. A route declared with any other verb
       # (`route :purge, '/cache'`) is accepted at definition time and is
@@ -28,8 +30,11 @@ def compile!
         optimized_map = routes.map.with_index { |route, index| route.to_regexp(index) }
         @optimized_map[method] = resolve_capture_groups(Regexp.union(optimized_map), routes)
       end
+      compile_neighbour_map
+      @neutral_regexes = nil
       @map.freeze
       @optimized_map.freeze
+      @neighbour_map.freeze
       @compiled = true
     end
 
@@ -104,7 +109,9 @@ def transaction(input, method, env)
     # +method+ have declined. +cascaded+ says whether any of them matched: the
     # auto-OPTIONS and 405 answers are only right when none did.
     def neighbours(input, method, env, response, cascaded)
-      last_neighbor_route = greedy_match?(input)
+      # Only ever read while nothing for +method+ has matched, so once a route
+      # has -- and cascaded -- there is no neighbour to look for.
+      last_neighbor_route = neighbour_match?(input, method) unless cascaded
 
       # If last_neighbor_route exists and request method is OPTIONS,
       # return response by using #include_allow_header.
@@ -207,6 +214,51 @@ def greedy_match?(input)
       @union.match(input) { |m| @neutral_map.detect { |route| m[route.regexp_capture_group] } }
     end
 
+    # The greedy route for +input+ once the routes for +method+ have all missed
+    # it, from the ones #compile_neighbour_map left to try. A method with no
+    # routes of its own covers no path, so it still walks the whole of @union.
+    def neighbour_match?(input, method)
+      return greedy_match?(input) unless @map.key?(method)
+
+      union, groups = @neighbour_map[method]
+      union&.match(input) { |m| groups.find { |_route, group| m[group] }&.first }
+    end
+
+    # After a miss, #neighbours looks for a greedy route to answer 405 with.
+    # Walking the whole of @union for it would cost a 404 a second walk over
+    # every path, and most of that walk cannot succeed: a greedy route shares
+    # its pattern with the routes it was collected from (see
+    # API::Instance#collect_route_config_per_pattern), so once the union for
+    # +method+ has missed a path, the greedy route of every path +method+ has a
+    # route on has missed it too. What is left to try is the paths +method+ has
+    # no route on -- in an API where every path answers GET, none at all for a
+    # GET.
+    #
+    # Built from the same members as @union, in the same order, so a path
+    # resolves to the same greedy route either way. Methods that leave the same
+    # paths uncovered -- PUT and DELETE on a member, say -- share one union.
+    def compile_neighbour_map
+      unions = {}
+      @map.each do |method, routes|
+        covered = routes.to_set(&:pattern_regexp)
+        uncovered = @neutral_map.each_index.reject { |index| covered.include?(@neutral_map[index].pattern_regexp) }
+        @neighbour_map[method] = unions[uncovered] ||= neighbour_union(uncovered) unless uncovered.empty?
+      end
+    end
+
+    # The union of the greedy routes at +indices+ of @neutral_map, and the
+    # group each of them occupies in it: a subset of @union numbers its groups
+    # differently, so the ones #resolve_capture_groups recorded do not apply.
+    def neighbour_union(indices)
+      union = Regexp.union(@neutral_regexes.values_at(*indices))
+      named_captures = union.named_captures
+      groups = indices.to_h do |index|
+        route = @neutral_map[index]
+        [route, named_captures.fetch(route.regexp_capture_index).first]
+      end
+      [union, groups.freeze].freeze
+    end
+
     def cascade?(response)
       response && response[1]['X-Cascade'] == 'pass'
     end

spec/grape/router_spec.rb

@@ -179,4 +179,78 @@ def response_body
       expect(body['params']).to eq('name' => '123')
     end
   end
+
+  # When no route for the request's method matches its path, the router answers
+  # 405 if a route for another method does, and 404 otherwise. It only tries the
+  # paths the method has no route on, since the others share a pattern with a
+  # route that has just missed -- so these cover a path with no route for the
+  # method, a covered path whose pattern overlaps an uncovered one, and methods
+  # with no routes at all, for which every path is still a candidate.
+  describe 'answering a path no route for the method matches' do
+    let(:app) do
+      Class.new(Grape::API) do
+        format :json
+        get('/items') { 'index' }
+        post('/items') { 'create' }
+        get('/items/:id') { 'show' }
+        put('/items/:id') { 'update' }
+        delete('/items/:id') { 'destroy' }
+        # Digits only, so /users/me misses it and reaches the POST-only path.
+        get('/users/:id', requirements: { id: /\d+/ }) { 'user' }
+        post('/users/me') { 'me' }
+      end
+    end
+
+    it 'answers 405 from a path the method has no route on' do
+      post '/items/1'
+
+      expect(last_response.status).to eq(405)
+      expect(last_response.headers['Allow']).to eq('OPTIONS, GET, PUT, DELETE, HEAD')
+    end
+
+    it 'answers 405 from an uncovered path when a covered one overlaps it' do
+      get '/users/me'
+
+      expect(last_response.status).to eq(405)
+      expect(last_response.headers['Allow']).to eq('OPTIONS, POST')
+    end
+
+    it 'answers 405 to a method that has no routes at all' do
+      patch '/items/1'
+
+      expect(last_response.status).to eq(405)
+      expect(last_response.headers['Allow']).to eq('OPTIONS, GET, PUT, DELETE, HEAD')
+    end
+
+    it 'answers OPTIONS for a path' do
+      options '/items/1'
+
+      expect(last_response.status).to eq(204)
+      expect(last_response.headers['Allow']).to eq('OPTIONS, GET, PUT, DELETE, HEAD')
+    end
+
+    it 'answers 404 to a path no route matches' do
+      get '/nothing/here'
+      expect(last_response.status).to eq(404)
+
+      post '/nothing/here'
+      expect(last_response.status).to eq(404)
+    end
+
+    context 'when every path has a route for the method' do
+      let(:app) do
+        Class.new(Grape::API) do
+          get('/items') { 'index' }
+          get('/items/:id') { 'show' }
+          delete('/items/:id') { 'destroy' }
+        end
+      end
+
+      it 'answers 404 to a path no route matches' do
+        get '/nothing/here'
+
+        expect(last_response.status).to eq(404)
+      end
+    end
+  end
 end