Skip to content

Commit

Permalink
Zip::File no longer subclasses Zip::CentralDirectory.
Browse files Browse the repository at this point in the history 
It has bothered me for years that the central directory is exposed in
this way. A zip file should *have* a central directory, but it should
not *be* one.

This commit starts us down the path of properly separating the two.
  • Loading branch information
@hainesr
hainesr committed Jan 15, 2022
1 parent f8b9d07 commit 34731b1
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 33 deletions.
6 changes: 3 additions & 3 deletions lib/zip/central_directory.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@

module Zip
class CentralDirectory
include Enumerable

END_OF_CDS = 0x06054b50
ZIP64_END_OF_CDS = 0x06064b50
ZIP64_EOCD_LOCATOR = 0x07064b50
Expand All @@ -14,7 +12,9 @@ class CentralDirectory
MAX_FILE_COMMENT_SIZE = 1 << 16
MAX_END_OF_CDS_SIZE = MAX_FILE_COMMENT_SIZE + STATIC_EOCD_SIZE

attr_reader :comment
attr_accessor :comment

attr_reader :entry_set

# Returns an Enumerable containing the entries.
def entries
Expand Down
60 changes: 30 additions & 30 deletions lib/zip/file.rb
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
# frozen_string_literal: true

require 'forwardable'

require_relative 'file_split'

module Zip
Expand Down Expand Up @@ -45,8 +47,8 @@ module Zip
#
# ZipFileSystem offers an alternative API that emulates ruby's
# interface for accessing the filesystem, ie. the File and Dir classes.

class File < CentralDirectory
class File
extend Forwardable
extend FileSplit

IO_METHODS = [:tell, :seek, :read, :eof, :close].freeze
Expand All @@ -62,8 +64,7 @@ class File < CentralDirectory
# default -> true.
attr_accessor :restore_times

# Returns the zip files comment, if it has one
attr_accessor :comment
def_delegators :@cdir, :comment, :comment=, :each, :entries, :size

# Opens a zip archive. Pass create: true to create
# a new archive if it doesn't exist already.
Expand All @@ -73,38 +74,37 @@ def initialize(path_or_io, create: false, buffer: false, **options)
.merge(compression_level: ::Zip.default_compression)
.merge(options)
@name = path_or_io.respond_to?(:path) ? path_or_io.path : path_or_io
@comment = ''
@create = create ? true : false # allow any truthy value to mean true
@cdir = ::Zip::CentralDirectory.new

if ::File.size?(@name.to_s)
# There is a file, which exists, that is associated with this zip.
@create = false
@file_permissions = ::File.stat(@name).mode

if buffer
read_from_stream(path_or_io)
@cdir.read_from_stream(path_or_io)
else
::File.open(@name, 'rb') do |f|
read_from_stream(f)
@cdir.read_from_stream(f)
end
end
elsif buffer && path_or_io.size > 0
# This zip is probably a non-empty StringIO.
@create = false
read_from_stream(path_or_io)
elsif @create
# This zip is completely new/empty and is to be created.
@entry_set = EntrySet.new
elsif ::File.zero?(@name)
# A file exists, but it is empty.
@cdir.read_from_stream(path_or_io)
elsif !@create && ::File.zero?(@name)
# A file exists, but it is empty, and we've said we're
# NOT creating a new zip.
raise Error, "File #{@name} has zero size. Did you mean to pass the create flag?"
else
# Everything is wrong.
elsif !@create
# If we get here, and we're not creating a new zip, then
# everything is wrong.
raise Error, "File #{@name} not found"
end

