You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by GitBox <gi...@apache.org> on 2022/03/16 19:34:05 UTC
[GitHub] [commons-compress] garydgregory commented on a change in pull request #254: COMPRESS-612: improve TAR support for file times
garydgregory commented on a change in pull request #254:
URL: https://github.com/apache/commons-compress/pull/254#discussion_r828374324
##########
File path: src/main/java/org/apache/commons/compress/archivers/tar/TarArchiveOutputStream.java
##########
@@ -624,11 +635,48 @@ private void addPaxHeaderForBigNumber(final Map<String, String> paxHeaders,
}
}
+ private void addFileTimePaxHeaderForBigNumber(final Map<String, String> paxHeaders,
+ final String header, final FileTime value,
+ final long maxValue) {
+ if (value != null) {
+ Instant instant = value.toInstant();
+ long seconds = instant.getEpochSecond();
+ int nanos = instant.getNano();
+ if (nanos == 0) {
+ addPaxHeaderForBigNumber(paxHeaders, header, seconds, maxValue);
+ } else {
+ addInstantPaxHeader(paxHeaders, header, seconds, nanos);
+ }
+ }
+ }
+
+ private void addFileTimePaxHeader(final Map<String, String> paxHeaders,
+ final String header, final FileTime value) {
+ if (value != null) {
+ Instant instant = value.toInstant();
Review comment:
Use final where possible.
##########
File path: src/test/java/org/apache/commons/compress/archivers/tar/TarArchiveEntryTest.java
##########
@@ -306,6 +308,164 @@ public void getOrderedSparseHeadersRejectsStructsPointingBeyondOutputEntry() thr
te.getOrderedSparseHeaders();
}
+ @Test
+ public void shouldParseTimePaxHeadersAndNotCountAsExtraPaxHeaders() {
+ TarArchiveEntry entry = createEntryForTimeTests();
+ assertEquals("extra header count", 0, entry.getExtraPaxHeaders().size());
+ assertNull("size", entry.getExtraPaxHeader("size"));
+ assertNull("mtime", entry.getExtraPaxHeader("mtime"));
+ assertNull("atime", entry.getExtraPaxHeader("atime"));
+ assertNull("ctime", entry.getExtraPaxHeader("ctime"));
+ assertNull("birthtime", entry.getExtraPaxHeader("LIBARCHIVE.creationtime"));
+ assertEquals("size", entry.getSize(), 1);
+ assertEquals("mtime", FileTime.from(Instant.parse("2022-03-14T01:25:03.599853900Z")), entry.getLastModifiedTime());
+ assertEquals("atime", FileTime.from(Instant.parse("2022-03-14T01:31:00.706927200Z")), entry.getLastAccessTime());
+ assertEquals("ctime", FileTime.from(Instant.parse("2022-03-14T01:28:59.700505300Z")), entry.getStatusChangeTime());
+ assertEquals("birthtime", FileTime.from(Instant.parse("2022-03-14T01:29:00.723509000Z")), entry.getCreationTime());
+ }
+
+ @Test
+ public void shouldNotWriteTimePaxHeadersByDefault() throws IOException {
+ TarArchiveEntry entry = createEntryForTimeTests();
+
+ final ByteArrayOutputStream bos = new ByteArrayOutputStream();
+ final TarArchiveOutputStream tos = new TarArchiveOutputStream(bos);
+ tos.putArchiveEntry(entry);
+ tos.write('W');
+ tos.closeArchiveEntry();
+ tos.close();
+
+ final TarArchiveInputStream tis = new TarArchiveInputStream(new ByteArrayInputStream(bos.toByteArray()));
+ entry = tis.getNextTarEntry();
+ assertNotNull("couldn't get entry", entry);
+
+ assertEquals("extra header count", 0, entry.getExtraPaxHeaders().size());
+ assertNull("mtime", entry.getExtraPaxHeader("mtime"));
+ assertNull("atime", entry.getExtraPaxHeader("atime"));
+ assertNull("ctime", entry.getExtraPaxHeader("ctime"));
+ assertNull("birthtime", entry.getExtraPaxHeader("LIBARCHIVE.creationtime"));
+ assertEquals("mtime", FileTime.from(Instant.parse("2022-03-14T01:25:03Z")), entry.getLastModifiedTime());
+ assertNull("atime", entry.getLastAccessTime());
+ assertNull("ctime", entry.getStatusChangeTime());
+ assertNull("birthtime", entry.getCreationTime());
+
+ assertEquals('W', tis.read());
+ assertTrue("should be at end of entry", tis.read() < 0);
+
+ assertNull("should be at end of file", tis.getNextTarEntry());
+ tis.close();
+ }
+
+ @Test
+ public void shouldWriteTimesForStarMode() throws IOException {
+ TarArchiveEntry entry = createEntryForTimeTests();
+
+ final ByteArrayOutputStream bos = new ByteArrayOutputStream();
+ final TarArchiveOutputStream tos = new TarArchiveOutputStream(bos);
+ tos.setBigNumberMode(TarArchiveOutputStream.BIGNUMBER_STAR);
+ tos.putArchiveEntry(entry);
+ tos.write('W');
+ tos.closeArchiveEntry();
+ tos.close();
+
+ final TarArchiveInputStream tis = new TarArchiveInputStream(new ByteArrayInputStream(bos.toByteArray()));
+ entry = tis.getNextTarEntry();
+ assertNotNull("couldn't get entry", entry);
+
+ assertEquals("extra header count", 0, entry.getExtraPaxHeaders().size());
+ assertNull("mtime", entry.getExtraPaxHeader("mtime"));
+ assertNull("atime", entry.getExtraPaxHeader("atime"));
+ assertNull("ctime", entry.getExtraPaxHeader("ctime"));
+ assertNull("birthtime", entry.getExtraPaxHeader("LIBARCHIVE.creationtime"));
+ assertEquals("mtime", FileTime.from(Instant.parse("2022-03-14T01:25:03Z")), entry.getLastModifiedTime());
+ assertEquals("atime", FileTime.from(Instant.parse("2022-03-14T01:31:00Z")), entry.getLastAccessTime());
+ assertEquals("ctime", FileTime.from(Instant.parse("2022-03-14T01:28:59Z")), entry.getStatusChangeTime());
+ assertNull("birthtime", entry.getCreationTime());
+
+ assertEquals('W', tis.read());
+ assertTrue("should be at end of entry", tis.read() < 0);
+
+ assertNull("should be at end of file", tis.getNextTarEntry());
+ tis.close();
+ }
+
+ @Test
+ public void shouldWriteTimesAsPaxHeadersForPosixMode() throws IOException {
+ TarArchiveEntry entry = createEntryForTimeTests();
+
+ final ByteArrayOutputStream bos = new ByteArrayOutputStream();
Review comment:
Use try-with-resources. I know this is will not cause a real resource leak on a thrown exception this avoids compiler warnings and it is a best practice we want to exemplify.
##########
File path: src/main/java/org/apache/commons/compress/archivers/tar/TarArchiveOutputStream.java
##########
@@ -624,11 +635,48 @@ private void addPaxHeaderForBigNumber(final Map<String, String> paxHeaders,
}
}
+ private void addFileTimePaxHeaderForBigNumber(final Map<String, String> paxHeaders,
+ final String header, final FileTime value,
+ final long maxValue) {
+ if (value != null) {
+ Instant instant = value.toInstant();
Review comment:
Use final where possible.
##########
File path: src/main/java/org/apache/commons/compress/archivers/tar/TarConstants.java
##########
@@ -347,6 +361,13 @@
*/
String MAGIC_XSTAR = "tar\0";
+ /**
+ * Offset inside the header for the xtar multivolume data
+ *
Review comment:
Is there an RFC or resource for which we can provide a URL for these offsets?
##########
File path: src/test/java/org/apache/commons/compress/archivers/tar/TarArchiveEntryTest.java
##########
@@ -306,6 +308,164 @@ public void getOrderedSparseHeadersRejectsStructsPointingBeyondOutputEntry() thr
te.getOrderedSparseHeaders();
}
+ @Test
+ public void shouldParseTimePaxHeadersAndNotCountAsExtraPaxHeaders() {
+ TarArchiveEntry entry = createEntryForTimeTests();
+ assertEquals("extra header count", 0, entry.getExtraPaxHeaders().size());
+ assertNull("size", entry.getExtraPaxHeader("size"));
+ assertNull("mtime", entry.getExtraPaxHeader("mtime"));
+ assertNull("atime", entry.getExtraPaxHeader("atime"));
+ assertNull("ctime", entry.getExtraPaxHeader("ctime"));
+ assertNull("birthtime", entry.getExtraPaxHeader("LIBARCHIVE.creationtime"));
+ assertEquals("size", entry.getSize(), 1);
+ assertEquals("mtime", FileTime.from(Instant.parse("2022-03-14T01:25:03.599853900Z")), entry.getLastModifiedTime());
+ assertEquals("atime", FileTime.from(Instant.parse("2022-03-14T01:31:00.706927200Z")), entry.getLastAccessTime());
+ assertEquals("ctime", FileTime.from(Instant.parse("2022-03-14T01:28:59.700505300Z")), entry.getStatusChangeTime());
+ assertEquals("birthtime", FileTime.from(Instant.parse("2022-03-14T01:29:00.723509000Z")), entry.getCreationTime());
+ }
+
+ @Test
+ public void shouldNotWriteTimePaxHeadersByDefault() throws IOException {
+ TarArchiveEntry entry = createEntryForTimeTests();
+
+ final ByteArrayOutputStream bos = new ByteArrayOutputStream();
+ final TarArchiveOutputStream tos = new TarArchiveOutputStream(bos);
+ tos.putArchiveEntry(entry);
+ tos.write('W');
+ tos.closeArchiveEntry();
+ tos.close();
+
+ final TarArchiveInputStream tis = new TarArchiveInputStream(new ByteArrayInputStream(bos.toByteArray()));
+ entry = tis.getNextTarEntry();
+ assertNotNull("couldn't get entry", entry);
+
+ assertEquals("extra header count", 0, entry.getExtraPaxHeaders().size());
+ assertNull("mtime", entry.getExtraPaxHeader("mtime"));
+ assertNull("atime", entry.getExtraPaxHeader("atime"));
+ assertNull("ctime", entry.getExtraPaxHeader("ctime"));
+ assertNull("birthtime", entry.getExtraPaxHeader("LIBARCHIVE.creationtime"));
+ assertEquals("mtime", FileTime.from(Instant.parse("2022-03-14T01:25:03Z")), entry.getLastModifiedTime());
+ assertNull("atime", entry.getLastAccessTime());
+ assertNull("ctime", entry.getStatusChangeTime());
+ assertNull("birthtime", entry.getCreationTime());
+
+ assertEquals('W', tis.read());
+ assertTrue("should be at end of entry", tis.read() < 0);
+
+ assertNull("should be at end of file", tis.getNextTarEntry());
+ tis.close();
+ }
+
+ @Test
+ public void shouldWriteTimesForStarMode() throws IOException {
+ TarArchiveEntry entry = createEntryForTimeTests();
+
+ final ByteArrayOutputStream bos = new ByteArrayOutputStream();
Review comment:
Use try-with-resources. I know this is will not cause a real resource leak on a thrown exception this avoids compiler warnings and it is a best practice we want to exemplify.
##########
File path: src/test/java/org/apache/commons/compress/archivers/tar/TarArchiveEntryTest.java
##########
@@ -306,6 +308,164 @@ public void getOrderedSparseHeadersRejectsStructsPointingBeyondOutputEntry() thr
te.getOrderedSparseHeaders();
}
+ @Test
+ public void shouldParseTimePaxHeadersAndNotCountAsExtraPaxHeaders() {
+ TarArchiveEntry entry = createEntryForTimeTests();
+ assertEquals("extra header count", 0, entry.getExtraPaxHeaders().size());
+ assertNull("size", entry.getExtraPaxHeader("size"));
+ assertNull("mtime", entry.getExtraPaxHeader("mtime"));
+ assertNull("atime", entry.getExtraPaxHeader("atime"));
+ assertNull("ctime", entry.getExtraPaxHeader("ctime"));
+ assertNull("birthtime", entry.getExtraPaxHeader("LIBARCHIVE.creationtime"));
+ assertEquals("size", entry.getSize(), 1);
+ assertEquals("mtime", FileTime.from(Instant.parse("2022-03-14T01:25:03.599853900Z")), entry.getLastModifiedTime());
+ assertEquals("atime", FileTime.from(Instant.parse("2022-03-14T01:31:00.706927200Z")), entry.getLastAccessTime());
+ assertEquals("ctime", FileTime.from(Instant.parse("2022-03-14T01:28:59.700505300Z")), entry.getStatusChangeTime());
+ assertEquals("birthtime", FileTime.from(Instant.parse("2022-03-14T01:29:00.723509000Z")), entry.getCreationTime());
+ }
+
+ @Test
+ public void shouldNotWriteTimePaxHeadersByDefault() throws IOException {
+ TarArchiveEntry entry = createEntryForTimeTests();
+
+ final ByteArrayOutputStream bos = new ByteArrayOutputStream();
Review comment:
Use try-with-resources. I know this is will not cause a real resource leak on a thrown exception this avoids compiler warnings and it is a best practice we want to exemplify.
##########
File path: src/main/java/org/apache/commons/compress/archivers/tar/TarConstants.java
##########
@@ -227,6 +227,13 @@
*/
byte LF_OLDNORM = 0;
+ /**
+ * Offset inside the header for the "link flag" field.
+ *
+ * @since 1.22
+ */
+ int LF_OFFSET = 156;
Review comment:
No need for whitespace IMO.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org