Skip to content

Commit

Permalink
Check archive names prior to writing anything to remote server.
Browse files Browse the repository at this point in the history 
Also, check asset names after globbing, not before.
  • Loading branch information
@clonezone
clonezone committed Oct 8, 2014
1 parent 7b42735 commit 9b0041b
Show file tree
Hide file tree
Showing 6 changed files with 173 additions and 71 deletions.
9 changes: 9 additions & 0 deletions Changes
View file
@@ -1,3 +1,12 @@
v1.20.1.beta.1 - 2014/10/x

Bug fixes:

- During publishing, now checks asset names after file name globbing, not
before. Also, archive names are checked for extraction support prior to
writing to remote repository. (Yes, that's the archive /names/ and not the
archive /files/.)

v1.20.0 - 2014/10/7

New features:
Expand Down
1 change: 1 addition & 0 deletions TODO.md
View file
@@ -1,6 +1,7 @@
# Code changes

* Retrieve statements should validate their paths the same way that path statements do.
* Require Ruby v1.9.3 and fix those bits that work around earlier versions. (Run "`ack -Q 1.9.3`".)
* Produce nice error messages when packages contain file names that Windows can't deal with.
* Periodically `ack '\bTODO:'` and fix what we can.

Expand Down
161 changes: 106 additions & 55 deletions lib/fig/repository_package_publisher.rb
View file
Expand Up @@ -56,14 +56,16 @@ def package_statements=(statements)

def publish_package()
derive_publish_metadata()
validate_asset_names()

temp_dir = publish_temp_dir()
@operating_system.delete_and_recreate_directory(temp_dir)
@operating_system.delete_and_recreate_directory(@local_directory_for_package)
@operating_system.delete_and_recreate_directory(
@local_directory_for_package
)

fig_file = File.join(temp_dir, Fig::Repository::PACKAGE_FILE_IN_REPO)
content, published_package = derive_definition_file
content, published_package =
derive_definition_file_and_create_resource_archive
@operating_system.write(fig_file, content)

publish_package_contents
Expand Down Expand Up @@ -101,42 +103,19 @@ def derive_login()
return Etc.getlogin()
end

def validate_asset_names()
asset_statements = @text_assembler.asset_input_statements

asset_names = Set.new()
asset_statements.each do
|statement|

asset_name = statement.asset_name()
if not asset_name.nil?
if asset_name == Fig::Repository::RESOURCES_FILE
Fig::Logging.fatal \
%Q<You cannot have an asset with the name "#{Fig::Repository::RESOURCES_FILE}"#{statement.position_string()} due to Fig implementation details.>
end

if asset_names.include?(asset_name)
Fig::Logging.fatal \
%Q<Found multiple archives with the name "#{asset_name}"#{statement.position_string()}. If these were allowed, archives would overwrite each other.>
raise Fig::RepositoryError.new
else
asset_names.add(asset_name)
end
end
end
end

def derive_definition_file()
def derive_definition_file_and_create_resource_archive()
add_package_metadata_comments()
add_output_statements_and_create_resource_archive()
assemble_output_statements_and_accumulate_assets()
validate_asset_names()
create_resource_archive()
add_unparsed_text()

file_content, explanations = @text_assembler.assemble_package_definition()
if Fig::Logging.info?
explanations.each {|explanation| Fig::Logging.info explanation}
end

published_package = nil
to_be_published_package = nil
begin
unparsed_package = Fig::NotYetParsedPackage.new
unparsed_package.descriptor = @descriptor
Expand All @@ -146,15 +125,15 @@ def derive_definition_file()
unparsed_package.source_description = '<package to be published>'
unparsed_package.unparsed_text = file_content

published_package =
to_be_published_package =
Fig::Parser.new(nil, false).parse_package(unparsed_package)
rescue Fig::PackageParseError => error
raise \
"Bug in code! Could not parse package definition to be published.\n" +
"#{error}\n\nGenerated contents:\n#{file_content}"
end

return file_content, published_package
return file_content, to_be_published_package
end

