Skip to content

Commit

Permalink
Don't serve up assets without digests in development
Browse files Browse the repository at this point in the history
If config.assets.digest = true and
config.assets.raise_runtime_errors = true, only serve an asset if
the request has a digest.

Here are the different cases:
- If the asset request has a digest and it matches the asset's digest, we
serve the asset.
- If the asset request has a digest and it does not match the asset's
  digest, we redirect to the asset.
- If the asset request does not have a digest, we raise a NoDigestError.

Fixes rails#117
  • Loading branch information
Dan Kang committed May 16, 2014
1 parent 94a0332 commit 6e5b82f
Show file tree
Hide file tree
Showing 7 changed files with 170 additions and 5 deletions.
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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*
Expand Down
4 changes: 3 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`**

Expand Down
39 changes: 39 additions & 0 deletions lib/sprockets/rails/environment.rb
Original file line number Diff line number Diff line change
@@ -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
5 changes: 3 additions & 2 deletions lib/sprockets/railtie.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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'

Expand All @@ -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}"
Expand Down
114 changes: 114 additions & 0 deletions test/test_environment.rb
Original file line number Diff line number Diff line change
@@ -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
3 changes: 2 additions & 1 deletion test/test_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
Expand Down
3 changes: 2 additions & 1 deletion test/test_task.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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')
Expand Down

0 comments on commit 6e5b82f

Please sign in to comment.