8000 Options not working in File class · Issue #395 · rubyzip/rubyzip · GitHub
[go: up one dir, main page]
Skip to content

Options not working in File class #395

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.

Sign up for GitHub

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

Jump to bottom
Closed
augustocravosilva opened this issue May 10, 2019 · 4 comments · Fixed by #418 or #421
Closed

Options not working in File class #395

augustocravosilva opened this issue May 10, 2019 · 4 comments · Fixed by #418 or #421

Comments

@augustocravosilva
Copy link

This code

rubyzip/lib/zip/file.rb

Lines 88 to 89 in 249775f

@restore_permissions = options[:restore_permissions] || true
@restore_times = options[:restore_times] || true

doesn't do what it is intended to do, because when you pass in restore_permissions: false or restore_times: false, it will be evaluating to false || true, which will always be true
@hainesr
Copy link
Member
hainesr commented Sep 16, 2019

Interestingly the options code doesn't agree with the comments about defaults above either:

rubyzip/lib/zip/file.rb

Lines 56 to 61 in 94b7fa2

# default -> false
attr_accessor :restore_ownership
# default -> false
attr_accessor :restore_permissions
# default -> true
attr_accessor :restore_times

vs

rubyzip/lib/zip/file.rb

Lines 101 to 103 in 94b7fa2

@restore_ownership = options[:restore_ownership] || false
@restore_permissions = options[:restore_permissions] || true
@restore_times = options[:restore_times] || true

I'll cook up a PR that fixes this issue and works as expected to keep the test passing - adjusting the comments as necessary.

@hainesr
Copy link
Member
hainesr commented Sep 16, 2019

Also, depressingly, it doesn't matter what those options are set to; the tests always all pass. I'll add some tests.

hainesr added a commit to hainesr/rubyzip that referenced this issue Sep 16, 2019
@hainesr
Correctly set/default options in the File class.
cfcdf83 
Fixes rubyzip#395.
hainesr added a commit to hainesr/rubyzip that referenced this issue Oct 1, 2019
@hainesr
Correctly set/default options in the File class.
2e393f4 
Fixes rubyzip#395.
@hainesr
Copy link
Member
hainesr commented Oct 6, 2019

So, on further investigation, I don't think that restore_times: true can ever work, because I don't think that when you extract a file it ever even attempts to set the timestamp.

As far as I can tell, this is the code which does the actual extraction to a file:

rubyzip/lib/zip/entry.rb

Lines 597 to 624 in 34d2074

def create_file(dest_path, _continue_on_exists_proc = proc { Zip.continue_on_exists_proc })
if ::File.exist?(dest_path) && !yield(self, dest_path)
raise ::Zip::DestinationFileExistsError,
"Destination '#{dest_path}' already exists"
end
::File.open(dest_path, 'wb') do |os|
get_input_stream do |is|
set_extra_attributes_on_path(dest_path)
bytes_written = 0
warned = false
buf = ''.dup
while (buf = is.sysread(::Zip 8000 ::Decompressor::CHUNK_SIZE, buf))
os << buf
bytes_written += buf.bytesize
if bytes_written > size && !warned
message = "Entry #{name} should be #{size}B but is larger when inflated"
if ::Zip.validate_entry_sizes
raise ::Zip::EntrySizeError, message
else
puts "WARNING: #{message}"
warned = true
end
end
end
end
end
end

And there's nothing there that would restore times...

I'm looking at this in more depth.

hainesr added a commit to hainesr/rubyzip that referenced this issue Oct 6, 2019
@hainesr
Correctly set/default options in the File class.
8bc9441 
Fixes rubyzip#395.
@hainesr hainesr mentioned this issue Oct 7, 2019
Merged
@hainesr hainesr mentioned this issue Oct 19, 2019
Merged
@jdleesmiller
Copy link
Member

Sorry, overzealous auto-closing. This is still a problem.

@jdleesmiller jdleesmiller reopened this Oct 19, 2019
This was referenced Oct 30, 2019
Merged
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
4 participants
@hainesr @jdleesmiller @augustocravosilva and others
0