Permalink
Browse files

Process archives in a backwards-compatible way

For the time being, we'll use libarchive, along with a relaxed and less
efficient metadata seeking process to process cookbook archives. Doing
so allows the data migration to complete. This commit should revert
cleanly once the backwards-compatible period is over.
  • Loading branch information...
bcobb committed Jun 23, 2014
1 parent 9e8f5ac commit 21114b276a68dd677038479f1cfab56b24778235
@@ -9,6 +9,9 @@ notifications:
on_start: false # default: false
rvm:
- 2.0.0
before_install:
- sudo apt-get update -qq
- sudo apt-get install -qq libarchive-dev
before_script:
- psql -c 'create database supermarket_test;' -U postgres
- psql supermarket_test -c 'create extension pg_trgm' -U postgres
@@ -34,6 +34,7 @@ gem 'semverse'
gem 'sitemap_generator'
gem 'redis-rails'
gem 'yajl-ruby'
gem 'libarchive', github: 'bcobb/winebarrel-libarchive-ruby-clone', ref: 'format-security-fix'
gem 'sentry-raven', '~> 0.8.0', require: false
gem 'statsd-ruby', require: 'statsd'
@@ -1,3 +1,10 @@
GIT
remote: git://github.com/bcobb/winebarrel-libarchive-ruby-clone.git
revision: a5a748c731415cb622282350dc92c82f8de77e5d
ref: format-security-fix
specs:
libarchive (0.1.2)
GIT
remote: git://github.com/gofullstack/chef-legacy.git
revision: 3a27451f99f06b93b178b10e660e0b65f4dbfa99
@@ -460,6 +467,7 @@ DEPENDENCIES
jbuilder
kaminari
launchy
libarchive!
license_finder
magiconf
mail_view
@@ -1,4 +1,4 @@
require 'rubygems/package'
require 'libarchive'
class CookbookUpload
#
@@ -39,10 +39,10 @@ def initialize(source)
def find(pattern)
matches = []
each do |entry|
next unless entry.full_name.match(pattern)
each do |entry, _|
next unless entry.pathname.match(pattern)
matches << entry.full_name
matches << entry.pathname
end
matches
@@ -59,10 +59,10 @@ def find(pattern)
def read(path)
match = nil
each do |entry|
next unless entry.full_name == path
each do |entry, archive|
next unless entry.pathname == path
match = entry.read
match = archive.read_data
break
end
@@ -78,24 +78,31 @@ def read(path)
# @raise [NoPath] if the source has no path
# @raise [Error] if the source is not a compatible archive
#
# @yieldparam [::Gem::Package::TarReader::Entry] entry
# @yieldparam [::Archive::Entry] entry
# @yieldparam [::Archive::Reader] archive
#
# @example
# archive = CookbookUpload::Archive.new(tarball)
# archive.each do |entry|
# puts "#{entry.full_name} has the following content:\n#{entry.read}"
# archive.each do |entry, archive|
# puts "#{entry.pathname} has the following content:\n#{archive.read_data}"
# end
#
def each
raise NoPath unless @source.respond_to?(:path)
begin
Zlib::GzipReader.open(@source.path) do |gzip|
Gem::Package::TarReader.new(gzip) do |tar|
tar.each { |entry| yield entry }
::Archive.read_open_filename(@source.path) do |archive|
loop do
entry = archive.next_header
if entry
yield entry, archive
else
break
end
end
end
rescue Zlib::GzipFile::Error => e
rescue ::Archive::Error => e
raise Error, e.message
end
end
@@ -140,19 +140,29 @@ def parse_tarball_metadata(&block)
errors = ActiveModel::Errors.new([])
begin
path = archive.find(%r{\A(\.\/)?[^\/]+\/metadata\.json\Z}).first
paths = archive.find(%r{\A(\.\/)?[^\/]+\/metadata\.json})
if path
metadata = Metadata.new(JSON.parse(archive.read(path)))
json, non_json = paths.partition do |path|
begin
JSON.parse(archive.read(path))
rescue JSON::ParserError
false
end
end
if json.empty?
if non_json.any?
errors.add(:base, I18n.t('api.error_messages.metadata_not_json'))
else
errors.add(:base, I18n.t('api.error_messages.missing_metadata'))
end
else
errors.add(:base, I18n.t('api.error_messages.missing_metadata'))
metadata = Metadata.new(JSON.parse(archive.read(json.first)))
end
rescue JSON::ParserError
errors.add(:base, I18n.t('api.error_messages.metadata_not_json'))
rescue Virtus::CoercionError
errors.add(:base, I18n.t('api.error_messages.invalid_metadata'))
rescue Archive::Error
errors.add(:base, I18n.t('api.error_messages.tarball_not_gzipped'))
errors.add(:base, I18n.t('api.error_messages.tarball_not_archive'))
rescue Archive::NoPath
errors.add(:base, I18n.t('api.error_messages.tarball_has_no_path'))
end
@@ -184,7 +194,7 @@ def extract_tarball_readme(&block)
errors.add(:base, I18n.t('api.error_messages.missing_readme'))
end
rescue Archive::Error
errors.add(:base, I18n.t('api.error_messages.tarball_not_gzipped'))
errors.add(:base, I18n.t('api.error_messages.tarball_not_archive'))
rescue Archive::NoPath
errors.add(:base, I18n.t('api.error_messages.tarball_has_no_path'))
end
@@ -40,7 +40,7 @@ en:
missing_tarball: "Mulipart POST must include a part named 'tarball'"
tarball_has_no_path: "Multipart POST part 'tarball' must be a file"
non_existent_category: "Category '%{category_name}' does not exist"
tarball_not_gzipped: "Multipart POST part 'tarball' must be GZipped"
tarball_not_archive: "Multipart POST part 'tarball' must be a file archive"
missing_metadata: "Multipart POST part 'tarball' must contain a metadata.json entry"
missing_readme: "Multipart POST part 'tarball' must contain a README"
metadata_not_json: "Tarball entry named 'metadata.json' could not be parsed as JSON"
@@ -60,6 +60,26 @@ def params(hash)
expect(params.metadata.name).to eql('multiple')
end
it 'is not extracted from metadata.json~' do
tarball = Tempfile.new('weird-metadata', 'tmp').tap do |file|
io = AndFeathers.build('weird-metadata') do |base|
base.file('metadata.json') do
JSON.dump(name: 'correct')
end
base.file('metadata.json~') do
'this is not json'
end
end.to_io(AndFeathers::GzippedTarball, :reverse_each)
file.write(io.read)
file.rewind
end
params = params(cookbook: '{}', tarball: tarball)
expect(params.metadata.name).to eql('correct')
end
it 'is blank if the tarball parameter is not a file' do
params = params(cookbook: '{}', tarball: 'tarball!')
@@ -99,14 +99,14 @@
to include(I18n.t('api.error_messages.tarball_has_no_path'))
end
it 'yields an error if the tarball is not GZipped' do
tarball = File.open('spec/support/cookbook_fixtures/not-actually-gzipped.tgz')
it 'yields an error if the tarball is not an archive' do
tarball = File.open('spec/support/cookbook_fixtures/not-an-archive.tgz')
upload = CookbookUpload.new(user, cookbook: '{}', tarball: tarball)
errors = upload.finish { |e, _| e }
expect(errors.full_messages).
to include(I18n.t('api.error_messages.tarball_not_gzipped'))
to include(I18n.t('api.error_messages.tarball_not_archive'))
end
it 'yields an error if the tarball has no metadata.json entry' do
Binary file not shown.

0 comments on commit 21114b2

Please sign in to comment.