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

Can't set time with get_output_stream #503

Closed
u3shit opened this issue Nov 6, 2021 · 4 comments · Fixed by #504
Closed

Can't set time with get_output_stream #503

u3shit opened this issue Nov 6, 2021 · 4 comments · Fixed by #504
Assignees
Labels
Milestone

Comments

@u3shit
Copy link

u3shit commented Nov 6, 2021

I'm trying to put an entry with some arbitrary timestamp into the zip file, but it doesn't work, it always writes Time.now:

require 'zip'
time = Time.new '2020-01-01' # some random date
Zip::File.open 'foo.zip', create: true do |zip|
  e = Zip::Entry.new zip.name, 'some_file'
  e.time = Zip::DOSTime.from_time time
  zip.get_output_stream(e) {|s| s.puts 'foobar' }
end

But unzip reports the current date:

$ unzip -l foo.zip 
Archive:  foo.zip
  Length      Date    Time    Name
---------  ---------- -----   ----
        7  11-06-2021 17:56   some_file
---------                     -------
        7                     1 file

It looks like File#get_output_stream adds a StreamableStream to @entry_set, but later in OutputStream#put_next_entry it checks entry_name.kind_of?(Entry), which will be false and it creates a new Entry ignoring everything I've set up in the entry. The same happens if I don't create the entry manually but pass the time to #get_output_stream.
File#add works correctly, but it requires creating a temporary file and manually setting the last modification date on it.
Using ruby 3.0.2p107 and rubyzip 2.3.2.

@hainesr
Copy link
Member

hainesr commented Nov 7, 2021

Hi @u3shit, thanks for reporting this.

I think I see what's going wrong here: presumably the type mismatch between StreamableStream and Entry. I wasn't around when this was written, so I don't know the subtleties of why StreamableStream is a delegate, rather than a subclass, of Entry, but just naively widening the kind_of? check you mention above causes plenty of other errors.

I can see a way to a fix though, so I'll get this sorted as soon as I can.

@hainesr hainesr self-assigned this Nov 7, 2021
@hainesr hainesr added the bug label Nov 7, 2021
@hainesr hainesr added this to the 3.0 milestone Nov 7, 2021
hainesr added a commit to hainesr/rubyzip that referenced this issue Nov 7, 2021
When passing an `Entry` type to `File#get_output_stream` the entry is
used to create a `StreamableStream`, which preserves all the info in the
entry, such as timestamp, etc. But then in `put_next_entry` all that is
lost due to the test for `kind_of?(Entry)` which a `StreamableStream` is
not. See rubyzip#503 for details.

This change tests for `StreamableStream`s in `put_next_entry` and uses
them directly. Some set-up within `Entry` needed to be made more robust
to cope with this, but otherwise it's a low impact change, which does
fix the problem.

The reason this case was being missed before is that the tests weren't
testing `get_output_stream` with an `Entry` object, so I have also added
that test too.

Fixes rubyzip#503.
hainesr added a commit to hainesr/rubyzip that referenced this issue Nov 7, 2021
When passing an `Entry` type to `File#get_output_stream` the entry is
used to create a `StreamableStream`, which preserves all the info in the
entry, such as timestamp, etc. But then in `put_next_entry` all that is
lost due to the test for `kind_of?(Entry)` which a `StreamableStream` is
not. See rubyzip#503 for details.

This change tests for `StreamableStream`s in `put_next_entry` and uses
them directly. Some set-up within `Entry` needed to be made more robust
to cope with this, but otherwise it's a low impact change, which does
fix the problem.

The reason this case was being missed before is that the tests weren't
testing `get_output_stream` with an `Entry` object, so I have also added
that test too.

Fixes rubyzip#503.
@hainesr
Copy link
Member

hainesr commented Nov 8, 2021

Hi @u3shit, I've implemented a fix for this in #504. It fixes the example you posted above (for me), so if you have time to test as well that would be useful, thanks.

@u3shit
Copy link
Author

u3shit commented Nov 8, 2021

Thanks, it looks like it's working with my code (after I fixed the 3.0 api break with optional parameters, uh-oh. But at least it looks like I don't even have to construct an Entry manually now since I can pass time to get_output_stream without having to type countless nils)

@hainesr
Copy link
Member

hainesr commented Nov 8, 2021

Thanks for checking! ⚡

I am a bit nervous about the API changes, but hopefully people will see the benefits of the more modern style. As you say - not so many nils...

hainesr added a commit to hainesr/rubyzip that referenced this issue Nov 19, 2021
When passing an `Entry` type to `File#get_output_stream` the entry is
used to create a `StreamableStream`, which preserves all the info in the
entry, such as timestamp, etc. But then in `put_next_entry` all that is
lost due to the test for `kind_of?(Entry)` which a `StreamableStream` is
not. See rubyzip#503 for details.

This change tests for `StreamableStream`s in `put_next_entry` and uses
them directly. Some set-up within `Entry` needed to be made more robust
to cope with this, but otherwise it's a low impact change, which does
fix the problem.

The reason this case was being missed before is that the tests weren't
testing `get_output_stream` with an `Entry` object, so I have also added
that test too.

Fixes rubyzip#503.
hainesr added a commit to hainesr/rubyzip that referenced this issue Nov 19, 2021
When passing an `Entry` type to `File#get_output_stream` the entry is
used to create a `StreamableStream`, which preserves all the info in the
entry, such as timestamp, etc. But then in `put_next_entry` all that is
lost due to the test for `kind_of?(Entry)` which a `StreamableStream` is
not. See rubyzip#503 for details.

