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

Fix .sprockets-manifest and gzip reproducibility issues #761

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ Get upgrade notes from Sprockets 3.x to 4.x at https://github.com/rails/sprocket

- Add support for Rack 3.0. Headers set by sprockets will now be lower case. [#758](https://github.com/rails/sprockets/pull/758)
- Make `Sprockets::Utils.module_include` thread safe on JRuby. [#759](https://github.com/rails/sprockets/pull/759)
- Improve reproducibility [#761](https://github.com/rails/sprockets/pull/761)

## 4.1.0

Expand Down
15 changes: 11 additions & 4 deletions lib/sprockets/manifest.rb
Original file line number Diff line number Diff line change
Expand Up @@ -56,14 +56,14 @@ def initialize(*args)
@filename = find_directory_manifest(@directory, logger)
end

unless @directory && @filename
raise ArgumentError, "manifest requires output filename"
unless @directory
raise ArgumentError, "manifest requires output directory"
end

data = {}

begin
if File.exist?(@filename)
if [email protected]? && File.exist?(@filename)
data = json_decode(File.read(@filename))
end
rescue JSON::ParserError => e
Expand Down Expand Up @@ -172,7 +172,7 @@ def compile(*args)
end

assets_to_export.each do |asset|
mtime = Time.now.iso8601
mtime = Time.at(1).utc.to_datetime.iso8601 # for reproducibility
files[asset.digest_path] = {
'logical_path' => asset.logical_path,
'mtime' => mtime,
Expand Down Expand Up @@ -277,6 +277,13 @@ def clobber
# Persist manifest back to FS
def save
data = json_encode(@data)

# If directory is given w/o filename, use reproducible manifest location
if @directory && @filename.nil?
etag = DigestUtils.pack_hexdigest(DigestUtils.digest(@data))
@filename = File.join(@directory, ".sprockets-manifest-#{etag}.json")
end

FileUtils.mkdir_p File.dirname(@filename)
PathUtils.atomic_write(@filename) do |f|
f.write(data)
Expand Down
20 changes: 3 additions & 17 deletions lib/sprockets/manifest_utils.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,21 +9,6 @@ module ManifestUtils

MANIFEST_RE = /^\.sprockets-manifest-[0-9a-f]{32}.json$/

# Public: Generate a new random manifest path.
#
# Manifests are not intended to be accessed publicly, but typically live
# alongside public assets for convenience. To avoid being served, the
# filename is prefixed with a "." which is usually hidden by web servers
# like Apache. To help in other environments that may not control this,
# a random hex string is appended to the filename to prevent people from
# guessing the location. If directory indexes are enabled on the server,
# all bets are off.
#
# Return String path.
def generate_manifest_path
".sprockets-manifest-#{SecureRandom.hex(16)}.json"
end

# Public: Find or pick a new manifest filename for target build directory.
#
# dirname - String dirname
Expand All @@ -33,15 +18,16 @@ def generate_manifest_path
# find_directory_manifest("/app/public/assets")
# # => "/app/public/assets/.sprockets-manifest-abc123.json"
#
# Returns String filename.
# Returns String filename or nil if it cannot find one.
def find_directory_manifest(dirname, logger = Logger.new($stderr))
entries = File.directory?(dirname) ? Dir.entries(dirname) : []
manifest_entries = entries.select { |e| e =~ MANIFEST_RE }
if manifest_entries.length > 1
manifest_entries.sort!
logger.warn("Found multiple manifests: #{manifest_entries}. Choosing the first alphabetically: #{manifest_entries.first}")
end
entry = manifest_entries.first || generate_manifest_path
entry = manifest_entries.first
return nil if entry.nil?
File.join(dirname, entry)
end
end
Expand Down
2 changes: 1 addition & 1 deletion lib/sprockets/utils/gzip.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ class Gzip
module ZlibArchiver
def self.call(file, source, mtime)
gz = Zlib::GzipWriter.new(file, Zlib::BEST_COMPRESSION)
gz.mtime = mtime
gz.mtime = 1 # for reproducibility
gz.write(source)
gz.close

Expand Down
3 changes: 1 addition & 2 deletions test/test_manifest.rb
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,10 @@ def teardown
assert_equal filename, manifest.path
end

test "specify manifest directory yields random .sprockets-manifest-*.json" do
test "specify manifest directory reproducible .sprockets-manifest-*.json" do
manifest = Sprockets::Manifest.new(@env, @dir)

assert_equal @dir, manifest.directory
assert_match(/^\.sprockets-manifest-[a-f0-9]{32}.json/, File.basename(manifest.filename))

manifest.save
assert_match(/^\.sprockets-manifest-[a-f0-9]{32}.json/, File.basename(manifest.filename))
Expand Down
6 changes: 4 additions & 2 deletions test/test_manifest_utils.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,10 @@
class TestManifestUtils < MiniTest::Test
include Sprockets::ManifestUtils

def test_generate_manifest_path
assert_match(MANIFEST_RE, generate_manifest_path)
def test_returns_no_manifest_nil
root = File.expand_path("../fixtures/manifest_utils", __FILE__)
assert_equal nil,
find_directory_manifest("#{root}/")
end

def test_find_directory_manifest
Expand Down