From 11c64a58996156a3572c458a0d437a3909842bc4 Mon Sep 17 00:00:00 2001 From: edubraqd Date: Thu, 17 Sep 2026 09:16:07 -0300 Subject: [PATCH 1/2] Answer /api and /api/ with the JSON 404 The catch-all under the api namespace was `*unmatched`, and a wildcard segment needs at least one character, so the namespace root itself fell through to the HTML 404 page. Making the glob optional turns the route into `/api(/*unmatched)`, which also covers the bare prefix. Adds an integration test for /api, /api/, /api/v1 and /api/nope (GET) and /api (POST); the first two and the POST fail without the change. Fixes #89 Co-Authored-By: Claude Opus 5 --- config/routes.rb | 6 +++-- test/integration/api_unmatched_route_test.rb | 25 ++++++++++++++++++++ 2 files changed, 29 insertions(+), 2 deletions(-) create mode 100644 test/integration/api_unmatched_route_test.rb diff --git a/config/routes.rb b/config/routes.rb index 51533f2..5edd550 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -22,8 +22,10 @@ # Everything under /api answers in JSON, including a wrong path. Without # this, an unknown route falls through to the HTML 404 page and the phone - # gets markup where it expected a body it can parse. - match "*unmatched", to: "base#unmatched_route", via: :all, format: false + # gets markup where it expected a body it can parse. The glob is optional + # because a wildcard needs at least one character: without the parentheses + # the namespace root itself (/api, /api/) would still fall through to HTML. + match "(*unmatched)", to: "base#unmatched_route", via: :all, format: false end # Reveal health status on /up that returns 200 if the app boots with no exceptions, otherwise 500. diff --git a/test/integration/api_unmatched_route_test.rb b/test/integration/api_unmatched_route_test.rb new file mode 100644 index 0000000..4ad37e9 --- /dev/null +++ b/test/integration/api_unmatched_route_test.rb @@ -0,0 +1,25 @@ +require "test_helper" + +# Every path under /api must answer in JSON, including the namespace root +# itself: a client that hits the bare prefix should get the same body it can +# parse for any other typo, not the HTML 404 page. +class ApiUnmatchedRouteTest < ActionDispatch::IntegrationTest + [ "/api", "/api/", "/api/v1", "/api/nope" ].each do |path| + test "GET #{path} returns a json 404" do + get path, as: :json + + assert_response :not_found + assert_equal "application/json", response.media_type + assert_equal "not_found", response.parsed_body["error"] + assert_equal "No route matches GET #{path.delete_suffix("/")}", response.parsed_body["message"] + end + end + + test "POST /api returns a json 404" do + post "/api", as: :json + + assert_response :not_found + assert_equal "application/json", response.media_type + assert_equal "not_found", response.parsed_body["error"] + end +end From 38cd85a2cb361ca3c0f639d5e83b3fca5183a7f5 Mon Sep 17 00:00:00 2001 From: edubraqd Date: Thu, 17 Sep 2026 09:51:43 -0300 Subject: [PATCH 2/2] Move the API catch-all to its own controller and reject non-JSON formats `unmatched_route` was a public method on `Api::BaseController`, so every API controller inherited it as an action (`Api::V1::TasksController .action_methods` included it). It now lives on a dedicated `Api::ErrorsController`, which is the only thing the catch-all route reaches. `GET /api/v1/tasks.xml` answered 200 with a JSON body, as if the extension had been honoured. The tasks resources now carry `constraints: { format: :json }`: `/tasks` and `/tasks.json` behave as before, and any other extension (`/tasks.xml`, `/tasks/1.xml`) falls through to the JSON 404 like any other unknown path. Tests cover both, and assert the catch-all is no longer an action on the tasks controller. Fixes #90 Fixes #91 Co-Authored-By: Claude Opus 5 --- app/controllers/api/base_controller.rb | 9 ---- app/controllers/api/errors_controller.rb | 16 ++++++ config/routes.rb | 8 ++- test/integration/api_unmatched_route_test.rb | 55 +++++++++++++++++--- 4 files changed, 69 insertions(+), 19 deletions(-) create mode 100644 app/controllers/api/errors_controller.rb diff --git a/app/controllers/api/base_controller.rb b/app/controllers/api/base_controller.rb index 47e5b00..5c38439 100644 --- a/app/controllers/api/base_controller.rb +++ b/app/controllers/api/base_controller.rb @@ -14,15 +14,6 @@ class BaseController < ActionController::API # cannot read. rescue_from ActionDispatch::Http::Parameters::ParseError, with: :malformed_json - # Anything under /api that does not match a route: a typo, or a client built - # against a newer version of this API. Still answered in JSON. - def unmatched_route - render json: { - error: "not_found", - message: "No route matches #{request.request_method} #{request.path}" - }, status: :not_found - end - private def not_found(error) render json: { error: "not_found", message: error.message }, status: :not_found diff --git a/app/controllers/api/errors_controller.rb b/app/controllers/api/errors_controller.rb new file mode 100644 index 0000000..49542f4 --- /dev/null +++ b/app/controllers/api/errors_controller.rb @@ -0,0 +1,16 @@ +module Api + # Answers the catch-all route under /api. It is the only action that route + # reaches, and it lives here rather than on BaseController so that it is not + # inherited as a public action by every API controller. + class ErrorsController < Api::BaseController + # Anything under /api that does not match a route: a typo, a format the + # API does not speak, or a client built against a newer version of this + # API. Still answered in JSON. + def unmatched_route + render json: { + error: "not_found", + message: "No route matches #{request.request_method} #{request.path}" + }, status: :not_found + end + end +end diff --git a/config/routes.rb b/config/routes.rb index 5edd550..478ee3e 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -17,7 +17,11 @@ namespace :v1 do # No new/edit: those serve HTML forms, which this API never renders. They # would also shadow the JSON 404 below with a missing action error. - resources :tasks, except: %i[ new edit ] + # + # The format constraint keeps /tasks and /tasks.json, and sends any other + # extension (/tasks.xml, /tasks/1.xml) to the JSON 404 below instead of + # quietly answering it with JSON as if the extension had been honoured. + resources :tasks, except: %i[ new edit ], constraints: { format: :json } end # Everything under /api answers in JSON, including a wrong path. Without @@ -25,7 +29,7 @@ # gets markup where it expected a body it can parse. The glob is optional # because a wildcard needs at least one character: without the parentheses # the namespace root itself (/api, /api/) would still fall through to HTML. - match "(*unmatched)", to: "base#unmatched_route", via: :all, format: false + match "(*unmatched)", to: "errors#unmatched_route", via: :all, format: false end # Reveal health status on /up that returns 200 if the app boots with no exceptions, otherwise 500. diff --git a/test/integration/api_unmatched_route_test.rb b/test/integration/api_unmatched_route_test.rb index 4ad37e9..72647e9 100644 --- a/test/integration/api_unmatched_route_test.rb +++ b/test/integration/api_unmatched_route_test.rb @@ -1,25 +1,64 @@ require "test_helper" # Every path under /api must answer in JSON, including the namespace root -# itself: a client that hits the bare prefix should get the same body it can -# parse for any other typo, not the HTML 404 page. +# itself and a format the API does not speak: a client that hits the bare +# prefix or asks for .xml should get the same body it can parse for any other +# typo, not the HTML 404 page or a JSON body pretending to be XML. class ApiUnmatchedRouteTest < ActionDispatch::IntegrationTest [ "/api", "/api/", "/api/v1", "/api/nope" ].each do |path| test "GET #{path} returns a json 404" do get path, as: :json - assert_response :not_found - assert_equal "application/json", response.media_type - assert_equal "not_found", response.parsed_body["error"] - assert_equal "No route matches GET #{path.delete_suffix("/")}", response.parsed_body["message"] + assert_json_not_found "GET", path end end test "POST /api returns a json 404" do post "/api", as: :json - assert_response :not_found + assert_json_not_found "POST", "/api" + end + + test "GET /api/v1/tasks.xml returns a json 404" do + get "/api/v1/tasks.xml" + + assert_json_not_found "GET", "/api/v1/tasks.xml" + end + + test "GET /api/v1/tasks/:id.xml returns a json 404 for an existing task" do + path = "/api/v1/tasks/#{tasks(:pending_task).id}.xml" + get path + + assert_json_not_found "GET", path + end + + test "POST /api/v1/tasks.xml does not create a task" do + assert_no_difference "Task.count" do + post "/api/v1/tasks.xml", params: { task: { title: "Ship it" } }, as: :json + end + + assert_json_not_found "POST", "/api/v1/tasks.xml" + end + + test "GET /api/v1/tasks.json still answers" do + get "/api/v1/tasks.json" + + assert_response :success assert_equal "application/json", response.media_type - assert_equal "not_found", response.parsed_body["error"] end + + test "the catch-all is not an action on every API controller" do + assert_not_includes Api::V1::TasksController.action_methods, "unmatched_route" + assert_includes Api::ErrorsController.action_methods, "unmatched_route" + end + + private + def assert_json_not_found(method, path) + assert_response :not_found + assert_equal "application/json", response.media_type + assert_equal "not_found", response.parsed_body["error"] + # Rails drops a trailing slash from request.path before it reaches the + # controller, so /api/ reports itself as /api. + assert_equal "No route matches #{method} #{path.delete_suffix("/")}", response.parsed_body["message"] + end end