Reject unknown file type in CPIO entry mode - #790
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Adjusts CPIO header parsing to reject unsupported file-type bits in c_mode without leaking a raw IllegalArgumentException, and adds regression tests for the three supported header formats.
Changes:
- Introduces a
setMode(...)helper to validate and applyc_modeacross new ASCII, old ASCII, and old binary readers. - Attempts to convert invalid mode/file-type failures into an
ArchiveException. - Adds unit tests covering invalid file type bits in
c_modefor all three magic formats.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| src/main/java/org/apache/commons/compress/archivers/cpio/CpioArchiveInputStream.java | Centralizes mode parsing/validation and changes exception behavior for invalid file types. |
| src/test/java/org/apache/commons/compress/archivers/cpio/CpioArchiveInputStreamTest.java | Adds regression tests asserting the new error behavior for invalid c_mode file types. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (CpioUtil.fileType(mode) != 0) { // mode is initialized to 0 | ||
| newEntry.setMode(mode); | ||
| } | ||
| setMode(newEntry, mode); |
| if (CpioUtil.fileType(mode) != 0) { | ||
| ret.setMode(mode); | ||
| } | ||
| setMode(ret, mode); |
| if (CpioUtil.fileType(mode) != 0) { | ||
| oldEntry.setMode(mode); | ||
| } | ||
| setMode(oldEntry, mode); |
| private void setMode(final CpioArchiveEntry entry, final long mode) throws ArchiveException { | ||
| if (CpioUtil.fileType(mode) == 0) { | ||
| return; | ||
| } | ||
| try { | ||
| entry.setMode(mode); | ||
| } catch (final IllegalArgumentException e) { | ||
| throw new ArchiveException("Corrupted CPIO archive: Invalid file mode 0%s at byte: %,d", Long.toOctalString(mode), getBytesRead()); | ||
| } | ||
| } |
| try (CpioArchiveInputStream cpio = CpioArchiveInputStream.builder() | ||
| .setByteArray(header.getBytes(StandardCharsets.US_ASCII)) | ||
| .get()) { | ||
| assertThrows(ArchiveException.class, cpio::getNextEntry); | ||
| } |
|
Hello @kali834x |
Unknown file type in a CPIO mode field escapes as IllegalArgumentException
The three header readers gate the mode field on
CpioUtil.fileType(mode) != 0, butCpioArchiveEntry.setModeaccepts only the eight file types CPIO defines, so the seven other non-zeroS_IFMTvalues (0170000, 030000, ...) make it throw a rawIllegalArgumentExceptionout ofgetNextEntry, which declaresIOException. Routed the mode through a helper that reports such a header asArchiveException, at all three readers.mvn; that'smvnon the command line by itself.