def add_package_metadata_comments()
Expand Down Expand Up @@ -353,19 +332,9 @@ def get_git_sha1()
return reference
end

# Deals with Archive and Resource statements. It downloads any remote
# files (those where the statement references a URL as opposed to a local
# file) and then copies all files into the local repository and the remote
# repository (if not a local-only publish).
def add_output_statements_and_create_resource_archive()
assemble_output_statements()
create_resource_archive()

return
end

def assemble_output_statements()
@resource_paths = []
def assemble_output_statements_and_accumulate_assets()
@local_resource_paths = []
@asset_names_with_associated_input_statements = {}

@text_assembler.input_statements.each do
|statement|
Expand All @@ -382,39 +351,121 @@ def assemble_output_statements()

def add_asset_to_output_statements(asset_statement)
if Fig::URL.is_url? asset_statement.location
@text_assembler.add_output asset_statement
add_output_asset_statement_based_upon_input_statement(
asset_statement, asset_statement
)
elsif asset_statement.is_a? Fig::Statement::Archive
if asset_statement.requires_globbing?
expand_globs_from( [asset_statement.location] ).each do
|file|

@text_assembler.add_output(
add_output_asset_statement_based_upon_input_statement(
Fig::Statement::Archive.new(
nil,
%Q<[synthetic statement created in #{__FILE__} line #{__LINE__}]>,
file,
false # No globbing
)
),
asset_statement
)
end
else
@text_assembler.add_output asset_statement
add_output_asset_statement_based_upon_input_statement(
asset_statement, asset_statement
)
end
elsif asset_statement.requires_globbing?
@resource_paths.concat expand_globs_from( [asset_statement.location] )
@local_resource_paths.concat expand_globs_from(
[asset_statement.location]
)
else
@resource_paths << asset_statement.location
@local_resource_paths << asset_statement.location
end

return
end

def add_output_asset_statement_based_upon_input_statement(
output_statement, input_statement
)
@text_assembler.add_output output_statement

asset_name = output_statement.asset_name

input_statements =
@asset_names_with_associated_input_statements[asset_name]
if input_statements.nil?
input_statements = []

@asset_names_with_associated_input_statements[asset_name] =
input_statements
end

input_statements << input_statement

return
end

def validate_asset_names()
found_problem = false

@asset_names_with_associated_input_statements.keys.sort.each do
|asset_name|

statements = @asset_names_with_associated_input_statements[asset_name]

if asset_name == Fig::Repository::RESOURCES_FILE
Fig::Logging.fatal \
%Q<You cannot have an asset with the name "#{Fig::Repository::RESOURCES_FILE}"#{collective_position_string(statements)} due to Fig implementation details.>
found_problem = true
end

archive_statements =
statements.select { |s| s.is_a? Fig::Statement::Archive }
if (
not archive_statements.empty? \
and not (
asset_name =~ /\.tar\.gz$/ \
or asset_name =~ /\.tgz$/ \
or asset_name =~ /\.tar\.bz2$/ \
or asset_name =~ /\.zip$/
)
)
Fig::Logging.fatal \
%Q<Unknown archive type "#{asset_name}"#{collective_position_string(archive_statements)}.>
found_problem = true
end

if statements.size > 1
Fig::Logging.fatal \
%Q<Found multiple assets with the name "#{asset_name}"#{collective_position_string(statements)}. If these were allowed, assets would overwrite each other.>
found_problem = true
end
end

if found_problem
raise Fig::RepositoryError.new
end
end

def collective_position_string(statements)
return (
statements.map {
|statement|

position_string = statement.position_string

position_string.empty? ? '<unknown>' : position_string.strip
}
).join(', ')
end

def create_resource_archive()
if @resource_paths.size > 0
check_asset_paths(@resource_paths)
if @local_resource_paths.size > 0
check_asset_paths(@local_resource_paths)

file = File.join publish_temp_dir, Fig::Repository::RESOURCES_FILE
@operating_system.create_archive(file, @resource_paths)
@operating_system.create_archive(file, @local_resource_paths)
Fig::AtExit.add { FileUtils.rm_f(file) }

@text_assembler.add_output(
Expand Down
12 changes: 6 additions & 6 deletions spec/command/grammar_asset_spec.rb
View file
Expand Up @@ -11,7 +11,7 @@
def test_published_asset_with_url_with_symbol(
asset_type, symbol, quote, version
)
file_name = "with#{symbol}symbol"
file_name = "with#{symbol}symbol.zip"
escaped_file = CGI.escape file_name
quoted_url =
"#{quote}file://#{CURRENT_DIRECTORY}/#{escaped_file}#{quote}"
Expand Down Expand Up @@ -85,7 +85,7 @@ def test_published_file_asset_with_url_with_symbol(
def test_published_asset_with_file_with_symbol(
asset_type, symbol, quote, version
)
file_name = "with#{symbol}symbol"
file_name = "with#{symbol}symbol.zip"
quoted_name = "#{quote}#{file_name}#{quote}"

it %Q<with file «#{quoted_name}»> do
Expand Down Expand Up @@ -176,10 +176,10 @@ def testable_v0_special_characters()
|quote|

begin
value = "#{quote}nothing-special#{quote}"
value = "#{quote}nothing-special.zip#{quote}"

it %Q<with file «#{value}»> do
write_file "#{CURRENT_DIRECTORY}/nothing-special", ''
write_file "#{CURRENT_DIRECTORY}/nothing-special.zip", ''

fig(
[%w< --publish foo/1.2.3 --set x=y >, "--#{asset_type}", value],
Expand Down Expand Up @@ -259,10 +259,10 @@ def testable_v0_special_characters()
|quote|

begin
value = "#{quote}nothing-special#{quote}"
value = "#{quote}nothing-special.zip#{quote}"

it %Q<with file «#{value}»> do
write_file "#{CURRENT_DIRECTORY}/nothing-special", ''
write_file "#{CURRENT_DIRECTORY}/nothing-special.zip", ''

input = <<-"END"
grammar v1
Expand Down
34 changes: 24 additions & 10 deletions spec/command/publishing_spec.rb
View file
Expand Up @@ -25,17 +25,17 @@
out, err, exit_code =
fig(%w<--publish foo/1.2.3>, input, :no_raise_on_error => true)

err.should =~ /multiple archives/
err.should =~ /multiple assets/
err.should =~ /duplicate-archive\.tar\.gz/
exit_code.should_not == 0
end

it 'complains when archives are named the same as the resources tarball' do
%w< archive resource >.each do
|statement_type|
%w< archive resource >.each do
|asset_type|

it "complains when #{asset_type}s refer to a URL with a base name the same as the resources tarball" do
input = <<-END
#{statement_type} http://some-host/#{Fig::Repository::RESOURCES_FILE}
#{asset_type} http://some-host/#{Fig::Repository::RESOURCES_FILE}
config default end
END

Expand All @@ -46,18 +46,14 @@
/cannot have an asset with the name "#{Regexp.escape(Fig::Repository::RESOURCES_FILE)}"/
exit_code.should_not == 0
end
end

%w< archive resource >.each do
|asset_type|

it "complains when #{asset_type}s refer to non-existent local paths" do
out, err, exit_code =
fig(
[
%w<--publish foo/1.2.3>,
"--#{asset_type}",
'does not exist',
'does not exist.zip',
%w<--set VARIABLE=VALUE>
].flatten,
:no_raise_on_error => true
Expand All @@ -69,6 +65,24 @@
end
end

it "complains when globbing for archives picks up a file with the same as the resources tarball" do
write_file(
"#{CURRENT_DIRECTORY}/#{Fig::Repository::RESOURCES_FILE}", ''
)

input = <<-END
archive *
config default end
END

out, err, exit_code =
fig(%w<--publish foo/1.2.3>, input, :no_raise_on_error => true)

err.should =~
/cannot have an asset with the name "#{Regexp.escape(Fig::Repository::RESOURCES_FILE)}"/
exit_code.should_not == 0
end

it 'publishes to remote repository' do
input = <<-END
config default
Expand Down

0 comments on commit 9b0041b

Please sign in to comment.