From dac2199f8d5bca2d3c1419c142cb360b93c8881e Mon Sep 17 00:00:00 2001 From: schneems Date: Fri, 9 Mar 2018 16:15:48 -0600 Subject: [PATCH] Fix storing absolute path in cache (again) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Introduced in #441 the `to_load` and `to_link` sets are used to store assets that will be loaded or linked. At the time of implementation it was thought that the key was removed in the Bundle processor before being stored in the cache. Unfortunately this is not the case as Sprockets pipelines are confusing and the Bundle processor gets called in the “self” pipeline which does not get called until after the “default” pipeline produces an asset that is stored to the cache. This PR fixes this issue by forcing the path into a relative path before being put in the cache, and then expanding from relative path after reading from cache. I also added a test that should catch if __any__ absolute values make their way into the cache, not just in the keys that we specify. --- lib/sprockets/loader.rb | 13 ++++++++++++- test/test_caching.rb | 15 +++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/lib/sprockets/loader.rb b/lib/sprockets/loader.rb index f43efa4b7..3680bbaa7 100644 --- a/lib/sprockets/loader.rb +++ b/lib/sprockets/loader.rb @@ -81,6 +81,8 @@ def asset_from_cache(key) asset[:metadata][:links].map! { |uri| expand_from_root(uri) } if asset[:metadata][:links] asset[:metadata][:stubbed].map! { |uri| expand_from_root(uri) } if asset[:metadata][:stubbed] asset[:metadata][:required].map! { |uri| expand_from_root(uri) } if asset[:metadata][:required] + asset[:metadata][:to_load].map! { |uri| expand_from_root(uri) } if asset[:metadata][:to_load] + asset[:metadata][:to_link].map! { |uri| expand_from_root(uri) } if asset[:metadata][:to_link] asset[:metadata][:dependencies].map! { |uri| uri.start_with?("file-digest://") ? expand_from_root(uri) : uri } if asset[:metadata][:dependencies] asset[:metadata].each_key do |k| @@ -140,7 +142,6 @@ def load_from_unloaded(unloaded) # Read into memory and process if theres a processor pipeline if processors.any? - result = call_processors(processors, { environment: self, cache: self.cache, @@ -224,6 +225,16 @@ def store_asset(asset, unloaded) cached_asset[:metadata][:required].map! { |uri| compress_from_root(uri) } end + if cached_asset[:metadata][:to_load] && !cached_asset[:metadata][:to_load].empty? + cached_asset[:metadata][:to_load] = cached_asset[:metadata][:to_load].dup + cached_asset[:metadata][:to_load].map! { |uri| compress_from_root(uri) } + end + + if cached_asset[:metadata][:to_link] && !cached_asset[:metadata][:to_link].empty? + cached_asset[:metadata][:to_link] = cached_asset[:metadata][:to_link].dup + cached_asset[:metadata][:to_link].map! { |uri| compress_from_root(uri) } + end + if cached_asset[:metadata][:dependencies] && !cached_asset[:metadata][:dependencies].empty? cached_asset[:metadata][:dependencies] = cached_asset[:metadata][:dependencies].dup cached_asset[:metadata][:dependencies].map! do |uri| diff --git a/test/test_caching.rb b/test/test_caching.rb index 15721704a..abf033112 100644 --- a/test/test_caching.rb +++ b/test/test_caching.rb @@ -429,6 +429,21 @@ def teardown end end + test "no absolute paths are stored in the cache by accident" do + environment = Sprockets::Environment.new(fixture_path('default')) do |env| + env.append_path(".") + env.cache = @cache + end + cache = environment.cache + def cache.set(key, value, local = false) + if value.to_s.match?(Dir.pwd) + raise "Expected '#{value}' to not contain absolute path '#{Dir.pwd}' but did" + end + end + + environment['schneems.js'] + end + test "no absolute paths are retuned from cache" do env1 = Sprockets::Environment.new(fixture_path('default')) do |env| env.append_path(".")