Handle MissingEOCD as invalid zip file, not job failure (#2572)
ZipKit::FileReader raises MissingEOCD for truncated, corrupted, or non-zip files. This exception is a direct StandardError subclass, not a subclass of InvalidStructure or ReadError, so it escaped as an unhandled job failure instead of being caught and surfaced to the user. Broaden the rescue in ZipFile::Reader#initialize to catch ReadError (parent of InvalidStructure), MissingEOCD, and UnsupportedFeature.
This commit is contained in:
@@ -2,7 +2,7 @@ class ZipFile::Reader
|
|||||||
def initialize(io)
|
def initialize(io)
|
||||||
@io = io
|
@io = io
|
||||||
@reader = ZipKit::FileReader.read_zip_structure(io: io)
|
@reader = ZipKit::FileReader.read_zip_structure(io: io)
|
||||||
rescue ZipKit::FileReader::InvalidStructure => e
|
rescue ZipKit::FileReader::ReadError, ZipKit::FileReader::MissingEOCD, ZipKit::FileReader::UnsupportedFeature => e
|
||||||
raise ZipFile::InvalidFileError, e.message
|
raise ZipFile::InvalidFileError, e.message
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|||||||
@@ -96,6 +96,27 @@ class Account::ImportTest < ActiveSupport::TestCase
|
|||||||
tempfile&.unlink
|
tempfile&.unlink
|
||||||
end
|
end
|
||||||
|
|
||||||
|
test "check sets failure_reason to invalid_export for non-ZIP file" do
|
||||||
|
target_account = Account.create!(name: "Import Test")
|
||||||
|
import = Account::Import.create!(identity: identities(:david), account: target_account)
|
||||||
|
|
||||||
|
tempfile = Tempfile.new([ "not_a_zip", ".zip" ])
|
||||||
|
tempfile.write("this is not a zip file at all")
|
||||||
|
tempfile.rewind
|
||||||
|
|
||||||
|
Current.set(account: target_account) do
|
||||||
|
import.file.attach(io: tempfile, filename: "export.zip", content_type: "application/zip")
|
||||||
|
end
|
||||||
|
|
||||||
|
assert_raises(ZipFile::InvalidFileError) { import.check }
|
||||||
|
|
||||||
|
assert import.failed?
|
||||||
|
assert_equal "invalid_export", import.failure_reason
|
||||||
|
ensure
|
||||||
|
tempfile&.close
|
||||||
|
tempfile&.unlink
|
||||||
|
end
|
||||||
|
|
||||||
test "check sets failure_reason to conflict when records already exist" do
|
test "check sets failure_reason to conflict when records already exist" do
|
||||||
source_account = accounts("37s")
|
source_account = accounts("37s")
|
||||||
exporter = users(:david)
|
exporter = users(:david)
|
||||||
|
|||||||
@@ -119,6 +119,17 @@ class ZipFileTest < ActiveSupport::TestCase
|
|||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
test "reader raises InvalidFileError for non-zip file" do
|
||||||
|
tempfile = Tempfile.new([ "not_a_zip", ".zip" ])
|
||||||
|
tempfile.write("this is not a zip file at all")
|
||||||
|
tempfile.rewind
|
||||||
|
|
||||||
|
assert_raises(ZipFile::InvalidFileError) { ZipFile::Reader.new(tempfile) }
|
||||||
|
ensure
|
||||||
|
tempfile&.close
|
||||||
|
tempfile&.unlink
|
||||||
|
end
|
||||||
|
|
||||||
private
|
private
|
||||||
def create_test_zip(files)
|
def create_test_zip(files)
|
||||||
tempfile = Tempfile.new([ "test", ".zip" ])
|
tempfile = Tempfile.new([ "test", ".zip" ])
|
||||||
|
|||||||
Reference in New Issue
Block a user