Skip to content

Commit e78e02c

Browse files
ppkarwaszCopilot
andauthored
Pack200: enforce strict attribute layout parsing (apache#747)
* Pack200: enforce strict attribute layout parsing The parser for the [Pack200 attribute layout definitions](https://docs.oracle.com/en/java/javase/11/docs/specs/pack-spec.html#attribute-layout-definitions) micro-language was previously overly permissive, allowing invalid layouts and masking archive corruption. This change implements strict parsing in accordance with the [Java 11 specification](https://docs.oracle.com/en/java/javase/11/docs/specs/pack-spec.html) and removes duplication of the micro-language parsing logic between `pack200.NewAttributeBands` and `unpack200.NewAttributeBands`. * fix: PMD failure * fix: restore purpose of `lastPIntegral` * fix: remove type parameters from Javadoc This fails on JDK up to 11 and is yet another Javadoc quirk. * fix: additional bracket in Javadoc * fix: AttributeLayoutParser Javadoc * fix: formatting * Apply suggestions from code review Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * fix: Javadoc copy/paste * fix: remove unused import * fix: additional Javadoc * fix: `checkAnyIntTag` error message * fix: remove difference `AttributeLayoutElement`/`LayoutElement` * feat: add tests with deeply nested layouts * fix: Add nesting level limit This change adds a limit to the nesting level of `layout_element`s (64). Since all standard attribute layouts have a nesting level lower than 3, the limit should be suitable for most purposes. * fix: AttributeLayoutUtils Javadoc * fix: invalid test assertions * fix: checkstyle * Replace `Range<Integer>` with `IntegerRange` * Improve internal package Javadoc --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
1 parent defeeff commit e78e02c

14 files changed

Lines changed: 1279 additions & 558 deletions

File tree

pom.xml

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,11 @@ Brotli, Zstandard and ar, cpio, jar, tar, zip, dump, 7z, arj.
5050
<commons.distSvnStagingUrl>scm:svn:https://dist.apache.org/repos/dist/dev/commons/${commons.componentid}</commons.distSvnStagingUrl>
5151
<commons.manifestlocation>${project.build.outputDirectory}/META-INF</commons.manifestlocation>
5252
<commons.manifestfile>${commons.manifestlocation}/MANIFEST.MF</commons.manifestfile>
53+
<!-- Don't export internal packages in OSGi manifest -->
54+
<commons.osgi.export>
55+
!org.apache.commons.*.internal.*,
56+
org.apache.commons.*;version=${project.version};-noimport:=true
57+
</commons.osgi.export>
5358
<commons.osgi.import>
5459
org.tukaani.xz;resolution:=optional,
5560
org.brotli.dec;resolution:=optional,
@@ -540,14 +545,42 @@ Brotli, Zstandard and ar, cpio, jar, tar, zip, dump, 7z, arj.
540545
</build>
541546
</profile>
542547
<profile>
543-
<id>java9+</id>
548+
<!-- Same id as the `commons-parent` profile it extends -->
549+
<id>java-9-up</id>
544550
<activation>
545551
<jdk>[9,)</jdk>
546552
</activation>
547553
<properties>
548554
<maven.compiler.release>8</maven.compiler.release>
549555
<animal.sniffer.skip>true</animal.sniffer.skip>
550556
</properties>
557+
<!--
558+
Excludes internal packages from the list of exported packages.
559+
-->
560+
<build>
561+
<plugins>
562+
<plugin>
563+
<groupId>org.moditect</groupId>
564+
<artifactId>moditect-maven-plugin</artifactId>
565+
<executions>
566+
<execution>
567+
<id>add-module-infos</id>
568+
<configuration>
569+
<module>
570+
<moduleInfo>
571+
<exports>
572+
!org.apache.commons.compress.harmony.internal.*;
573+
!org.apache.commons.compress.harmony.archive.internal.*;
574+
*;
575+
</exports>
576+
</moduleInfo>
577+
</module>
578+
</configuration>
579+
</execution>
580+
</executions>
581+
</plugin>
582+
</plugins>
583+
</build>
551584
</profile>
552585
<profile>
553586
<id>java17</id>

src/changes/changes.xml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@ The <action> type attribute can be add,update,fix,remove.
8686
<action type="fix" dev="pkarwasz" due-to="Piotr P. Karwasz">Add strict header validation in ARJ input stream and `selfExtracting` option.</action>
8787
<!-- FIX unpack200 -->
8888
<action type="fix" dev="ggregory" due-to="Gary Gregory, Stanislav Fort">org.apache.commons.compress.harmony.unpack200 now throws Pack200Exception, IllegalArgumentException, and IllegalStateException instead of other runtime exceptions and Error.</action>
89+
<action type="fix" dev="pkarwasz" due-to="Piotr P. Karwasz">Enforce strict attribute layout parsing in Pack200.</action>
8990
<!-- FIX pack200 -->
9091
<action type="fix" dev="ggregory" due-to="Gary Gregory, Igor Morgenstern">org.apache.commons.compress.harmony.pack200 now throws Pack200Exception, IllegalArgumentException, IllegalStateException, instead of other runtime exceptions and Error.</action>
9192
<action type="fix" dev="ppkarwasz" due-to="Raeps">Extract duplicate code in org.apache.commons.compress.harmony.pack200.IntList.</action>

src/main/java/org/apache/commons/compress/harmony/archive/internal/nls/package-info.java

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,13 @@
1616
* specific language governing permissions and limitations
1717
* under the License.
1818
*/
19-
2019
/**
21-
* Internal package.
20+
* Internal implementation package, <strong>not for public use</strong>.
21+
*
22+
* <p>This package is deliberately <strong>not exported</strong> in either the OSGi manifest or the JPMS module descriptor, preventing access from external code
23+
* and other modules.</p>
24+
*
25+
* <p>All classes and methods in this package are implementation details and may change or be removed at any time without notice. External code must not depend
26+
* on them.</p>
2227
*/
2328
package org.apache.commons.compress.harmony.archive.internal.nls;

0 commit comments

Comments
 (0)