Interference between Marketable Pages & ActiveStorage (Rails 5.2)

[Obi-TOB]

RefineryCMS (with Rails 5.2. support) in combination with SpreeCommerce 3.6.2:

When mounting Refinery in root

Rails.application.routes.draw do
  mount Spree::Core::Engine, at: '/shop'
  mount Refinery::Core::Engine, at: '/'
end

marketable_page routes interfere with ActiveStorage, i.e. the Refinery route matches and Refinery::PagesController takes over request:

Started GET "/rails/active_storage/disk/eyJf [...] g.jpeg" for 127.0.0.1 at 2018-07-20 21:31:58 +0200
Processing by Refinery::PagesController#show as JPEG
  Parameters: {"content_type"=>"image/jpeg", "disposition"=>"inline; filename=\"ror_bag.jpeg\"; filename*=UTF-8''ror_bag.jpeg", "path"=>"rails/active_storage/disk/eyJ [..] g", "locale"=>"en"}

I'm not really sure why this happens as the ActiveStorage routes are above the Refinery routes:

rake routes | grep 'pages#show\|rails'
[...]
rails_disk_service GET  /rails/active_storage/disk/:encoded_key/*filename(.:format)                              active_storage/disk#show
[...]
marketable_page GET    /*path(.:format)                           refinery/pages#show
[...]

Removing the mount point for Refinery or moving it to a different path however resolves the issue.

I resolve this now by a hacky route constraint in Refinery, however I would be interested why this happpens and how to resolve it 'right'.

def append_marketable_routes
  Refinery::Core::Engine.routes.append do
    get '*path', :to => 'pages#show', constraints: lambda { |req| !req.env["REQUEST_URI"].starts_with? "/rails/active_storage/" }, :as => :marketable_page
  end
end

[bricesanchez]

Hi @Obi-TOB ! Thanks for you bug report!

We do not use ActiveStorage so we did not see this bug, did you tried to add the keys rails and active_storage to your friendly_id_reserved_words config in config/initilizers/refinery/pages.rb

https://github.com/refinery/refinerycms/blob/master/pages/lib/refinery/pages/configuration.rb#L64

[Obi-TOB]

Hi @bricesanchez!

I tried this:

  config.friendly_id_reserved_words = ["index", "new", "session", "login", "logout", "users", "refinery", "admin", "images"]
  config.friendly_id_reserved_words << ["rails", "active_storage", "rails/active_storage"]

bit it did not help. I would think these friendly_id_reserved_words prohibit that someone saves pages with slugs containing these words. However they don't seem to change the behavior of routes.

I think this is more an ActiveStorage issue than a refineryCMS issue. See rails/rails#31228

Although I do not think that a route constraint is a good solution, however I don't see that anyone has come up with a better solution so far, so if you want me to prepare a pull request I can do so.

[bricesanchez]

It's also used in this method

refinerycms/pages/lib/refinery/pages/engine.rb

Lines 56 to 66 in 911cde3

	def add_route_parts_as_reserved_words
	ActiveSupport.on_load(:active_record) do
	# do not add routes with :allow_slug => true
	included_routes = Rails.application.routes.named_routes.to_a.reject{ |name, route| route.defaults[:allow_slug] }
	route_paths = included_routes.map { |name, route| route.path.spec }
	route_paths.reject! { |path| path.to_s =~ %r{^/(rails|refinery)}}
	Refinery::Pages.friendly_id_reserved_words |= route_paths.map { |path|
	path.to_s.gsub(%r{^/}, '').to_s.split('(').first.to_s.split(':').first.to_s.split('/')
	}.flatten.reject { |w| w =~ %r{_|\.} }.uniq
	end
	end

@parndt Could you help us to understand how this method works, it's pretty hard to understand and i'm not sure if it still works? :D

[parndt]

That method is pretty complex, huh? But it isn't anything to do with this. The method you're after is this one:

refinerycms/pages/lib/refinery/pages/engine.rb

Lines 49 to 53 in 911cde3

	def append_marketable_routes
	Refinery::Core::Engine.routes.append do
	get '*path', :to => 'pages#show', :as => :marketable_page
	end
	end

[parndt]

I would say that the best way to fix this is to add constraints like @Obi-TOB recommends. Maybe it can be

constraints: -> { |request| !request.env['REQUEST_URI'].start_with?('/rails/active_storage') }

[parndt]

What might also work is mounting Spree after Refinery as, from memory, in a routes file the routes that are declared last override routes declared before that point. However, that won't fix our underlying issue.

What we could also do is expose a configuration value in pages config like:

config.reserved_paths = %w(/active_storage)

Then we can use it like this:

constraints: -> { |request| !Refinery::Pages.config.reserved_paths.any? { |path| request.env['REQUEST_URI'].start_with?(path) } }

[parndt]

Also see this rails/rails#31228 (comment)

[bricesanchez]

Hi @parndt, thanks for your answers!

I like to add reserved_paths config. It looks better than rails/rails#31228 (comment)

@Obi-TOB Would you like to provide a Pull Request?