diff --git a/CHANGELOG.md b/CHANGELOG.md index 66aa5cb6..98866c59 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,12 @@ ### Unreleased +* Don't serve up assets without digests in development. + + If `config.assets.digest = true` and `config.assets.raise_runtime_errors = true`, + serve an asset only if the request has a digest. + + *Dan Kang* + * Fix issues related `config.assets.manifest` option, including issues with `assets:precompile` Rake task. *Johnny Shields* diff --git a/README.md b/README.md index e6bde0ff..6f3d7a63 100644 --- a/README.md +++ b/README.md @@ -89,7 +89,9 @@ Defines the full path to be used for the asset precompiler's manifest file. Defa **`config.assets.digest`** -Link to undigest asset filenames. This option will eventually go away. Unless when `compile` is disabled. +When enabled, fingerprints will be added to asset filenames. +If `config.assets.raise_runtime_errors` is also enabled, requests for assets +will require fingerprints. **`config.assets.debug`** diff --git a/lib/sprockets/rails/environment.rb b/lib/sprockets/rails/environment.rb new file mode 100644 index 00000000..29e1cf45 --- /dev/null +++ b/lib/sprockets/rails/environment.rb @@ -0,0 +1,39 @@ +require 'sprockets' +require 'sprockets/rails/helper' + +module Sprockets + module Rails + class Environment < Sprockets::Environment + class NoDigestError < StandardError + def initialize(asset) + msg = "Assets should not be requested directly without their digests: " << + "Use the helpers in ActionView::Helpers to request #{asset}" + super(msg) + end + end + + def call(env) + if Sprockets::Rails::Helper.raise_runtime_errors && context_class.digest_assets + path = unescape(env['PATH_INFO'].to_s.sub(/^\//, '')) + + if fingerprint = path_fingerprint(path) + path = path.sub("-#{fingerprint}", '') + else + raise NoDigestError.new(path) + end + + asset = find_asset(path) + if asset && asset.digest != fingerprint + asset_path = File.join(context_class.assets_prefix || "/", asset.digest_path) + asset_path += '?' + env['QUERY_STRING'] if env['QUERY_STRING'] + [302, {"Location" => asset_path}, []] + else + super(env) + end + else + super(env) + end + end + end + end +end diff --git a/lib/sprockets/railtie.rb b/lib/sprockets/railtie.rb index 1c5448b2..205a34eb 100644 --- a/lib/sprockets/railtie.rb +++ b/lib/sprockets/railtie.rb @@ -3,6 +3,7 @@ require 'action_controller/railtie' require 'active_support/core_ext/module/remove_method' require 'sprockets' +require 'sprockets/rails/environment' require 'sprockets/rails/helper' require 'sprockets/rails/version' @@ -18,9 +19,9 @@ class Configuration remove_possible_method :assets remove_possible_method :assets= - # Returns Sprockets::Environment for app config. + # Returns Sprockets::Rails::Environment for app config. def assets - @assets ||= Sprockets::Environment.new(root.to_s) do |env| + @assets ||= Sprockets::Rails::Environment.new(root.to_s) do |env| env.version = ::Rails.env path = "#{config.root}/tmp/cache/assets/#{::Rails.env}" diff --git a/test/test_environment.rb b/test/test_environment.rb new file mode 100644 index 00000000..34e417e5 --- /dev/null +++ b/test/test_environment.rb @@ -0,0 +1,114 @@ +require 'minitest/autorun' + +require 'rack/test' +require 'sprockets/rails/environment' +require 'sprockets/rails/helper' + +Minitest::Test = MiniTest::Unit::TestCase unless defined?(Minitest::Test) + +class EnvironmentTest < Minitest::Test + include Rack::Test::Methods + + FIXTURES_PATH = File.expand_path("../fixtures", __FILE__) + + def setup + @assets = Sprockets::Rails::Environment.new + @assets.append_path FIXTURES_PATH + @assets.context_class.class_eval do + include ::Sprockets::Rails::Helper + end + @assets.context_class.assets_prefix = "/assets" + @assets.context_class.digest_assets = true + + @foo_js_digest = @assets['foo.js'].digest + + Sprockets::Rails::Helper.raise_runtime_errors = true + end + + def default_app + env = @assets + + Rack::Builder.new do + map "/assets" do + run env + end + end + end + + def app + @app ||= default_app + end +end + +class DigestTest < EnvironmentTest + def setup + super + @assets.context_class.digest_assets = true + end + + def test_assets_with_digest + get "/assets/foo-#{@foo_js_digest}.js" + assert_equal 200, last_response.status + end + + def test_assets_with_no_digest + assert_raises(Sprockets::Rails::Environment::NoDigestError) do + get "/assets/foo.js" + end + end + + def test_assets_with_wrong_digest + wrong_digest = "0" * 32 + get "/assets/foo-#{wrong_digest}.js" + assert_equal 302, last_response.status + + follow_redirect! + assert_equal "/assets/foo-#{@foo_js_digest}.js", last_request.path + assert_equal 200, last_response.status + end + + def test_assets_with_wrong_digest_with_query_parameters + wrong_digest = "0" * 32 + get "/assets/foo-#{wrong_digest}.js?body=1" + assert_equal 302, last_response.status + + follow_redirect! + assert_equal "/assets/foo-#{@foo_js_digest}.js", last_request.path + assert_equal "body=1", last_request.query_string + assert_equal 200, last_response.status + end +end + +class NoDigestTest < EnvironmentTest + def setup + super + @assets.context_class.digest_assets = false + end + + def test_assets_with_digest + get "/assets/foo-#{@foo_js_digest}.js" + assert_equal 200, last_response.status + end + + def test_assets_with_no_digest + get "/assets/foo.js" + assert_equal 200, last_response.status + end +end + +class NoRuntimeErrorTest < EnvironmentTest + def setup + super + Sprockets::Rails::Helper.raise_runtime_errors = false + end + + def test_assets_with_digest + get "/assets/foo-#{@foo_js_digest}.js" + assert_equal 200, last_response.status + end + + def test_assets_with_no_digest + get "/assets/foo.js" + assert_equal 200, last_response.status + end +end diff --git a/test/test_helper.rb b/test/test_helper.rb index f90ddb36..b3d47314 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -2,6 +2,7 @@ require 'action_view' require 'sprockets' +require 'sprockets/rails/environment' require 'sprockets/rails/helper' Minitest::Test = MiniTest::Unit::TestCase unless defined?(Minitest::Test) @@ -10,7 +11,7 @@ class HelperTest < Minitest::Test FIXTURES_PATH = File.expand_path("../fixtures", __FILE__) def setup - assets = @assets = Sprockets::Environment.new + assets = @assets = Sprockets::Rails::Environment.new @assets.append_path FIXTURES_PATH @assets.context_class.class_eval do include ::Sprockets::Rails::Helper diff --git a/test/test_task.rb b/test/test_task.rb index 49e93192..aacc57d0 100644 --- a/test/test_task.rb +++ b/test/test_task.rb @@ -2,6 +2,7 @@ require 'tmpdir' require 'sprockets' +require 'sprockets/rails/environment' require 'sprockets/rails/task' Minitest::Test = MiniTest::Unit::TestCase unless defined?(Minitest::Test) @@ -13,7 +14,7 @@ def setup @rake = Rake::Application.new Rake.application = @rake - @assets = Sprockets::Environment.new + @assets = Sprockets::Rails::Environment.new @assets.append_path FIXTURES_PATH @dir = File.join(Dir::tmpdir, 'rails', 'task')