Check for negative millisecond offset #171
Conversation
File.setLastModified() does not allow negative values, but those can be present in Date. One such source is ZipEntry.getTime(), which returns -1 to indicate a non-applicable time. Signed-off-by: Robert Varga <[email protected]>
| final long millis = entryDate.getTime(); | ||
| if ( millis >= 0 ) | ||
| { | ||
| targetFileName.setLastModified( millis ); |
There was a problem hiding this comment.
I find setLastModified behavior strange because lastModified works perfectly fine with negative values (dates before 1970-01-01). We can use Files.setLastModifiedTime, which works with negative values[1], and use null for missing date. Some archives, such as tar, support negative values so it should be up to the specific UnArchiver implementation to decide if the date is known or not.
[1] Whether the underlying file system supports it is different question. Unfortunately the behavior is unspecified if the date is out of range for the file system. Maybe we can just catch the exception so failure to set the date does not fail the file extraction?
There was a problem hiding this comment.
Pull Request Overview
This PR adds a validation check to prevent passing negative millisecond values to File.setLastModified(). The change addresses an issue where ZipEntry.getTime() can return -1 to indicate a non-applicable time, which would cause problems when passed directly to setLastModified().
- Added millisecond value extraction and validation before calling
setLastModified() - Only calls
setLastModified()when the timestamp is non-negative
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
3 participants
File.setLastModified() does not allow negative values, but those
can be present in Date. One such source is ZipEntry.getTime(), which
returns -1 to indicate a non-applicable time. Fixes #170.
Signed-off-by: Robert Varga [email protected]