This change tests for `StreamableStream`s in `put_next_entry` and uses
them directly. Some set-up within `Entry` needed to be made more robust
to cope with this, but otherwise it's a low impact change, which does
fix the problem.

The reason this case was being missed before is that the tests weren't
testing `get_output_stream` with an `Entry` object, so I have also added
that test too.

Fixes rubyzip#503.
hainesr added a commit to hainesr/rubyzip that referenced this issue Nov 20, 2021
When passing an `Entry` type to `File#get_output_stream` the entry is
used to create a `StreamableStream`, which preserves all the info in the
entry, such as timestamp, etc. But then in `put_next_entry` all that is
lost due to the test for `kind_of?(Entry)` which a `StreamableStream` is
not. See rubyzip#503 for details.

This change tests for `StreamableStream`s in `put_next_entry` and uses
them directly. Some set-up within `Entry` needed to be made more robust
to cope with this, but otherwise it's a low impact change, which does
fix the problem.

The reason this case was being missed before is that the tests weren't
testing `get_output_stream` with an `Entry` object, so I have also added
that test too.

Fixes rubyzip#503.
hainesr added a commit to hainesr/rubyzip that referenced this issue Dec 1, 2021
When passing an `Entry` type to `File#get_output_stream` the entry is
used to create a `StreamableStream`, which preserves all the info in the
entry, such as timestamp, etc. But then in `put_next_entry` all that is
lost due to the test for `kind_of?(Entry)` which a `StreamableStream` is
not. See rubyzip#503 for details.

This change tests for `StreamableStream`s in `put_next_entry` and uses
them directly. Some set-up within `Entry` needed to be made more robust
to cope with this, but otherwise it's a low impact change, which does
fix the problem.

The reason this case was being missed before is that the tests weren't
testing `get_output_stream` with an `Entry` object, so I have also added
that test too.

Fixes rubyzip#503.
hainesr added a commit to hainesr/rubyzip that referenced this issue Jan 2, 2022
When passing an `Entry` type to `File#get_output_stream` the entry is
used to create a `StreamableStream`, which preserves all the info in the
entry, such as timestamp, etc. But then in `put_next_entry` all that is
lost due to the test for `kind_of?(Entry)` which a `StreamableStream` is
not. See rubyzip#503 for details.

This change tests for `StreamableStream`s in `put_next_entry` and uses
them directly. Some set-up within `Entry` needed to be made more robust
to cope with this, but otherwise it's a low impact change, which does
fix the problem.

The reason this case was being missed before is that the tests weren't
testing `get_output_stream` with an `Entry` object, so I have also added
that test too.

Fixes rubyzip#503.
hainesr added a commit to hainesr/rubyzip that referenced this issue Jan 11, 2022
When passing an `Entry` type to `File#get_output_stream` the entry is
used to create a `StreamableStream`, which preserves all the info in the
entry, such as timestamp, etc. But then in `put_next_entry` all that is
lost due to the test for `kind_of?(Entry)` which a `StreamableStream` is
not. See rubyzip#503 for details.

This change tests for `StreamableStream`s in `put_next_entry` and uses
them directly. Some set-up within `Entry` needed to be made more robust
to cope with this, but otherwise it's a low impact change, which does
fix the problem.

The reason this case was being missed before is that the tests weren't
testing `get_output_stream` with an `Entry` object, so I have also added
that test too.

Fixes rubyzip#503.
hainesr added a commit to hainesr/rubyzip that referenced this issue Jan 11, 2022
When passing an `Entry` type to `File#get_output_stream` the entry is
used to create a `StreamableStream`, which preserves all the info in the
entry, such as timestamp, etc. But then in `put_next_entry` all that is
lost due to the test for `kind_of?(Entry)` which a `StreamableStream` is
not. See rubyzip#503 for details.

This change tests for `StreamableStream`s in `put_next_entry` and uses
them directly. Some set-up within `Entry` needed to be made more robust
to cope with this, but otherwise it's a low impact change, which does
fix the problem.

The reason this case was being missed before is that the tests weren't
testing `get_output_stream` with an `Entry` object, so I have also added
that test too.

Fixes rubyzip#503.
hainesr added a commit to hainesr/rubyzip that referenced this issue Jan 18, 2022
When passing an `Entry` type to `File#get_output_stream` the entry is
used to create a `StreamableStream`, which preserves all the info in the
entry, such as timestamp, etc. But then in `put_next_entry` all that is
lost due to the test for `kind_of?(Entry)` which a `StreamableStream` is
not. See rubyzip#503 for details.

This change tests for `StreamableStream`s in `put_next_entry` and uses
them directly. Some set-up within `Entry` needed to be made more robust
to cope with this, but otherwise it's a low impact change, which does
fix the problem.

The reason this case was being missed before is that the tests weren't
testing `get_output_stream` with an `Entry` object, so I have also added
that test too.

Fixes rubyzip#503.
hainesr added a commit that referenced this issue Jan 20, 2022
When passing an `Entry` type to `File#get_output_stream` the entry is
used to create a `StreamableStream`, which preserves all the info in the
entry, such as timestamp, etc. But then in `put_next_entry` all that is
lost due to the test for `kind_of?(Entry)` which a `StreamableStream` is
not. See #503 for details.

This change tests for `StreamableStream`s in `put_next_entry` and uses
them directly. Some set-up within `Entry` needed to be made more robust
to cope with this, but otherwise it's a low impact change, which does
fix the problem.

The reason this case was being missed before is that the tests weren't
testing `get_output_stream` with an `Entry` object, so I have also added
that test too.

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

Successfully merging a pull request may close this issue.

2 participants