Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#parse_buffer unexpectedly modifies input buffer #453

Open
HosokawaR opened this issue Apr 7, 2024 · 4 comments
Open

#parse_buffer unexpectedly modifies input buffer #453

HosokawaR opened this issue Apr 7, 2024 · 4 comments

Comments

@HosokawaR
Copy link

HosokawaR commented Apr 7, 2024

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. #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.

@weshatheleopard
Copy link
Owner

weshatheleopard commented Apr 7, 2024

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.

@HosokawaR
Copy link
Author

I confirmed that this issue is resolved with rubyzip 2.4.rc1.
rubyzip/rubyzip#280 (comment)

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

@weshatheleopard
Copy link
Owner

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
Copy link
Owner

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants