From: Tom Hughes <tom@compton.nu>
Date: Tue, 15 Apr 2025 17:24:59 +0000 (+0100)
Subject: Drop custom CORS rack module
X-Git-Tag: live~29^2
X-Git-Url: https://git.openstreetmap.org./rails.git/commitdiff_plain/95ef2ea964343cd846292e86e8ec429b9140fb65

Drop custom CORS rack module

Rack::Cors has handled caching properly for some time now by
adding a Vary header so we no longer need out custom module.
---

diff --git a/config/initializers/cors.rb b/config/initializers/cors.rb
index 710e2c009..75fe503e3 100644
--- a/config/initializers/cors.rb
+++ b/config/initializers/cors.rb
@@ -1,24 +1,11 @@
 # Be sure to restart your server when you modify this file.
 
-# Mark CORS responses as uncacheable as we don't want a browser to
-# try and reuse a response that had a different origin, even with
-# revalidation, as the origin check will fail.
-module OpenStreetMap
-  class Cors < Rack::Cors
-    def call(env)
-      status, headers, body = super
-      headers["Cache-Control"] = "no-cache" if headers["Access-Control-Allow-Origin"]
-      [status, headers, body]
-    end
-  end
-end
-
 # Allow any and all cross-origin requests to the API. Allow any origin, and
 # any headers. Non-browser requests do not have origin or header restrictions,
 # so browser-requests should be similarly permitted. (Though the API does not
 # require any custom headers, Ajax frameworks may automatically add headers
 # such as X-Requested-By to requests.)
-Rails.application.config.middleware.insert_before 0, OpenStreetMap::Cors do
+Rails.application.config.middleware.insert_before 0, Rack::Cors do
   allow do
     origins "*"
     resource "/oauth/*", :headers => :any, :methods => [:get, :post]
diff --git a/test/integration/cors_test.rb b/test/integration/cors_test.rb
index 88201231b..0c17ecfd3 100644
--- a/test/integration/cors_test.rb
+++ b/test/integration/cors_test.rb
@@ -2,19 +2,30 @@ require "test_helper"
 
 class CORSTest < ActionDispatch::IntegrationTest
   def test_api_routes_allow_cross_origin_requests
-    process :options, "/api/capabilities", :headers => {
+    options "/api/capabilities", :headers => {
       "Origin" => "http://www.example.com",
       "Access-Control-Request-Method" => "GET"
     }
 
     assert_response :success
     assert_equal "*", response.headers["Access-Control-Allow-Origin"]
+    assert_nil response.headers["Vary"]
     assert_nil response.media_type
     assert_equal "", response.body
+
+    get "/api/capabilities", :headers => {
+      "Origin" => "http://www.example.com",
+      "Access-Control-Request-Method" => "GET"
+    }
+
+    assert_response :success
+    assert_equal "*", response.headers["Access-Control-Allow-Origin"]
+    assert_equal "Origin", response.headers["Vary"]
+    assert_equal "application/xml", response.media_type
   end
 
   def test_non_api_routes_dont_allow_cross_origin_requests
-    process :options, "/", :headers => {
+    options "/", :headers => {
       "Origin" => "http://www.example.com",
       "Access-Control-Request-Method" => "GET"
     }
@@ -23,5 +34,14 @@ class CORSTest < ActionDispatch::IntegrationTest
     assert_nil response.headers["Access-Control-Allow-Origin"]
     assert_nil response.media_type
     assert_equal "", response.body
+
+    get "/", :headers => {
+      "Origin" => "http://www.example.com",
+      "Access-Control-Request-Method" => "GET"
+    }
+
+    assert_response :success
+    assert_nil response.headers["Access-Control-Allow-Origin"]
+    assert_equal "text/html", response.media_type
   end
 end