You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The extraction logic appears incorrect - in the v3 branch, file_name is passed as the second parameter instead of the full path, which may cause files to be extracted to wrong locations or with wrong names.
The v3 extraction method only uses destination_directory parameter but ignores the calculated to path, potentially breaking the existing file structure and naming logic.
The rubyzip v3 extraction parameters are incorrect. The second parameter should be the entry name, not the file name, to maintain consistent extraction behavior.
if Zipper::RUBYZIP_V3
- zip.extract(entry, file_name, destination_directory: target_directory)+ zip.extract(entry, entry.name, destination_directory: "#{target_directory}#{file_name}")
else
zip.extract(entry, "#{target_directory}#{file_name}")
end
Apply / Chat
Suggestion importance[1-10]: 9
__
Why: This suggestion correctly identifies a bug where the downloaded file would be extracted with an incorrect name (file_name instead of entry.name), and provides a fix that ensures consistent behavior across rubyzip versions.
High
Fix rubyzip v3 extraction path
The rubyzip v3 API change breaks the extraction logic. The destination_directory parameter should be combined with the entry name to maintain the same file structure as the v2 behavior.
if RUBYZIP_V3
- zip.extract(entry, destination_directory: destination)+ zip.extract(entry, entry.name, destination_directory: destination)
else
zip.extract(entry, to)
end
Apply / Chat
Suggestion importance[1-10]: 3
__
Why: The suggestion is correct that the improved code works, but the original code is also correct because the dest_path argument in zip.extract defaults to entry.name in rubyzip v3, making the change redundant.
After quite a while, it was finally released.
There are a few breaking changes to be aware of:
https://github.com/rubyzip/rubyzip/wiki/Updating-to-version-3.x
Almost everything is already abstracted away behind he upper module, so the needed
changes are rather small.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
User description
💥 What does this PR do?
Allow to use
rubyzipv3. After quite a while, it was finally released.There are a few breaking changes to be aware of:
https://github.com/rubyzip/rubyzip/wiki/Updating-to-version-3.x
Closes #16111
🔧 Implementation Notes
Almost everything is already abstracted away behind he upper module, so the needed changes are rather small.
🔄 Types of changes
PR Type
Enhancement
Description
Update
rubyzipdependency to support version 3.xAdd version detection for backward compatibility
Modify zip extraction methods for API changes
Update gemspec dependency constraint
Diagram Walkthrough
File Walkthrough
has_file_downloads.rb
Add rubyzip v3 compatibility to file downloadsrb/lib/selenium/webdriver/common/driver_extensions/has_file_downloads.rb
zipper.rb
Implement rubyzip v3 compatibility layerrb/lib/selenium/webdriver/common/zipper.rb
RUBYZIP_V3selenium-webdriver.gemspec
Allow rubyzip version 3.x in dependenciesrb/selenium-webdriver.gemspec
< 3.0to< 4.0