Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lowercase headers returned from Sprockets 4.2.0 not caught within Actionpack router #47096

Closed
markedmondson opened this issue Jan 21, 2023 · 3 comments · Fixed by #48845 or rails/sprockets#790

Comments

@markedmondson
Copy link
Contributor

markedmondson commented Jan 21, 2023

Steps to reproduce

The Gemlock specifies Sprockets 4.2.0 but this is incompatible with the Actionpack router. In Sprockets 4.2.0, the returned headers were changes to lowercase to support Rack 3, the Actionpack router has case sensitive matching for the X-Cascade header which means failed routes don't cascade to other potential matches.

Toggle the Sprockets version from 4.2.0 to 4.1.1 below to see the issue.

For a Sprockets application serving from /assets and an additional route serving from /assets/some-path, the latter should still respond after Sprockets has failed to match the route.

# frozen_string_literal: true

require "bundler/inline"

gemfile(true) do
  source "https://rubygems.org"

  git_source(:github) { |repo| "https://github.com/#{repo}.git" }

  # Activate the gem you are reporting the issue against.
  gem "rails", "~> 7.0.4.1"
  gem "sprockets", "~> 4.2.0"
  gem "sprockets-rails", "~> 3.4.2"
  gem "capybara"
end

require "rails"
require "action_controller/railtie"
require "action_view/railtie"
require "sprockets/railtie"

class HelloWorld
  def self.call(env)
    [200, {}, ["hello world"]]
  end
end

class TestApp < Rails::Application
  config.root = __dir__
  config.hosts = []
  config.session_store :cookie_store, key: "cookie_store_key"
  config.assets.compile = true
  secrets.secret_key_base = "secret_key_base"

  config.logger = Logger.new($stdout)
  Rails.logger  = config.logger
end

require 'fileutils'
FileUtils.mkdir_p('app/assets/config/')
FileUtils.touch('app/assets/config/manifest.js')

Rails.application.initialize!

TestApp.routes.draw do
  get "/assets/test", to: ::HelloWorld
end

FileUtils.rm('app/assets/config/manifest.js')
FileUtils.rmdir('app/assets/config/')

require "minitest/autorun"
require 'capybara/rails'
require 'capybara/minitest'

class ActionDispatch::IntegrationTest
  # Make the Capybara DSL available in all integration tests
  include Capybara::DSL
  # Make `assert_*` methods behave like Minitest assertions
  include Capybara::Minitest::Assertions

  # Reset sessions and driver between tests
  teardown do
    Capybara.reset_sessions!
    Capybara.use_default_driver
  end
end

class BugTest < ActionDispatch::IntegrationTest
  def test_assets_route
    visit "/assets/test"

    assert_equal 200, page.status_code
    assert_content "hello world"
  end
end

Expected behavior

Both X-Cascade and x-cascade headers should be matched from Sprockets.
Alternatively, Sprockets should be locked to 4.1.1 until this is resolved in Rails.

Actual behavior

X-Cascade header isn't matched and matching routes are missed.

System configuration

Rails version: 7.x & 6.1.x

Ruby version: 3.x

@ghiculescu
Copy link
Member

Hmm, based on my read of rails/sprockets#758 it seems like rack >= 2.2.4 is also required. Do you know what version you are using?

cc @lsylvester

@markedmondson
Copy link
Contributor Author

Using rack 2.2.6.2

@lsylvester
Copy link
Contributor

I am using rack 2.2.6.2 - but I don't rely on this cascade behavior in my applications so I haven't run into this. I have added #47108 to fix this issue.

adrianna-chang-shopify added a commit to adrianna-chang-shopify/rails that referenced this issue Jul 28, 2023
This commit changes the router to use the expected casing for the
x-cascade header: in Rack 2, this is mixed-case, and in Rack 3, this is
lower case.

This also fixes rails#47096.
paulreece pushed a commit to paulreece/rails-paulreece that referenced this issue Aug 26, 2023
This commit changes the router to use the expected casing for the
x-cascade header: in Rack 2, this is mixed-case, and in Rack 3, this is
lower case.

This also fixes rails#47096.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment