Skip to content

Commit

Permalink
Ensure compatibility with --enable-frozen-string-literal
Browse files Browse the repository at this point in the history
Ref: https://bugs.ruby-lang.org/issues/20205

In Ruby 3.4 string literals will be "chilled" by default. Meaning
they are still mutable, but will pretend to be frozen.

In most case it has no impact, just emit a few warnings there and
there, but there is one thing it impacts is the `StringIO.new('')`
pattern. `StringIO` checks if the given string is frozen, and if
it is will act as a read only IO.

This breaks rubyzip 2.x.

This commit make the 2.x branch compatible with frozen string literals.
  • Loading branch information
byroot authored and hainesr committed Apr 8, 2024
1 parent 46689d7 commit 385ebd0
Show file tree
Hide file tree
Showing 17 changed files with 62 additions and 38 deletions.
28 changes: 26 additions & 2 deletions .github/workflows/tests.yml
Expand Up @@ -8,7 +8,7 @@ jobs:
fail-fast: false
matrix:
os: [ubuntu]
ruby: ['2.4', '2.5', '2.6', '2.7', '3.0', '3.1', jruby, truffleruby]
ruby: ['2.4', '2.5', '2.6', '2.7', '3.0', '3.1', '3.2', '3.3', jruby, truffleruby]
include:
- os: macos
ruby: '2.4'
Expand All @@ -31,12 +31,36 @@ jobs:
FULL_ZIP64_TEST: 1
run: bundle exec rake

test-frozen-string-literal:
strategy:
fail-fast: false
matrix:
os: [ubuntu]
ruby: ['3.3']
runs-on: ${{ matrix.os }}-latest
continue-on-error: true
steps:
- name: Checkout rubyzip code
uses: actions/checkout@v2

- name: Install and set up ruby
uses: ruby/setup-ruby@v1
with:
ruby-version: ${{ matrix.ruby }}
bundler-cache: true

- name: Run the tests
env:
RUBYOPT: --enable-frozen-string-literal
FULL_ZIP64_TEST: 1
run: bundle exec rake

test-yjit:
strategy:
fail-fast: false
matrix:
os: [ubuntu, macos]
ruby: ['3.1', head]
ruby: ['3.1', '3.2', '3.3', head]
runs-on: ${{ matrix.os }}-latest
continue-on-error: true
steps:
Expand Down
2 changes: 1 addition & 1 deletion README.md
Expand Up @@ -188,7 +188,7 @@ If `::Zip::InputStream` finds such entry in the zip archive it will raise an exc
Rubyzip supports reading/writing zip files with traditional zip encryption (a.k.a. "ZipCrypto"). AES encryption is not yet supported. It can be used with buffer streams, e.g.:

```ruby
Zip::OutputStream.write_buffer(::StringIO.new(''), Zip::TraditionalEncrypter.new('password')) do |out|
Zip::OutputStream.write_buffer(::StringIO.new, Zip::TraditionalEncrypter.new('password')) do |out|
out.put_next_entry("my_file.txt")
out.write my_data
end.string
Expand Down
2 changes: 1 addition & 1 deletion lib/zip/extra_field/zip64.rb
Expand Up @@ -59,7 +59,7 @@ def pack_for_local

def pack_for_c_dir
# central directory entries contain only fields that didn't fit in the main entry part
packed = ''.force_encoding('BINARY')
packed = ''.b
packed << [@original_size].pack('Q<') if @original_size
packed << [@compressed_size].pack('Q<') if @compressed_size
packed << [@relative_header_offset].pack('Q<') if @relative_header_offset
Expand Down
4 changes: 2 additions & 2 deletions lib/zip/file.rb
Expand Up @@ -140,7 +140,7 @@ def open(file_name, dep_create = false, create: false, **options)
def add_buffer
Zip.warn_about_v3_api('Zip::File.add_buffer')

io = ::StringIO.new('')
io = ::StringIO.new
zf = ::Zip::File.new(io, true, true)
yield zf
zf.write_buffer(io)
Expand Down Expand Up @@ -411,7 +411,7 @@ def commit
end

# Write buffer write changes to buffer and return
def write_buffer(io = ::StringIO.new(''))
def write_buffer(io = ::StringIO.new)
return unless commit_required?

::Zip::OutputStream.write_buffer(io) do |zos|
Expand Down
4 changes: 2 additions & 2 deletions lib/zip/filesystem.rb
Expand Up @@ -239,7 +239,7 @@ def directory?(filename)
end

def open(filename, mode = 'r', permissions = 0o644, &block)
mode.delete!('b') # ignore b option
mode = mode.delete('b') # ignore b option
case mode
when 'r'
@mapped_zip.get_input_stream(filename, &block)
Expand Down Expand Up @@ -619,7 +619,7 @@ def each
end

def expand_path(path)
expanded = path.start_with?('/') ? path : ::File.join(@pwd, path)
expanded = path.start_with?('/') ? path.dup : ::File.join(@pwd, path)
expanded.gsub!(/\/\.(\/|$)/, '')
expanded.gsub!(/[^\/]+\/\.\.(\/|$)/, '')
expanded.empty? ? '/' : expanded
Expand Down
4 changes: 2 additions & 2 deletions lib/zip/inflater.rb
Expand Up @@ -3,11 +3,11 @@ class Inflater < Decompressor #:nodoc:all
def initialize(*args)
super

@buffer = +''
@buffer = ''.b
@zlib_inflater = ::Zlib::Inflate.new(-Zlib::MAX_WBITS)
end

def read(length = nil, outbuf = '')
def read(length = nil, outbuf = ''.b)
return (length.nil? || length.zero? ? '' : nil) if eof

while length.nil? || (@buffer.bytesize < length)
Expand Down
4 changes: 2 additions & 2 deletions lib/zip/ioextras.rb
Expand Up @@ -6,14 +6,14 @@ module IOExtras #:nodoc:

class << self
def copy_stream(ostream, istream)
ostream.write(istream.read(CHUNK_SIZE, '')) until istream.eof?
ostream.write(istream.read(CHUNK_SIZE, ''.b)) until istream.eof?
end

def copy_stream_n(ostream, istream, nbytes)
toread = nbytes
while toread > 0 && !istream.eof?
tr = toread > CHUNK_SIZE ? CHUNK_SIZE : toread
ostream.write(istream.read(tr, ''))
ostream.write(istream.read(tr, ''.b))
toread -= tr
end
end
Expand Down
8 changes: 4 additions & 4 deletions lib/zip/ioextras/abstract_input_stream.rb
Expand Up @@ -11,13 +11,13 @@ def initialize
super
@lineno = 0
@pos = 0
@output_buffer = ''
@output_buffer = ''.b
end

attr_accessor :lineno
attr_reader :pos

def read(number_of_bytes = nil, buf = '')
def read(number_of_bytes = nil, buf = ''.b)
tbuf = if @output_buffer.bytesize > 0
if number_of_bytes <= @output_buffer.bytesize
@output_buffer.slice!(0, number_of_bytes)
Expand All @@ -26,7 +26,7 @@ def read(number_of_bytes = nil, buf = '')
rbuf = sysread(number_of_bytes, buf)
out = @output_buffer
out << rbuf if rbuf
@output_buffer = ''
@output_buffer = ''.b
out
end
else
Expand Down Expand Up @@ -93,7 +93,7 @@ def ungetc(byte)

def flush
ret_val = @output_buffer
@output_buffer = ''
@output_buffer = ''.b
ret_val
end

Expand Down
2 changes: 1 addition & 1 deletion lib/zip/output_stream.rb
Expand Up @@ -62,7 +62,7 @@ def open(file_name, dep_encrypter = nil, encrypter: nil)
end

# Same as #open but writes to a filestream instead
def write_buffer(io = ::StringIO.new(''), dep_encrypter = nil, encrypter: nil)
def write_buffer(io = ::StringIO.new, dep_encrypter = nil, encrypter: nil)
Zip.warn_about_v3_api('Zip::OutputStream.write_buffer') unless dep_encrypter.nil?

io.binmode if io.respond_to?(:binmode)
Expand Down
2 changes: 1 addition & 1 deletion lib/zip/pass_thru_decompressor.rb
Expand Up @@ -5,7 +5,7 @@ def initialize(*args)
@read_so_far = 0
end

def read(length = nil, outbuf = '')
def read(length = nil, outbuf = ''.b)
return (length.nil? || length.zero? ? '' : nil) if eof

if length.nil? || (@read_so_far + length) > decompressed_size
Expand Down
2 changes: 1 addition & 1 deletion test/encryption_test.rb
Expand Up @@ -19,7 +19,7 @@ def test_encrypt
@rand = [250, 143, 107, 13, 143, 22, 155, 75, 228, 150, 12]
@output = ::Zip::DOSTime.stub(:now, ::Zip::DOSTime.new(2014, 12, 17, 15, 56, 24)) do
Random.stub(:rand, ->(_range) { @rand.shift }) do
Zip::OutputStream.write_buffer(::StringIO.new(''), Zip::TraditionalEncrypter.new('password')) do |zos|
Zip::OutputStream.write_buffer(::StringIO.new, Zip::TraditionalEncrypter.new('password')) do |zos|
zos.put_next_entry('file1.txt')
zos.write ::File.open(INPUT_FILE1).read
end.string
Expand Down
2 changes: 1 addition & 1 deletion test/file_test.rb
Expand Up @@ -503,7 +503,7 @@ def test_write_buffer
zf = ::Zip::File.new(TEST_ZIP.zip_name)
old_name = zf.entries.first
zf.rename(old_name, new_name)
io = ::StringIO.new('')
io = ::StringIO.new
buffer = zf.write_buffer(io)
File.open(TEST_ZIP.zip_name, 'wb') { |f| f.write buffer.string }
zf_read = ::Zip::File.new(TEST_ZIP.zip_name)
Expand Down
4 changes: 2 additions & 2 deletions test/input_stream_test.rb
Expand Up @@ -136,7 +136,7 @@ def test_rewind
assert_equal(TestZipFile::TEST_ZIP2.entry_names[0], e.name)

# Do a little reading
buf = ''
buf = ''.b
buf << zis.read(100)
assert_equal(100, zis.pos)
buf << (zis.gets || '')
Expand All @@ -145,7 +145,7 @@ def test_rewind

zis.rewind

buf2 = ''
buf2 = ''.b
buf2 << zis.read(100)
buf2 << (zis.gets || '')
buf2 << (zis.gets || '')
Expand Down
14 changes: 7 additions & 7 deletions test/ioextras/abstract_output_stream_test.rb
Expand Up @@ -8,7 +8,7 @@ class TestOutputStream
attr_accessor :buffer

def initialize
@buffer = ''
@buffer = ''.b
end

def <<(data)
Expand Down Expand Up @@ -58,12 +58,12 @@ def test_print
assert_equal("hello world. You ok out there?\nI sure hope so!\n", @output_stream.buffer)

$, = 'X'
@output_stream.buffer = ''
@output_stream.buffer = ''.b
@output_stream.print('monkey', 'duck', 'zebra')
assert_equal("monkeyXduckXzebra\n", @output_stream.buffer)

$\ = nil
@output_stream.buffer = ''
@output_stream.buffer = ''.b
@output_stream.print(20)
assert_equal('20', @output_stream.buffer)
end
Expand All @@ -87,19 +87,19 @@ def test_puts
@output_stream.puts('hello', 'world')
assert_equal("\nhello\nworld\n", @output_stream.buffer)

@output_stream.buffer = ''
@output_stream.buffer = ''.b
@output_stream.puts("hello\n", "world\n")
assert_equal("hello\nworld\n", @output_stream.buffer)

@output_stream.buffer = ''
@output_stream.buffer = ''.b
@output_stream.puts(%W[hello\n world\n])
assert_equal("hello\nworld\n", @output_stream.buffer)

@output_stream.buffer = ''
@output_stream.buffer = ''.b
@output_stream.puts(%W[hello\n world\n], 'bingo')
assert_equal("hello\nworld\nbingo\n", @output_stream.buffer)

@output_stream.buffer = ''
@output_stream.buffer = ''.b
@output_stream.puts(16, 20, 50, 'hello')
assert_equal("16\n20\n50\nhello\n", @output_stream.buffer)
end
Expand Down
4 changes: 2 additions & 2 deletions test/output_stream_test.rb
Expand Up @@ -23,7 +23,7 @@ def test_open
end

def test_write_buffer
io = ::StringIO.new('')
io = ::StringIO.new
buffer = ::Zip::OutputStream.write_buffer(io) do |zos|
zos.comment = TEST_ZIP.comment
write_test_zip(zos)
Expand All @@ -33,7 +33,7 @@ def test_write_buffer
end

def test_write_buffer_binmode
io = ::StringIO.new('')
io = ::StringIO.new
buffer = ::Zip::OutputStream.write_buffer(io) do |zos|
zos.comment = TEST_ZIP.comment
write_test_zip(zos)
Expand Down
2 changes: 1 addition & 1 deletion test/test_helper.rb
Expand Up @@ -145,7 +145,7 @@ class TestOutputStream
attr_accessor :buffer

def initialize
@buffer = ''
@buffer = ''.b
end

def <<(data)
Expand Down
12 changes: 6 additions & 6 deletions test/version_3_api_test.rb
Expand Up @@ -177,7 +177,7 @@ def test_open

def test_open_buffer
assert_v3_api_warning do
::Zip::InputStream.open_buffer(StringIO.new(''))
::Zip::InputStream.open_buffer(StringIO.new)
end
end
end
Expand All @@ -194,11 +194,11 @@ def test_new
end

refute_v3_api_warning do
::Zip::OutputStream.new(StringIO.new(''), stream: true)
::Zip::OutputStream.new(StringIO.new, stream: true)
end

assert_v3_api_warning do
::Zip::OutputStream.new(StringIO.new(''), true)
::Zip::OutputStream.new(StringIO.new, true)
end

refute_v3_api_warning do
Expand Down Expand Up @@ -226,15 +226,15 @@ def test_open

def test_write_buffer
refute_v3_api_warning do
::Zip::OutputStream.write_buffer(StringIO.new('')) {}
::Zip::OutputStream.write_buffer(StringIO.new) {}
end

refute_v3_api_warning do
::Zip::OutputStream.write_buffer(StringIO.new(''), encrypter: true) {}
::Zip::OutputStream.write_buffer(StringIO.new, encrypter: true) {}
end

assert_v3_api_warning do
::Zip::OutputStream.write_buffer(StringIO.new(''), true) {}
::Zip::OutputStream.write_buffer(StringIO.new, true) {}
end
end
end
Expand Down

0 comments on commit 385ebd0

Please sign in to comment.