rubyXL icon indicating copy to clipboard operation
rubyXL copied to clipboard

`#parse_buffer` unexpectedly modifies input buffer

Open HosokawaR opened this issue 1 year ago • 5 comments

Thank you for creating such a wonderful library.

I noticed that RubyXL::Parser.parse_buffer implicitly changes the buffer received as an argument. But considering the name #parse_buffer, I don't think the passed argument buffer should be modifed.

Below is the minimal example. buffer has been modified and increased in size.

$ gem list | grep rubyXL
rubyXL (3.4.26)

$ irb
irb(main):001> require 'rubyXL'
irb(main):002> buffer = File.read("./sample.xlsx")
irb(main):003> buffer.size
=> 213663
irb(main):004> RubyXL::Parser.parse_buffer(buffer)
irb(main):005> buffer.size
=> 272845  # buffer size changed !

The cause was that when Zip::File.open_buffer was called with block, the supplied argument buffer was changed. https://github.com/rubyzip/rubyzip/blob/73c8e110ed1dbcff08ffa48bb1b094abd0348502/lib/zip/file.rb#L122

I think we can fix this problem by not using block.

→ I also created a PR. https://github.com/weshatheleopard/rubyXL/pull/454

It's a minor problem, but I get it sometimes, so I think it would be nice if it could be fixed. In my case, I discovered this problem because when I attached the parsed Excel file to an email, the Excel file was corrupted.

HosokawaR avatar Apr 07 '24 09:04 HosokawaR

You see, I'm temped to NOT do anything about this issue in RubyXL since all that it does is passing the buffer to RubyZip; from that point, it's RubyZip responsebility: if it modifies buffer, it's it's fault, not RubyXLs. I do not appreciate this behavior, as there's no reason why it should ever modify the buffer, but that's what they do. I figured out that when I pass a String to RubyZip, then .freeze'ing it before passing it over does the trick. I don't have a problem adding a warning about that workaround to the documentation. But I think RubyZip's issues need to be handled with its developers. See the discussion here.

weshatheleopard avatar Apr 07 '24 19:04 weshatheleopard

I confirmed that this issue is resolved with rubyzip 2.4.rc1. https://github.com/rubyzip/rubyzip/issues/280#issuecomment-2043040338

I hope to close this issue with an rubyzip official release and an upgrade of rubyzip's dependencies.

HosokawaR avatar Apr 09 '24 01:04 HosokawaR

Yes, I tried to update gems but rubyzip is not showing up as updated. I assume the reason is that it's RC and not released yet. Once it is, I will update dependencies.

weshatheleopard avatar Apr 09 '24 16:04 weshatheleopard

It has been nearly 5 months but the latest version on Rubygems is still 2.3.2?

weshatheleopard avatar Sep 04 '24 02:09 weshatheleopard

It looks like the problem still exists with rubyzip 2.4.1 and we cannot upgrade to 3.x due to #473

weshatheleopard avatar Oct 31 '25 05:10 weshatheleopard