@stored_entries = @entry_set.dup
@stored_comment = @comment
@stored_entries = @cdir.entry_set.dup
@stored_comment = @cdir.comment
@restore_ownership = options[:restore_ownership]
@restore_permissions = options[:restore_permissions]
@restore_times = options[:restore_times]
Expand Down Expand Up @@ -222,7 +222,7 @@ def get_output_stream(entry, permissions: nil, comment: nil,
end
new_entry.unix_perms = permissions
zip_streamable_entry = StreamableStream.new(new_entry)
@entry_set << zip_streamable_entry
@cdir.entry_set << zip_streamable_entry
zip_streamable_entry.get_output_stream(&a_proc)
end

Expand Down Expand Up @@ -250,7 +250,7 @@ def add(entry, src_path, &continue_on_exists_proc)
end
new_entry.gather_fileinfo_from_srcpath(src_path)
new_entry.dirty = true
@entry_set << new_entry
@cdir.entry_set << new_entry
end

# Convenience method for adding the contents of a file to the archive
Expand All @@ -264,16 +264,16 @@ def add_stored(entry, src_path, &continue_on_exists_proc)

# Removes the specified entry.
def remove(entry)
@entry_set.delete(get_entry(entry))
@cdir.entry_set.delete(get_entry(entry))
end

# Renames the specified entry.
def rename(entry, new_name, &continue_on_exists_proc)
found_entry = get_entry(entry)
check_entry_exists(new_name, continue_on_exists_proc, 'rename')
@entry_set.delete(found_entry)
@cdir.entry_set.delete(found_entry)
found_entry.name = new_name
@entry_set << found_entry
@cdir.entry_set << found_entry
end

# Replaces the specified entry with the contents of src_path (from
Expand All @@ -298,7 +298,7 @@ def commit

on_success_replace do |tmp_file|
::Zip::OutputStream.open(tmp_file) do |zos|
@entry_set.each do |e|
@cdir.each do |e|
e.write_to_zip_output_stream(zos)
e.dirty = false
e.clean_up
Expand All @@ -315,7 +315,7 @@ def write_buffer(io = ::StringIO.new)
return unless commit_required?

::Zip::OutputStream.write_buffer(io) do |zos|
@entry_set.each { |e| e.write_to_zip_output_stream(zos) }
@cdir.each { |e| e.write_to_zip_output_stream(zos) }
zos.comment = comment
end
end
Expand All @@ -328,16 +328,16 @@ def close
# Returns true if any changes has been made to this archive since
# the previous commit
def commit_required?
@entry_set.each do |e|
@cdir.each do |e|
return true if e.dirty
end
@comment != @stored_comment || @entry_set != @stored_entries || @create
comment != @stored_comment || @cdir.entry_set != @stored_entries || @create
end

# Searches for entry with the specified name. Returns nil if
# no entry is found. See also get_entry
def find_entry(entry_name)
selected_entry = @entry_set.find_entry(entry_name)
selected_entry = @cdir.entry_set.find_entry(entry_name)
return if selected_entry.nil?

selected_entry.restore_ownership = @restore_ownership
Expand All @@ -352,7 +352,7 @@ def find_entry(entry_name)
# `::File::FNM_PATHNAME | ::File::FNM_DOTMATCH | ::File::FNM_EXTGLOB`,
# which will be overridden if you set your own flags.
def glob(*args, &block)
@entry_set.glob(*args, &block)
@cdir.entry_set.glob(*args, &block)
end

# Searches for an entry just as find_entry, but throws Errno::ENOENT
Expand All @@ -370,7 +370,7 @@ def mkdir(entry_name, permission = 0o755)

entry_name = entry_name.dup.to_s
entry_name << '/' unless entry_name.end_with?('/')
@entry_set << ::Zip::StreamableDirectory.new(@name, entry_name, nil, permission)
@cdir.entry_set << ::Zip::StreamableDirectory.new(@name, entry_name, nil, permission)
end

private
Expand All @@ -389,7 +389,7 @@ def directory?(new_entry, src_path)

def check_entry_exists(entry_name, continue_on_exists_proc, proc_name)
continue_on_exists_proc ||= proc { Zip.continue_on_exists_proc }
return unless @entry_set.include?(entry_name)
return unless @cdir.entry_set.include?(entry_name)

if continue_on_exists_proc.call
remove get_entry(entry_name)
Expand Down

0 comments on commit 34731b1

Please sign in to comment.