This is an automated email from the ASF dual-hosted git repository. pkarwasz pushed a commit to branch fix/attribute-layout-parser in repository https://gitbox.apache.org/repos/asf/commons-compress.git
commit 1c14156e0a3db9942b8023df4a6785f1ae3ae91a Author: Piotr P. Karwasz <[email protected]> AuthorDate: Tue Nov 11 17:36:52 2025 +0100 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`. --- pom.xml | 35 +- src/changes/changes.xml | 1 + .../harmony/archive/internal/nls/package-info.java | 6 +- .../harmony/internal/AttributeLayoutParser.java | 362 ++++++++++++++++++++ .../harmony/internal/AttributeLayoutUtils.java | 252 ++++++++++++++ .../internal/nls => internal}/package-info.java | 9 +- .../harmony/pack200/NewAttributeBands.java | 367 ++++++-------------- .../harmony/unpack200/NewAttributeBands.java | 375 ++++++--------------- .../internal/AttributeLayoutParserTest.java | 149 ++++++++ .../harmony/internal/AttributeLayoutUtilsTest.java | 79 +++++ .../compress/harmony/pack200/Compress626Test.java | 6 +- .../compress/harmony/pack200/Compress628Test.java | 6 +- .../harmony/pack200/NewAttributeBandsTest.java | 2 +- .../harmony/unpack200/NewAttributeBandsTest.java | 29 +- 14 files changed, 1132 insertions(+), 546 deletions(-) diff --git a/pom.xml b/pom.xml index 987774995..c709cc20d 100644 --- a/pom.xml +++ b/pom.xml @@ -50,6 +50,11 @@ Brotli, Zstandard and ar, cpio, jar, tar, zip, dump, 7z, arj. <commons.distSvnStagingUrl>scm:svn:https://dist.apache.org/repos/dist/dev/commons/${commons.componentid}</commons.distSvnStagingUrl> <commons.manifestlocation>${project.build.outputDirectory}/META-INF</commons.manifestlocation> <commons.manifestfile>${commons.manifestlocation}/MANIFEST.MF</commons.manifestfile> + <!-- Don't export internal packages in OSGi manifest --> + <commons.osgi.export> + !org.apache.commons.*.internal.*, + org.apache.commons.*;version=${project.version};-noimport:=true + </commons.osgi.export> <commons.osgi.import> org.tukaani.xz;resolution:=optional, org.brotli.dec;resolution:=optional, @@ -540,7 +545,8 @@ Brotli, Zstandard and ar, cpio, jar, tar, zip, dump, 7z, arj. </build> </profile> <profile> - <id>java9+</id> + <!-- Same id as the `commons-parent` profile it extends --> + <id>java-9-up</id> <activation> <jdk>[9,)</jdk> </activation> @@ -548,6 +554,33 @@ Brotli, Zstandard and ar, cpio, jar, tar, zip, dump, 7z, arj. <maven.compiler.release>8</maven.compiler.release> <animal.sniffer.skip>true</animal.sniffer.skip> </properties> + <!-- + Excludes internal packages from the list of exported packages. + --> + <build> + <plugins> + <plugin> + <groupId>org.moditect</groupId> + <artifactId>moditect-maven-plugin</artifactId> + <executions> + <execution> + <id>add-module-infos</id> + <configuration> + <module> + <moduleInfo> + <exports> + !org.apache.commons.compress.harmony.internal.*; + !org.apache.commons.compress.harmony.archive.internal.*; + *; + </exports> + </moduleInfo> + </module> + </configuration> + </execution> + </executions> + </plugin> + </plugins> + </build> </profile> <profile> <id>java17</id> diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 20c0c855b..d192ff9c2 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -86,6 +86,7 @@ The <action> type attribute can be add,update,fix,remove. <action type="fix" dev="pkarwasz" due-to="Piotr P. Karwasz">Add strict header validation in ARJ input stream and `selfExtracting` option.</action> <!-- FIX unpack200 --> <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> + <action type="fix" dev="pkarwasz" due-to="Piotr P. Karwasz">Enforce strict attribute layout parsing in Pack200.</action> <!-- FIX pack200 --> <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> <action type="fix" dev="ppkarwasz" due-to="Raeps">Extract duplicate code in org.apache.commons.compress.harmony.pack200.IntList.</action> diff --git a/src/main/java/org/apache/commons/compress/harmony/archive/internal/nls/package-info.java b/src/main/java/org/apache/commons/compress/harmony/archive/internal/nls/package-info.java index 672547a16..9b2dbdcd9 100644 --- a/src/main/java/org/apache/commons/compress/harmony/archive/internal/nls/package-info.java +++ b/src/main/java/org/apache/commons/compress/harmony/archive/internal/nls/package-info.java @@ -16,8 +16,10 @@ * specific language governing permissions and limitations * under the License. */ - /** - * Internal package. + * Internal implementation details. + * + * <p>This package is not exported and must not be used by external code. + * Its contents may change or be removed at any time without notice.</p> */ package org.apache.commons.compress.harmony.archive.internal.nls; diff --git a/src/main/java/org/apache/commons/compress/harmony/internal/AttributeLayoutParser.java b/src/main/java/org/apache/commons/compress/harmony/internal/AttributeLayoutParser.java new file mode 100644 index 000000000..b1d9f7a83 --- /dev/null +++ b/src/main/java/org/apache/commons/compress/harmony/internal/AttributeLayoutParser.java @@ -0,0 +1,362 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.commons.compress.harmony.internal; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; + +import org.apache.commons.compress.harmony.pack200.Pack200Exception; +import org.apache.commons.lang3.Range; + +/** + * Encapsulates parsing of attribute layout definitions. + * + * <p>Previously the parsing logic was duplicated in {@link org.apache.commons.compress.harmony.pack200.NewAttributeBands} and + * {@link org.apache.commons.compress.harmony.unpack200.NewAttributeBands}.</p> + * + * @param <LAYOUT_ELEMENT> the type corresponding to the {@code layout_element} production + * @param <ATTRIBUTE_LAYOUT_ELEMENT> the type corresponding to the elements of the {@code attribute_layout} production: either {@code layout_element} or + * {@code callable} + */ +public final class AttributeLayoutParser<ATTRIBUTE_LAYOUT_ELEMENT, LAYOUT_ELEMENT extends ATTRIBUTE_LAYOUT_ELEMENT> { + + /** + * Factory interface for creating attribute layout elements. + */ + public interface Factory<ATTRIBUTE_LAYOUT_ELEMENT, LAYOUT_ELEMENT extends ATTRIBUTE_LAYOUT_ELEMENT> { + LAYOUT_ELEMENT createCall(int callableIndex); + + ATTRIBUTE_LAYOUT_ELEMENT createCallable(String body) throws Pack200Exception; + + LAYOUT_ELEMENT createIntegral(String tag); + + LAYOUT_ELEMENT createReference(String tag); + + LAYOUT_ELEMENT createReplication(String unsignedInt, String body) throws Pack200Exception; + + LAYOUT_ELEMENT createUnion(String anyInt, List<UnionCaseData> cases, String body) throws Pack200Exception; + } + + /** + * Data class representing a union case in an attribute layout definition. + */ + public static final class UnionCaseData { + public final String body; + public final List<Range<Integer>> tagRanges; + + private UnionCaseData(final List<Range<Integer>> tagRanges, final String body) { + this.tagRanges = Collections.unmodifiableList(tagRanges); + this.body = body; + } + } + + private final CharSequence definition; + private final Factory<ATTRIBUTE_LAYOUT_ELEMENT, LAYOUT_ELEMENT> factory; + private int p; + + public AttributeLayoutParser(final CharSequence definition, final Factory<ATTRIBUTE_LAYOUT_ELEMENT, LAYOUT_ELEMENT> factory) { + this.definition = definition; + this.factory = factory; + } + + private void ensureNotEof() throws Pack200Exception { + if (eof()) { + throw new Pack200Exception("Incomplete attribute layout definition: " + definition); + } + } + + private boolean eof() { + return p >= definition.length(); + } + + private char expect(char... expected) throws Pack200Exception { + final char c = next(); + for (char e : expected) { + if (c == e) { + return c; + } + } + throw new Pack200Exception("Invalid attribute layout definition: expected one of " + new String(expected) + ", found '" + c + "'"); + } + + private char next() throws Pack200Exception { + ensureNotEof(); + return definition.charAt(p++); + } + + private char peek() throws Pack200Exception { + ensureNotEof(); + return definition.charAt(p); + } + + private String readAnyInt() throws Pack200Exception { + final char first = next(); + return AttributeLayoutUtils.checkAnyIntTag(first == 'S' ? "S" + next() : String.valueOf(first)); + } + + /** + * Reads the next {@code attribute_layout_element} from the stream. + * + * <pre> + * attribute_layout_element: + * ( callable | layout_element ) + * callable: + * '[' (body)? ']' + * </pre> + * + * @return next AttributeLayoutElement from the stream or {@code null} if end of stream is reached. + * @throws Pack200Exception If the layout definition is invalid. + */ + public ATTRIBUTE_LAYOUT_ELEMENT readAttributeLayoutElement() throws Pack200Exception { + if (eof()) { + return null; + } + final char first = peek(); + if (first == '[') { + try { + final String body = readBody(); + if (body.isEmpty()) { + throw new Pack200Exception("Corrupted Pack200 archive: Callable body cannot be empty."); + } + return factory.createCallable(body); + } catch (final Exception ex) { + throw new Pack200Exception(String.format("Corrupted Pack200 archive: Invalid layout '%s' at position %d.", definition, p), ex); + } + } + return readLayoutElement(); + } + + private String readBody() throws Pack200Exception { + expect('['); + int depth = 1; + final StringBuilder body = new StringBuilder(); + char c; + while (true) { + c = next(); + if (c == '[') { + depth++; + } else if (c == ']') { + depth--; + if (depth == 0) { + break; + } + } + body.append(c); + } + return body.toString(); + } + + private String readIntegralTag() throws Pack200Exception { + char c = next(); + final char[] buf = new char[3]; + int len = 0; + buf[len++] = c; + if (c == 'F' || c == 'O' || c == 'P' || c == 'S') { + c = next(); + buf[len++] = c; + if (c == 'O' || c == 'S') { + c = next(); + buf[len++] = c; + } + } + return AttributeLayoutUtils.checkIntegralTag(new String(buf, 0, len)); + } + + /** + * Reads the next {@code layout_element} from the stream. + * + * <pre> + * layout_element: + * ( integral | replication | union | call | reference ) + * </pre> + * + * @return next LayoutElement from the stream or {@code null} if end of stream is reached. + * @throws Pack200Exception If the layout definition is invalid. + */ + public LAYOUT_ELEMENT readLayoutElement() throws Pack200Exception { + if (eof()) { + return null; + } + try { + switch (peek()) { + // Integrals + case 'B': + case 'H': + case 'I': + case 'V': + case 'S': + case 'F': + case 'O': + case 'P': + return factory.createIntegral(readIntegralTag()); + // Replication + case 'N': { + next(); + final String integral = readUnsignedInt(); + final String body = readBody(); + if (body.isEmpty()) { + throw new Pack200Exception("Corrupted Pack200 archive: Replication body cannot be empty."); + } + return factory.createReplication(integral, body); + } + // Union + case 'T': { + next(); + final String anyInt = readAnyInt(); + final List<UnionCaseData> unionCases = new ArrayList<>(); + UnionCaseData data; + while ((data = readUnionCase()) != null) { + unionCases.add(data); + } + expect('('); + expect(')'); + final String body = readBody(); + return factory.createUnion(anyInt, unionCases, body); + } + // Call + case '(': { + next(); + final int callNumber = readNumeral(); + expect(')'); + return factory.createCall(callNumber); + } + // Reference + case 'K': + case 'R': { + return factory.createReference(readReferenceTag()); + } + default: { + throw new Pack200Exception("Unexpected character '" + peek() + "' in attribute layout definition."); + } + } + } catch (final Exception ex) { + throw new Pack200Exception(String.format("Corrupted Pack200 archive: Invalid layout '%s' at position %d.", definition, p), ex); + } + } + + /** + * Reads a number from the stream. + * + * <p>Stops reading at the first character, which is not a digit.</p> + * + * <p><strong>Note:</strong> there is a typo in the + * <a href="https://docs.oracle.com/en/java/javase/11/docs/specs/pack-spec.html#attribute-layout-definitions";>official {@code numeral} definition</a>. + * Parentheses should <strong>not</strong> be part of the numeral definition.</p> + * + * <pre> + * numeral: + * ('-')? (digit)+ + * </pre> + * + * @return A number from the stream. + * @throws IOException If an I/O error occurs. + */ + private int readNumeral() throws IOException { + // Determine if the number is negative + final char first = peek(); + final boolean negative = first == '-'; + if (negative) { + next(); + } + // Read the number + final char firstDigit = next(); + if (!Character.isDigit(firstDigit)) { + throw new Pack200Exception("Corrupted Pack200 archive: numeral must start with a digit."); + } + long result = firstDigit - '0'; + while (!eof()) { + if (!Character.isDigit(peek())) { + break; + } + result = result * 10 + (next() - '0'); + // Check for overflow + if (result > Integer.MAX_VALUE) { + throw new Pack200Exception("Corrupted Pack200 archive: numeral value out of range."); + } + } + return (int) (negative ? -result : result); + } + + private String readReferenceTag() throws Pack200Exception { + final char[] buf = new char[4]; + int len = 0; + buf[len++] = next(); + buf[len++] = next(); + char c = next(); + buf[len++] = c; + if (c == 'N') { + c = next(); + buf[len++] = c; + } + return AttributeLayoutUtils.checkReferenceTag(new String(buf, 0, len)); + } + + /** + * Reads a non default {@code union_case} from the stream + * + * <p>Reads a non default {@code union_case} or returns {@code null} if the default case {@code ()} is encountered.</p> + * + * <pre> + * union_case: + * '(' union_case_tag (',' union_case_tag)* ')' '[' (body)? ']' + * union_case_tag: + * ( numeral | numeral '-' numeral ) + * </pre> + * + * @return A UnionCase from the stream or {@code null} if the default case is encountered. + * @throws IOException If an I/O error occurs. + */ + private UnionCaseData readUnionCase() throws IOException { + // Check for default case + expect('('); + char c = peek(); + if (c == ')') { + // Default case + p--; + return null; + } + // Read the tag ranges + final List<Range<Integer>> tagRanges = new ArrayList<>(); + int nextTag; + Integer startTag = null; + do { + nextTag = readNumeral(); + c = expect('-', ',', ')'); + if (startTag == null) { // First number of a range + if (c == '-') { + startTag = nextTag; + } else { + tagRanges.add(Range.of(nextTag, nextTag)); + } + } else { // Second number of a range + tagRanges.add(Range.of(startTag, nextTag)); + startTag = null; + } + } while (c != ')'); + // Read the body + final String body = readBody(); + return new UnionCaseData(tagRanges, body); + } + + private String readUnsignedInt() throws Pack200Exception { + return AttributeLayoutUtils.checkUnsignedIntTag(String.valueOf(next())); + } +} diff --git a/src/main/java/org/apache/commons/compress/harmony/internal/AttributeLayoutUtils.java b/src/main/java/org/apache/commons/compress/harmony/internal/AttributeLayoutUtils.java new file mode 100644 index 000000000..bbe1d1910 --- /dev/null +++ b/src/main/java/org/apache/commons/compress/harmony/internal/AttributeLayoutUtils.java @@ -0,0 +1,252 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.commons.compress.harmony.internal; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashSet; +import java.util.List; +import java.util.Set; +import java.util.stream.Collectors; + +import org.apache.commons.compress.harmony.pack200.Pack200Exception; +import org.apache.commons.lang3.Range; + +public final class AttributeLayoutUtils { + + /** + * <pre> + * integral: + * ( unsigned_int | signed_int | bc_index | bc_offset | flag ) + * signed_int: + * 'S' uint_type + * any_int: + * ( unsigned_int | signed_int ) + * bc_index: + * ( 'P' uint_type | 'PO' uint_type ) + * bc_offset: + * 'O' any_int + * flag: + * 'F' uint_type + * uint_type: + * ( 'B' | 'H' | 'I' | 'V' ) + * </pre> + */ + private static final Set<String> INTEGRAL_TAGS; + + /** + * <pre> + * reference: + * reference_type ( 'N' )? uint_type + * reference_type: + * ( constant_ref | schema_ref | utf8_ref | untyped_ref ) + * constant_ref: + * ( 'KI' | 'KJ' | 'KF' | 'KD' | 'KS' | 'KQ' | 'KM' | 'KT' | 'KL' ) + * schema_ref: + * ( 'RC' | 'RS' | 'RD' | 'RF' | 'RM' | 'RI' | 'RY' | 'RB' | 'RN' ) + * utf8_ref: + * 'RU' + * untyped_ref: + * 'RQ' + * uint_type: + * ( 'B' | 'H' | 'I' | 'V' ) + * </pre> + */ + private static final Set<String> REFERENCE_TAGS; + + /** + * <pre> + * uint_type: + * ( 'B' | 'H' | 'I' | 'V' ) + * </pre> + */ + private static final Set<String> UNSIGNED_INT_TAGS; + + static { + final Set<String> unsignedIntTags = new HashSet<>(); + Collections.addAll(unsignedIntTags, + // unsigned_int + "B", "H", "I", "V"); + UNSIGNED_INT_TAGS = Collections.unmodifiableSet(unsignedIntTags); + final Set<String> integralTags = new HashSet<>(); + UNSIGNED_INT_TAGS.forEach(tag -> Collections.addAll(integralTags, + // unsigned_int + tag, + // signed_int + "S" + tag, + // bc_index + "P" + tag, "PO" + tag, + // bc_offset + "O" + tag, "OS" + tag, + // flag + "F" + tag)); INTEGRAL_TAGS = Collections.unmodifiableSet(integralTags); + final Set<String> referenceTags = new HashSet<>(); + Collections.addAll(referenceTags, + // constant_ref + "KI", "KJ", "KF", "KD", "KS", "KQ", "KM", "KT", "KL", + // schema_ref + "RC", "RS", "RD", "RF", "RM", "RI", "RY", "RB", "RN", + // utf8_ref + "RU", + // untyped_ref + "RQ"); + REFERENCE_TAGS = Collections.unmodifiableSet(referenceTags); + } + + /** + * Validates that the given tag matches the {@code unsigned_int} layout definition production rule. + * + * @param tag the layout tag to validate + * @return the validated tag + * @throws IllegalArgumentException if the tag is invalid + */ + static String checkAnyIntTag(final String tag) { + if (UNSIGNED_INT_TAGS.contains(tag) || tag.startsWith("S") && UNSIGNED_INT_TAGS.contains(tag.substring(1))) { + return tag; + } + throw new IllegalArgumentException("Invalid unsigned int layout tag: " + tag); + } + + /** + * Validates that the given tag matches the {@code integral} layout definition production rule. + * + * @param tag the layout tag to validate + * @return the validated tag + * @throws IllegalArgumentException if the tag is invalid + */ + public static String checkIntegralTag(final String tag) { + if (INTEGRAL_TAGS.contains(tag)) { + return tag; + } + throw new IllegalArgumentException("Invalid integral layout tag: " + tag); + } + + /** + * Validates that the given tag matches the {@code reference} layout definition production rule. + * + * @param tag the layout tag to validate + * @return the validated tag + * @throws IllegalArgumentException if the tag is invalid + */ + public static String checkReferenceTag(final String tag) { + if (tag.length() >= 3) { + final String baseTag = tag.substring(0, 2); + final String uintType = tag.substring(tag.length() - 1); + if (REFERENCE_TAGS.contains(baseTag) && UNSIGNED_INT_TAGS.contains(uintType) + && (tag.length() == 3 || tag.length() == 4 && tag.charAt(2) == 'N')) { + return tag; + } + } + throw new IllegalArgumentException("Invalid reference layout tag: " + tag); + } + + /** + * Validates that the given tag matches the {@code unsigned_int} layout definition production rule. + * + * @param tag the layout tag to validate + * @return the validated tag + * @throws IllegalArgumentException if the tag is invalid + */ + static String checkUnsignedIntTag(final String tag) { + if (UNSIGNED_INT_TAGS.contains(tag)) { + return tag; + } + throw new IllegalArgumentException("Invalid unsigned int layout tag: " + tag); + } + + /** + * Reads a {@code attribute_layout} from the stream. + * + * <p>The returned list <strong>may</strong> be empty if the stream is empty.</p> + * + * <pre> + * attribute_layout: + * ( layout_element )* | ( callable )+ + * </pre> + * + * @param definition the attribute layout definition body. + * @param factory the factory to create AttributeLayoutElements. + * @param <ALE>> the type of AttributeLayoutElement. + * @param <LE> the type of LayoutElement. + * @return not empty list of LayoutElements from the body or, if the stream is empty, an empty list. + * @throws Pack200Exception If the layout definition is invalid. + */ + public static <ALE, LE extends ALE> List<ALE> readAttributeLayout(final String definition, + final AttributeLayoutParser.Factory<ALE, LE> factory) throws Pack200Exception { + final List<ALE> layoutElements = new ArrayList<>(); + final AttributeLayoutParser<ALE, LE> parser = new AttributeLayoutParser<>(definition, factory); + ALE e; + while ((e = parser.readAttributeLayoutElement()) != null) { + layoutElements.add(e); + } + return layoutElements; + } + + /** + * Reads a {@code body} from the stream. + * + * <p>The returned list <strong>may</strong> be empty if the stream is empty.</p> + * + * <pre> + * body: + * ( layout_element )+ + * </pre> + * + * @param body the attribute layout definition body. + * @param factory the factory to create LayoutElements. + * @param <ALE>> the type of AttributeLayoutElement. + * @param <LE> the type of LayoutElement. + * @return not empty list of LayoutElements from the body or, if the stream is empty, an empty list. + * @throws Pack200Exception If the layout definition is invalid. + */ + public static <ALE, LE extends ALE> List<LE> readBody(final String body, final AttributeLayoutParser.Factory<ALE, LE> factory) throws Pack200Exception { + final List<LE> layoutElements = new ArrayList<>(); + final AttributeLayoutParser<ALE, LE> parser = new AttributeLayoutParser<>(body, factory); + LE e; + while ((e = parser.readLayoutElement()) != null) { + layoutElements.add(e); + } + return layoutElements; + } + + /** + * Converts a list of integers to a list of ranges where each range represents a single integer. + * + * @param tags the list of integer tags + * @return a list of ranges representing the tags + */ + public static List<Range<Integer>> toRanges(final List<Integer> tags) { + return tags.stream().map(n -> Range.of(n, n)).collect(Collectors.toList()); + } + + /** + * Checks if any of the given tag ranges contains the specified tag. + * + * @param tagRanges the list of tag ranges + * @param tag the tag to check + * @return {@code true} if any range contains the tag, {@code false} otherwise + */ + public static boolean unionCaseMatches(final List<Range<Integer>> tagRanges, final int tag) { + return tagRanges.stream().anyMatch(r -> r.contains(tag)); + } + + private AttributeLayoutUtils() { + // Utility class + } +} diff --git a/src/main/java/org/apache/commons/compress/harmony/archive/internal/nls/package-info.java b/src/main/java/org/apache/commons/compress/harmony/internal/package-info.java similarity index 77% copy from src/main/java/org/apache/commons/compress/harmony/archive/internal/nls/package-info.java copy to src/main/java/org/apache/commons/compress/harmony/internal/package-info.java index 672547a16..d139a4f4e 100644 --- a/src/main/java/org/apache/commons/compress/harmony/archive/internal/nls/package-info.java +++ b/src/main/java/org/apache/commons/compress/harmony/internal/package-info.java @@ -16,8 +16,11 @@ * specific language governing permissions and limitations * under the License. */ - /** - * Internal package. + * Internal implementation details. + * + * <p>This package is not exported and must not be used by external code. + * Its contents may change or be removed at any time without notice.</p> */ -package org.apache.commons.compress.harmony.archive.internal.nls; + +package org.apache.commons.compress.harmony.internal; diff --git a/src/main/java/org/apache/commons/compress/harmony/pack200/NewAttributeBands.java b/src/main/java/org/apache/commons/compress/harmony/pack200/NewAttributeBands.java index 8b32099ac..ca2680ed1 100644 --- a/src/main/java/org/apache/commons/compress/harmony/pack200/NewAttributeBands.java +++ b/src/main/java/org/apache/commons/compress/harmony/pack200/NewAttributeBands.java @@ -22,15 +22,17 @@ import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; -import java.io.StringReader; import java.io.UncheckedIOException; import java.util.ArrayList; import java.util.Collections; import java.util.List; import java.util.Map; +import org.apache.commons.compress.harmony.internal.AttributeLayoutParser; +import org.apache.commons.compress.harmony.internal.AttributeLayoutParser.UnionCaseData; +import org.apache.commons.compress.harmony.internal.AttributeLayoutUtils; import org.apache.commons.compress.harmony.pack200.AttributeDefinitionBands.AttributeDefinition; -import org.apache.commons.compress.utils.ParsingUtils; +import org.apache.commons.lang3.Range; import org.objectweb.asm.Label; /** @@ -55,6 +57,43 @@ public interface AttributeLayoutElement { } + private class AttributeLayoutFactory implements AttributeLayoutParser.Factory<AttributeLayoutElement, LayoutElement> { + + @Override + public LayoutElement createCall(int callableIndex) { + return new Call(callableIndex); + } + + @Override + public AttributeLayoutElement createCallable(String body) throws Pack200Exception { + return new Callable(AttributeLayoutUtils.readBody(body, attributeLayoutFactory)); + } + + @Override + public LayoutElement createIntegral(String tag) { + return new Integral(tag); + } + + @Override + public LayoutElement createReference(String tag) { + return new Reference(tag); + } + + @Override + public LayoutElement createReplication(String unsignedInt, String body) throws Pack200Exception { + return new Replication(unsignedInt, body); + } + + @Override + public LayoutElement createUnion(String anyInt, List<UnionCaseData> cases, String body) throws Pack200Exception { + final List<UnionCase> unionCases = new ArrayList<>(); + for (final UnionCaseData unionCaseData : cases) { + unionCases.add(new UnionCase(unionCaseData.tagRanges, AttributeLayoutUtils.readBody(unionCaseData.body, attributeLayoutFactory), false)); + } + return new Union(anyInt, unionCases, AttributeLayoutUtils.readBody(body, attributeLayoutFactory)); + } + } + public class Call extends LayoutElement { private final int callableIndex; @@ -106,7 +145,16 @@ public class Callable implements AttributeLayoutElement { private int backwardsCallableIndex; - public Callable(final List<LayoutElement> body) { + /** + * Constructs a new Callable layout element with the given body. + * + * @param body the body of the callable. + * @throws Pack200Exception If the body is empty. + */ + public Callable(final List<LayoutElement> body) throws Pack200Exception { + if (body.isEmpty()) { + throw new Pack200Exception("Corrupted Pack200 archive: Callable body is empty"); + } this.body = body; } @@ -166,13 +214,26 @@ public class Integral extends LayoutElement { private Integral previousIntegral; private int previousPValue; + /** + * Constructs a new Integral layout element with the given tag. + * + * @param tag The tag. + * @throws IllegalArgumentException If the tag is invalid. + */ public Integral(final String tag) { - this.tag = tag; + this.tag = AttributeLayoutUtils.checkIntegralTag(tag); this.defaultCodec = getCodec(tag); } + /** + * Constructs a new Integral layout element with the given tag. + * + * @param tag The tag. + * @param previousIntegral The previous integral (for PO and OS types). + * @throws IllegalArgumentException If the tag is invalid. + */ public Integral(final String tag, final Integral previousIntegral) { - this.tag = tag; + this.tag = AttributeLayoutUtils.checkIntegralTag(tag); this.defaultCodec = getCodec(tag); this.previousIntegral = previousIntegral; } @@ -320,8 +381,14 @@ public class Reference extends LayoutElement { private final boolean nullsAllowed; + /** + * Constructs a new Reference layout element with the given tag. + * + * @param tag The tag. + * @throws IllegalArgumentException If the tag is invalid. + */ public Reference(final String tag) { - this.tag = tag; + this.tag = AttributeLayoutUtils.checkReferenceTag(tag); nullsAllowed = tag.indexOf('N') != -1; } @@ -371,14 +438,21 @@ public class Replication extends LayoutElement { private final Integral countElement; - private final List<LayoutElement> layoutElements = new ArrayList<>(); + private final List<LayoutElement> layoutElements; - public Replication(final String tag, final String contents) throws IOException { + /** + * Constructs a new Replication layout element. + * + * @param tag the tag of the Integral element. + * @param contents the contents of the replication. + * @throws IllegalArgumentException If the tag is invalid or the contents are empty. + * @throws Pack200Exception If the contents are invalid. + */ + public Replication(final String tag, final String contents) throws Pack200Exception { this.countElement = new Integral(tag); - final StringReader stream = new StringReader(contents); - LayoutElement e; - while ((e = readNextLayoutElement(stream)) != null) { - layoutElements.add(e); + this.layoutElements = AttributeLayoutUtils.readBody(contents, attributeLayoutFactory); + if (layoutElements.isEmpty()) { + throw new Pack200Exception("Corrupted Pack200 archive: Replication body is empty"); } } @@ -426,6 +500,14 @@ public class Union extends LayoutElement { private final List<UnionCase> unionCases; private final List<LayoutElement> defaultCaseBody; + /** + * Constructs a new Union layout element. + * + * @param tag the tag of the Integral element. + * @param unionCases the union cases. + * @param body the default case body. + * @throws IllegalArgumentException If the tag is invalid. + */ public Union(final String tag, final List<UnionCase> unionCases, final List<LayoutElement> body) { this.unionTag = new Integral(tag); this.unionCases = unionCases; @@ -489,17 +571,19 @@ public void renumberBci(final IntList bciRenumbering, final Map<Label, Integer> */ public class UnionCase extends LayoutElement { + private final List<Range<Integer>> tagRanges; private final List<LayoutElement> body; - private final List<Integer> tags; - public UnionCase(final List<Integer> tags) { - this.tags = tags; - this.body = Collections.EMPTY_LIST; + this(tags, Collections.emptyList()); } public UnionCase(final List<Integer> tags, final List<LayoutElement> body) { - this.tags = tags; + this(AttributeLayoutUtils.toRanges(tags), body, false); + } + + private UnionCase(final List<Range<Integer>> tagRanges, final List<LayoutElement> body, final boolean ignored) { + this.tagRanges = tagRanges; this.body = body; } @@ -515,7 +599,7 @@ public List<LayoutElement> getBody() { } public boolean hasTag(final long l) { - return tags.contains(Integer.valueOf((int) l)); + return AttributeLayoutUtils.unionCaseMatches(tagRanges, (int) l); } @Override @@ -546,6 +630,8 @@ public void renumberBci(final IntList bciRenumbering, final Map<Label, Integer> // used when parsing private Integral lastPIntegral; + private final AttributeLayoutFactory attributeLayoutFactory = new AttributeLayoutFactory(); + public NewAttributeBands(final int effort, final CpBands cpBands, final SegmentHeader header, final AttributeDefinition def) throws IOException { super(effort, header); this.def = def; @@ -591,35 +677,6 @@ public int getFlagIndex() { return def.index; } - /** - * Utility method to get the contents of the given stream, up to the next {@code ]}, (ignoring pairs of brackets {@code [} and {@code ]}) - * - * @param reader - * @return - * @throws IOException If an I/O error occurs. - */ - private StringReader getStreamUpToMatchingBracket(final StringReader reader) throws IOException { - final StringBuilder sb = new StringBuilder(); - int foundBracket = -1; - while (foundBracket != 0) { - final int read = reader.read(); - if (read == -1) { - break; - } - final char c = (char) read; - if (c == ']') { - foundBracket++; - } - if (c == '[') { - foundBracket--; - } - if (!(foundBracket == 0)) { - sb.append(c); - } - } - return new StringReader(sb.toString()); - } - public boolean isUsedAtLeastOnce() { return usedAtLeastOnce; } @@ -638,32 +695,11 @@ public void pack(final OutputStream outputStream) throws IOException, Pack200Exc private void parseLayout() throws IOException { final String layout = def.layout.getUnderlyingString(); if (attributeLayoutElements == null) { - attributeLayoutElements = new ArrayList<>(); - final StringReader reader = new StringReader(layout); - AttributeLayoutElement e; - while ((e = readNextAttributeElement(reader)) != null) { - attributeLayoutElements.add(e); - } + attributeLayoutElements = AttributeLayoutUtils.readAttributeLayout(layout, attributeLayoutFactory); resolveCalls(); } } - /** - * Reads a 'body' section of the layout from the given stream - * - * @param reader - * @return List of LayoutElements - * @throws IOException If an I/O error occurs. - */ - private List<LayoutElement> readBody(final StringReader reader) throws IOException { - final List<LayoutElement> layoutElements = new ArrayList<>(); - LayoutElement e; - while ((e = readNextLayoutElement(reader)) != null) { - layoutElements.add(e); - } - return layoutElements; - } - private int readInteger(final int i, final InputStream inputStream) { int result = 0; for (int j = 0; j < i; j++) { @@ -683,199 +719,6 @@ private int readInteger(final int i, final InputStream inputStream) { return result; } - private AttributeLayoutElement readNextAttributeElement(final StringReader reader) throws IOException { - reader.mark(1); - final int next = reader.read(); - if (next == -1) { - return null; - } - if (next == '[') { - return new Callable(readBody(getStreamUpToMatchingBracket(reader))); - } - reader.reset(); - return readNextLayoutElement(reader); - } - - private LayoutElement readNextLayoutElement(final StringReader reader) throws IOException { - final int nextChar = reader.read(); - if (nextChar == -1) { - return null; - } - - switch (nextChar) { - // Integrals - case 'B': - case 'H': - case 'I': - case 'V': - return new Integral(new String(new char[] { (char) nextChar })); - case 'S': - case 'F': - return new Integral(new String(new char[] { (char) nextChar, (char) reader.read() })); - case 'P': - reader.mark(1); - if (reader.read() != 'O') { - reader.reset(); - lastPIntegral = new Integral("P" + (char) reader.read()); - return lastPIntegral; - } - lastPIntegral = new Integral("PO" + (char) reader.read(), lastPIntegral); - return lastPIntegral; - case 'O': - reader.mark(1); - if (reader.read() != 'S') { - reader.reset(); - return new Integral("O" + (char) reader.read(), lastPIntegral); - } - return new Integral("OS" + (char) reader.read(), lastPIntegral); - - // Replication - case 'N': - final char uint_type = (char) reader.read(); - reader.read(); // '[' - final String str = readUpToMatchingBracket(reader); - return new Replication("" + uint_type, str); - - // Union - case 'T': - String int_type = String.valueOf((char) reader.read()); - if (int_type.equals("S")) { - int_type += (char) reader.read(); - } - final List<UnionCase> unionCases = new ArrayList<>(); - UnionCase c; - while ((c = readNextUnionCase(reader)) != null) { - unionCases.add(c); - } - reader.read(); // '(' - reader.read(); // ')' - reader.read(); // '[' - List<LayoutElement> body = null; - reader.mark(1); - final char next = (char) reader.read(); - if (next != ']') { - reader.reset(); - body = readBody(getStreamUpToMatchingBracket(reader)); - } - return new Union(int_type, unionCases, body); - - // Call - case '(': - final int number = readNumber(reader).intValue(); - reader.read(); // ')' - return new Call(number); - // Reference - case 'K': - case 'R': - final StringBuilder string = new StringBuilder("").append((char) nextChar).append((char) reader.read()); - final char nxt = (char) reader.read(); - string.append(nxt); - if (nxt == 'N') { - string.append((char) reader.read()); - } - return new Reference(string.toString()); - } - return null; - } - - /** - * Reads a UnionCase from the stream - * - * @param reader - * @return - * @throws IOException If an I/O error occurs. - */ - private UnionCase readNextUnionCase(final StringReader reader) throws IOException { - reader.mark(2); - reader.read(); // '(' - final int next = reader.read(); - char ch = (char) next; - if (ch == ')' || next == -1) { - reader.reset(); - return null; - } - reader.reset(); - reader.read(); // '(' - final List<Integer> tags = new ArrayList<>(); - Integer nextTag; - do { - nextTag = readNumber(reader); - if (nextTag != null) { - tags.add(nextTag); - reader.read(); // ',' or ')' - } - } while (nextTag != null); - reader.read(); // '[' - reader.mark(1); - ch = (char) reader.read(); - if (ch == ']') { - return new UnionCase(tags); - } - reader.reset(); - return new UnionCase(tags, readBody(getStreamUpToMatchingBracket(reader))); - } - - /** - * Reads a number from the stream and return it - * - * @param stream - * @return - * @throws IOException If an I/O error occurs. - */ - private Integer readNumber(final StringReader stream) throws IOException { - stream.mark(1); - final char first = (char) stream.read(); - final boolean negative = first == '-'; - if (!negative) { - stream.reset(); - } - stream.mark(100); - int i; - int length = 0; - while ((i = stream.read()) != -1 && Character.isDigit((char) i)) { - length++; - } - stream.reset(); - if (length == 0) { - return null; - } - final char[] digits = new char[length]; - final int read = stream.read(digits); - if (read != digits.length) { - throw new Pack200Exception("Error reading from the input stream"); - } - return ParsingUtils.parseIntValue((negative ? "-" : "") + new String(digits)); - } - - /** - * Utility method to get the contents of the given stream, up to the next ']', (ignoring pairs of brackets '[' and ']') - * - * @param reader - * @return - * @throws IOException If an I/O error occurs. - */ - private String readUpToMatchingBracket(final StringReader reader) throws IOException { - final StringBuilder sb = new StringBuilder(); - int foundBracket = -1; - while (foundBracket != 0) { - final int read = reader.read(); - if (read == -1) { - break; - } - final char c = (char) read; - if (c == ']') { - foundBracket++; - } - if (c == '[') { - foundBracket--; - } - if (!(foundBracket == 0)) { - sb.append(c); - } - } - return sb.toString(); - } - /** * Renumber any bytecode indexes or offsets as described in section 5.5.2 of the pack200 specification * diff --git a/src/main/java/org/apache/commons/compress/harmony/unpack200/NewAttributeBands.java b/src/main/java/org/apache/commons/compress/harmony/unpack200/NewAttributeBands.java index a6bd2d333..7dc2a18b9 100644 --- a/src/main/java/org/apache/commons/compress/harmony/unpack200/NewAttributeBands.java +++ b/src/main/java/org/apache/commons/compress/harmony/unpack200/NewAttributeBands.java @@ -20,11 +20,13 @@ import java.io.IOException; import java.io.InputStream; -import java.io.StringReader; import java.util.ArrayList; import java.util.Collections; import java.util.List; +import org.apache.commons.compress.harmony.internal.AttributeLayoutParser; +import org.apache.commons.compress.harmony.internal.AttributeLayoutParser.UnionCaseData; +import org.apache.commons.compress.harmony.internal.AttributeLayoutUtils; import org.apache.commons.compress.harmony.pack200.BHSDCodec; import org.apache.commons.compress.harmony.pack200.Codec; import org.apache.commons.compress.harmony.pack200.Pack200Exception; @@ -41,7 +43,7 @@ import org.apache.commons.compress.harmony.unpack200.bytecode.CPString; import org.apache.commons.compress.harmony.unpack200.bytecode.CPUTF8; import org.apache.commons.compress.harmony.unpack200.bytecode.NewAttribute; -import org.apache.commons.compress.utils.ParsingUtils; +import org.apache.commons.lang3.Range; /** * Sets of bands relating to a non-predefined attribute @@ -76,6 +78,43 @@ private interface AttributeLayoutElement { } + private class AttributeLayoutFactory implements AttributeLayoutParser.Factory<AttributeLayoutElement, LayoutElement> { + + @Override + public LayoutElement createCall(int callableIndex) { + return new Call(callableIndex); + } + + @Override + public AttributeLayoutElement createCallable(String body) throws Pack200Exception { + return new Callable(AttributeLayoutUtils.readBody(body, attributeLayoutFactory)); + } + + @Override + public LayoutElement createIntegral(String tag) { + return new Integral(tag); + } + + @Override + public LayoutElement createReference(String tag) { + return new Reference(tag); + } + + @Override + public LayoutElement createReplication(String unsignedInt, String body) throws Pack200Exception { + return new Replication(unsignedInt, body); + } + + @Override + public LayoutElement createUnion(String anyInt, List<UnionCaseData> cases, String body) throws Pack200Exception { + final List<UnionCase> unionCases = new ArrayList<>(); + for (final UnionCaseData unionCaseData : cases) { + unionCases.add(new UnionCase(unionCaseData.tagRanges, AttributeLayoutUtils.readBody(unionCaseData.body, attributeLayoutFactory), false)); + } + return new Union(anyInt, unionCases, AttributeLayoutUtils.readBody(body, attributeLayoutFactory)); + } + } + public class Call extends LayoutElement { private final int callableIndex; @@ -129,7 +168,16 @@ public static class Callable implements AttributeLayoutElement { private int index; - public Callable(final List<LayoutElement> body) { + /** + * Constructs a new Callable layout element with the given body. + * + * @param body the body of the callable. + * @throws Pack200Exception If the body is empty. + */ + public Callable(final List<LayoutElement> body) throws Pack200Exception { + if (body.isEmpty()) { + throw new Pack200Exception("Corrupted Pack200 archive: Callable body is empty"); + } this.body = body; } @@ -203,8 +251,14 @@ public class Integral extends LayoutElement { private int[] band; + /** + * Constructs a new Integral layout element with the given tag. + * + * @param tag The tag. + * @throws IllegalArgumentException If the tag is invalid. + */ public Integral(final String tag) { - this.tag = tag; + this.tag = AttributeLayoutUtils.checkIntegralTag(tag); } @Override @@ -317,8 +371,14 @@ public class Reference extends LayoutElement { private final int length; + /** + * Constructs a new Reference layout element with the given tag. + * + * @param tag The tag. + * @throws IllegalArgumentException If the tag is invalid. + */ public Reference(final String tag) { - this.tag = tag; + this.tag = AttributeLayoutUtils.checkReferenceTag(tag); length = getLength(tag.charAt(tag.length() - 1)); } @@ -393,14 +453,21 @@ public class Replication extends LayoutElement { private final Integral countElement; - private final List<LayoutElement> layoutElements = new ArrayList<>(); + private final List<LayoutElement> layoutElements; - public Replication(final String tag, final String contents) throws IOException { + /** + * Constructs a new Replication layout element. + * + * @param tag the tag of the Integral element. + * @param contents the contents of the replication. + * @throws IllegalArgumentException If the tag is invalid or the contents are empty. + * @throws Pack200Exception If the contents are invalid. + */ + public Replication(final String tag, final String contents) throws Pack200Exception { this.countElement = new Integral(tag); - final StringReader stream = new StringReader(contents); - LayoutElement e; - while ((e = readNextLayoutElement(stream)) != null) { - layoutElements.add(e); + this.layoutElements = AttributeLayoutUtils.readBody(contents, attributeLayoutFactory); + if (layoutElements.isEmpty()) { + throw new Pack200Exception("Corrupted Pack200 archive: Replication body is empty"); } } @@ -454,6 +521,14 @@ public class Union extends LayoutElement { private int[] caseCounts; private int defaultCount; + /** + * Constructs a new Union layout element. + * + * @param tag the tag of the Integral element. + * @param unionCases the union cases. + * @param body the default case body. + * @throws IllegalArgumentException If the tag is invalid. + */ public Union(final String tag, final List<UnionCase> unionCases, final List<LayoutElement> body) { this.unionTag = new Integral(tag); this.unionCases = unionCases; @@ -553,56 +628,58 @@ public void readBands(final InputStream in, final int count) throws IOException, */ public class UnionCase extends LayoutElement { - private List<LayoutElement> body; + private final List<Range<Integer>> tagRanges; + private final List<LayoutElement> body; - private final List<Integer> tags; + public UnionCase(final List<Integer> tags) throws IOException { + this(tags, Collections.emptyList()); + } - public UnionCase(final List<Integer> tags) { - this.tags = tags; + public UnionCase(final List<Integer> tags, final List<LayoutElement> body) throws IOException { + this(AttributeLayoutUtils.toRanges(tags), body, false); } - public UnionCase(final List<Integer> tags, final List<LayoutElement> body) { - this.tags = tags; + private UnionCase(final List<Range<Integer>> tagRanges, final List<LayoutElement> body, final boolean ignored) { + this.tagRanges = tagRanges; this.body = body; } @Override public void addToAttribute(final int index, final NewAttribute attribute) { - if (body != null) { - for (final LayoutElement element : body) { - element.addToAttribute(index, attribute); - } + for (final LayoutElement element : body) { + element.addToAttribute(index, attribute); } } public List<LayoutElement> getBody() { - return body == null ? Collections.EMPTY_LIST : body; + return body; } public boolean hasTag(final int i) { - return tags.contains(Integer.valueOf(i)); + return AttributeLayoutUtils.unionCaseMatches(tagRanges, i); } public boolean hasTag(final long l) { - return tags.contains(Integer.valueOf((int) l)); + return hasTag((int) l); } @Override public void readBands(final InputStream in, final int count) throws IOException, Pack200Exception { - if (body != null) { - for (final LayoutElement element : body) { - element.readBands(in, count); - } + for (final LayoutElement element : body) { + element.readBands(in, count); } } } + private final AttributeLayout attributeLayout; private int backwardsCallCount; protected List<AttributeLayoutElement> attributeLayoutElements; + private final AttributeLayoutFactory attributeLayoutFactory = new AttributeLayoutFactory(); + public NewAttributeBands(final Segment segment, final AttributeLayout attributeLayout) throws IOException { super(segment); this.attributeLayout = attributeLayout; @@ -652,35 +729,6 @@ private Attribute getOneAttribute(final int index, final List<AttributeLayoutEle return attribute; } - /** - * Utility method to get the contents of the given stream, up to the next {@code ]}, (ignoring pairs of brackets {@code [} and {@code ]}) - * - * @param stream - * @return - * @throws IOException If an I/O error occurs. - */ - private StringReader getStreamUpToMatchingBracket(final StringReader stream) throws IOException { - final StringBuilder sb = new StringBuilder(); - int foundBracket = -1; - while (foundBracket != 0) { - final int read = stream.read(); - if (read == -1) { - break; - } - final char c = (char) read; - if (c == ']') { - foundBracket++; - } - if (c == '[') { - foundBracket--; - } - if (!(foundBracket == 0)) { - sb.append(c); - } - } - return new StringReader(sb.toString()); - } - /** * Parse the bands relating to this AttributeLayout and return the correct class file attributes as a List of {@link Attribute}. * @@ -709,12 +757,7 @@ public List<Attribute> parseAttributes(final InputStream in, final int occurrenc */ private void parseLayout() throws IOException { if (attributeLayoutElements == null) { - attributeLayoutElements = new ArrayList<>(); - final StringReader stream = new StringReader(attributeLayout.getLayout()); - AttributeLayoutElement e; - while ((e = readNextAttributeElement(stream)) != null) { - attributeLayoutElements.add(e); - } + attributeLayoutElements = AttributeLayoutUtils.readAttributeLayout(attributeLayout.getLayout(), attributeLayoutFactory); resolveCalls(); } } @@ -729,212 +772,6 @@ public void read(final InputStream in) throws IOException, Pack200Exception { // does nothing - use parseAttributes instead } - /** - * Reads a 'body' section of the layout from the given stream - * - * @param stream - * @return List of LayoutElements - * @throws IOException If an I/O error occurs. - */ - private List<LayoutElement> readBody(final StringReader stream) throws IOException { - final List<LayoutElement> layoutElements = new ArrayList<>(); - LayoutElement e; - while ((e = readNextLayoutElement(stream)) != null) { - layoutElements.add(e); - } - return layoutElements; - } - - private AttributeLayoutElement readNextAttributeElement(final StringReader stream) throws IOException { - stream.mark(1); - final int next = stream.read(); - if (next == -1) { - return null; - } - if (next == '[') { - return new Callable(readBody(getStreamUpToMatchingBracket(stream))); - } - stream.reset(); - return readNextLayoutElement(stream); - } - - private LayoutElement readNextLayoutElement(final StringReader stream) throws IOException { - final int nextChar = stream.read(); - if (nextChar == -1) { - return null; - } - switch (nextChar) { - // Integrals - case 'B': - case 'H': - case 'I': - case 'V': - return new Integral(new String(new char[] { (char) nextChar })); - case 'S': - case 'F': - return new Integral(new String(new char[] { (char) nextChar, (char) stream.read() })); - case 'P': - stream.mark(1); - if (stream.read() != 'O') { - stream.reset(); - return new Integral("P" + (char) stream.read()); - } - return new Integral("PO" + (char) stream.read()); - case 'O': - stream.mark(1); - if (stream.read() != 'S') { - stream.reset(); - return new Integral("O" + (char) stream.read()); - } - return new Integral("OS" + (char) stream.read()); - - // Replication - case 'N': - final char uintType = (char) stream.read(); - stream.read(); // '[' - final String str = readUpToMatchingBracket(stream); - return new Replication("" + uintType, str); - - // Union - case 'T': - String intType = "" + (char) stream.read(); - if (intType.equals("S")) { - intType += (char) stream.read(); - } - final List<UnionCase> unionCases = new ArrayList<>(); - UnionCase c; - while ((c = readNextUnionCase(stream)) != null) { - unionCases.add(c); - } - stream.read(); // '(' - stream.read(); // ')' - stream.read(); // '[' - List<LayoutElement> body = null; - stream.mark(1); - final char next = (char) stream.read(); - if (next != ']') { - stream.reset(); - body = readBody(getStreamUpToMatchingBracket(stream)); - } - return new Union(intType, unionCases, body); - - // Call - case '(': - final int number = readNumber(stream).intValue(); - stream.read(); // ')' - return new Call(number); - // Reference - case 'K': - case 'R': - final StringBuilder string = new StringBuilder("").append((char) nextChar).append((char) stream.read()); - final char nxt = (char) stream.read(); - string.append(nxt); - if (nxt == 'N') { - string.append((char) stream.read()); - } - return new Reference(string.toString()); - } - return null; - } - - /** - * Reads a UnionCase from the stream. - * - * @param stream source stream. - * @return A UnionCase from the stream. - * @throws IOException If an I/O error occurs. - */ - private UnionCase readNextUnionCase(final StringReader stream) throws IOException { - stream.mark(2); - stream.read(); // '(' - final int next = stream.read(); - char ch = (char) next; - if (ch == ')' || next == -1) { - stream.reset(); - return null; - } - stream.reset(); - stream.read(); // '(' - final List<Integer> tags = new ArrayList<>(); - Integer nextTag; - do { - nextTag = readNumber(stream); - if (nextTag != null) { - tags.add(nextTag); - stream.read(); // ',' or ')' - } - } while (nextTag != null); - stream.read(); // '[' - stream.mark(1); - ch = (char) stream.read(); - if (ch == ']') { - return new UnionCase(tags); - } - stream.reset(); - return new UnionCase(tags, readBody(getStreamUpToMatchingBracket(stream))); - } - - /** - * Reads a number from the stream and return it - * - * @param stream - * @return - * @throws IOException If an I/O error occurs. - */ - private Integer readNumber(final StringReader stream) throws IOException { - stream.mark(1); - final char first = (char) stream.read(); - final boolean negative = first == '-'; - if (!negative) { - stream.reset(); - } - stream.mark(100); - int i; - int length = 0; - while ((i = stream.read()) != -1 && Character.isDigit((char) i)) { - length++; - } - stream.reset(); - if (length == 0) { - return null; - } - final char[] digits = new char[length]; - final int read = stream.read(digits); - if (read != digits.length) { - throw new Pack200Exception("Error reading from the input stream"); - } - return ParsingUtils.parseIntValue((negative ? "-" : "") + new String(digits)); - } - - /** - * Gets the contents of the given stream, up to the next {@code ]}, (ignoring pairs of brackets {@code [} and {@code ]}) - * - * @param stream input stream. - * @return the contents of the given stream. - * @throws IOException If an I/O error occurs. - */ - private String readUpToMatchingBracket(final StringReader stream) throws IOException { - final StringBuilder sb = new StringBuilder(); - int foundBracket = -1; - while (foundBracket != 0) { - final int read = stream.read(); - if (read == -1) { - break; - } - final char c = (char) read; - if (c == ']') { - foundBracket++; - } - if (c == '[') { - foundBracket--; - } - if (!(foundBracket == 0)) { - sb.append(c); - } - } - return sb.toString(); - } - /** * Resolve calls in the attribute layout and returns the number of backwards calls */ diff --git a/src/test/java/org/apache/commons/compress/harmony/internal/AttributeLayoutParserTest.java b/src/test/java/org/apache/commons/compress/harmony/internal/AttributeLayoutParserTest.java new file mode 100644 index 000000000..1b9ff8070 --- /dev/null +++ b/src/test/java/org/apache/commons/compress/harmony/internal/AttributeLayoutParserTest.java @@ -0,0 +1,149 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.commons.compress.harmony.internal; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.List; +import java.util.stream.Stream; + +import org.apache.commons.compress.harmony.pack200.Pack200Exception; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; +import org.junit.jupiter.params.provider.ValueSource; + +public class AttributeLayoutParserTest { + + private static class AttributeLayoutElement { + private AttributeLayoutElement() { + } + + private AttributeLayoutElement(String body) { + } + } + + private static class AttributeLayoutFactory implements AttributeLayoutParser.Factory<AttributeLayoutElement, LayoutElement> { + + @Override + public LayoutElement createCall(int callableIndex) { + return new LayoutElement(callableIndex); + } + + @Override + public AttributeLayoutElement createCallable(String body) throws Pack200Exception { + return new AttributeLayoutElement(body); + } + + @Override + public LayoutElement createIntegral(String tag) { + return new LayoutElement(tag); + } + + @Override + public LayoutElement createReference(String tag) { + return new LayoutElement(tag); + } + + @Override + public LayoutElement createReplication(String unsignedInt, String body) throws Pack200Exception { + return new LayoutElement(unsignedInt, body); + } + + @Override + public LayoutElement createUnion(String anyInt, List<AttributeLayoutParser.UnionCaseData> cases, String body) throws Pack200Exception { + return new LayoutElement(anyInt, cases, body); + } + } + + private static class LayoutElement extends AttributeLayoutElement { + private LayoutElement(int callableIndex) { + } + + private LayoutElement(String body) { + } + + private LayoutElement(String anyInt, List<AttributeLayoutParser.UnionCaseData> cases, String body) { + } + + private LayoutElement(String unsignedInt, String body) { + } + } + + private static final AttributeLayoutFactory FACTORY = new AttributeLayoutFactory(); + + static Stream<String> validCallableLayouts() { + return Stream.of( + // Valid callable layouts + "[SIK]", "[NI[SIK]O]"); + } + + static Stream<String> validLayouts() { + return Stream.of( + // Valid integral layouts + "B", "H", "I", "V", "SI", "PI", "OI", "FI", "OSI", "POI", + // Valid replication layouts + "NI[I]", "NI[SI]", "NI[NI[I]]", + // Valid reference layouts + "KII", "KINI", "RSI", "RUH", "RQB", + // Valid union layouts + "TI()[]", "TSI()[]", "TI(1)[]()[]", "TI(1-3)[](4)[]()[]", "TI(1-3,5)[](4,6)[]()[]", + // Valid call layouts + "(1)", "(-2)"); + } + + @ParameterizedTest + @ValueSource(strings = { + // Invalid letters + ")", "*", "+", ",", "-", ".", "/", "0", "1", "2", "3", "4", "5", "6", "7", "8", "9", ":", ";", "<", "=", ">", "?", "@", "A", "C", "D", "E", "F", + "G", "J", "L", "M", "N", "Q", "U", + // Invalid integral layouts + "S", "SA", "P", "PA", "PO", "POA", "O", "OA", "F", "FA", + // Invalid replication layouts + "N", "NSI", "NIA", "NI[I", "NI[]", + // Invalid reference layouts + "K", "KA", "KI", "KIA", "KIN", "KINA", + // Invalid callable layouts + "[", "[]", + // Invalid union layouts + "T", "TA", "TI", "TSA", "TI(A)[]()[]", + // Invalid call layouts + "()", "(A)", "(-A)", "(9999999999999999999999)", "(1234"}) + void testInvalidLayout(String layout) { + final AttributeLayoutParser<AttributeLayoutElement, LayoutElement> parser = new AttributeLayoutParser<>(layout, FACTORY); + final Pack200Exception ex = assertThrows(Pack200Exception.class, () -> parser.readAttributeLayoutElement()); + assertTrue(ex.getMessage().contains("Corrupted Pack200 archive"), "Unexpected exception message: " + ex.getMessage()); + assertTrue(ex.getMessage().contains(layout), "Unexpected exception message: " + ex.getMessage()); + } + + @ParameterizedTest + @MethodSource({"validLayouts", "validCallableLayouts"}) + void testReadAttributeLayoutElement(String layout) throws Pack200Exception { + final List<AttributeLayoutElement> result = AttributeLayoutUtils.readAttributeLayout(layout, FACTORY); + assertFalse(result.isEmpty(), "Expected at least one LayoutElement for layout: " + layout); + } + + @ParameterizedTest + @MethodSource("validLayouts") + void testReadBody(String layout) throws Pack200Exception { + final List<LayoutElement> result = AttributeLayoutUtils.readBody(layout, FACTORY); + assertFalse(result.isEmpty(), "Expected at least one LayoutElement for layout: " + layout); + } +} diff --git a/src/test/java/org/apache/commons/compress/harmony/internal/AttributeLayoutUtilsTest.java b/src/test/java/org/apache/commons/compress/harmony/internal/AttributeLayoutUtilsTest.java new file mode 100644 index 000000000..66dcef70e --- /dev/null +++ b/src/test/java/org/apache/commons/compress/harmony/internal/AttributeLayoutUtilsTest.java @@ -0,0 +1,79 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.commons.compress.harmony.internal; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.Arrays; +import java.util.List; + +import org.apache.commons.lang3.Range; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +class AttributeLayoutUtilsTest { + + @ParameterizedTest + @ValueSource(strings = { + // Invalid reference_type + "AB", "ABI", "CD", "EF", + // Missing uint_type + "KI", "RC", "RU", "KIN", "RCN", "RUN", + // Extra characters + "KIBB", "RCBB", "RUNNH"}) + void testCheckReferenceTag_Invalid(String tag) { + assertThrows(IllegalArgumentException.class, () -> { + AttributeLayoutUtils.checkReferenceTag(tag); + }); + } + + @ParameterizedTest + @ValueSource(strings = {"KIB", "KJH", "KFI", "KDV", "KSB", "KQV", "KMI", "KTI", "KLV", "RCB", "RCH", "RDI", "RFV", "RMI", "RII", "RYB", "RBNI"}) + void testCheckReferenceTag_Valid(String tag) { + assertEquals(tag, AttributeLayoutUtils.checkReferenceTag(tag)); + } + + @Test + void testToRanges() { + final List<Integer> integers = Arrays.asList(1, 2, 3, 4, 5, 6, 7, 8, 9); + final List<Range<Integer>> ranges = AttributeLayoutUtils.toRanges(integers); + assertEquals(ranges.size(), integers.size()); + for (int i = 0; i < ranges.size(); i++) { + assertEquals(Range.of(integers.get(i), integers.get(i)), ranges.get(i), "Range at index " + i + " should be a single value range."); + } + } + + @Test + void testUnionCaseMatches() { + final List<Range<Integer>> ranges = Arrays.asList(Range.of(1, 2), Range.of(4, 5), Range.of(7, 8)); + assertTrue(AttributeLayoutUtils.unionCaseMatches(ranges, 1)); + assertTrue(AttributeLayoutUtils.unionCaseMatches(ranges, 2)); + assertFalse(AttributeLayoutUtils.unionCaseMatches(ranges, 3)); + assertTrue(AttributeLayoutUtils.unionCaseMatches(ranges, 4)); + assertTrue(AttributeLayoutUtils.unionCaseMatches(ranges, 5)); + assertFalse(AttributeLayoutUtils.unionCaseMatches(ranges, 6)); + assertTrue(AttributeLayoutUtils.unionCaseMatches(ranges, 7)); + assertTrue(AttributeLayoutUtils.unionCaseMatches(ranges, 8)); + assertFalse(AttributeLayoutUtils.unionCaseMatches(ranges, 9)); + } +} diff --git a/src/test/java/org/apache/commons/compress/harmony/pack200/Compress626Test.java b/src/test/java/org/apache/commons/compress/harmony/pack200/Compress626Test.java index 8422b7d77..62c50a6ac 100644 --- a/src/test/java/org/apache/commons/compress/harmony/pack200/Compress626Test.java +++ b/src/test/java/org/apache/commons/compress/harmony/pack200/Compress626Test.java @@ -19,7 +19,7 @@ package org.apache.commons.compress.harmony.pack200; -import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertThrows; import java.io.IOException; import java.io.InputStream; @@ -37,7 +37,7 @@ class Compress626Test { void test() throws Exception { final CPUTF8 name = new CPUTF8(""); final CPUTF8 layout = new CPUTF8("["); - assertDoesNotThrow(() -> new NewAttributeBands(1, null, null, + assertThrows(Pack200Exception.class, () -> new NewAttributeBands(1, null, null, new AttributeDefinitionBands.AttributeDefinition(35, AttributeDefinitionBands.CONTEXT_CLASS, name, layout))); } @@ -45,7 +45,7 @@ void test() throws Exception { void testJar() throws IOException { try (InputStream inputStream = Files.newInputStream(Paths.get("src/test/resources/org/apache/commons/compress/COMPRESS-626/compress-626-pack200.jar")); JarOutputStream out = new JarOutputStream(NullOutputStream.INSTANCE);) { - Pack200.newUnpacker().unpack(inputStream, out); + assertThrows(Pack200Exception.class, () -> Pack200.newUnpacker().unpack(inputStream, out)); } } } diff --git a/src/test/java/org/apache/commons/compress/harmony/pack200/Compress628Test.java b/src/test/java/org/apache/commons/compress/harmony/pack200/Compress628Test.java index d8ed03fed..c777ff418 100644 --- a/src/test/java/org/apache/commons/compress/harmony/pack200/Compress628Test.java +++ b/src/test/java/org/apache/commons/compress/harmony/pack200/Compress628Test.java @@ -19,7 +19,7 @@ package org.apache.commons.compress.harmony.pack200; -import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertThrows; import org.junit.jupiter.api.Test; @@ -29,8 +29,8 @@ class Compress628Test { void test() throws Exception { final CPUTF8 name = new CPUTF8(""); final CPUTF8 layout = new CPUTF8("Re\\T"); - assertDoesNotThrow(() -> new NewAttributeBands(1, null, null, - new AttributeDefinitionBands.AttributeDefinition(35, AttributeDefinitionBands.CONTEXT_CLASS, name, layout))); + assertThrows(Pack200Exception.class, () -> new NewAttributeBands(1, null, null, new AttributeDefinitionBands.AttributeDefinition(35, + AttributeDefinitionBands.CONTEXT_CLASS, name, layout))); } } diff --git a/src/test/java/org/apache/commons/compress/harmony/pack200/NewAttributeBandsTest.java b/src/test/java/org/apache/commons/compress/harmony/pack200/NewAttributeBandsTest.java index 5f2d2c808..5c83d6899 100644 --- a/src/test/java/org/apache/commons/compress/harmony/pack200/NewAttributeBandsTest.java +++ b/src/test/java/org/apache/commons/compress/harmony/pack200/NewAttributeBandsTest.java @@ -183,7 +183,7 @@ void testLayoutWithCalls() throws IOException { final CPUTF8 name = new CPUTF8("TestAttribute"); // @formatter:off final CPUTF8 layout = new CPUTF8( - "[NH[(1)]][RSH NH[RUH(1)]][TB(66,67,73,83,90)[KIH](68)[KDH](70)[KFH](74)[KJH](99)[RSH](101)[RSH RUH](115)[RUH](91)[NH[(0)]](64)[RSH[RUH(0)]]()[]]" + "[NH[(1)]][RSHNH[RUH(1)]][TB(66,67,73,83,90)[KIH](68)[KDH](70)[KFH](74)[KJH](99)[RSH](101)[RSHRUH](115)[RUH](91)[NH[(0)]](64)[RSHNH[RUH(0)]]()[]]" ); // @formatter:on final MockNewAttributeBands newAttributeBands = new MockNewAttributeBands(1, null, null, diff --git a/src/test/java/org/apache/commons/compress/harmony/unpack200/NewAttributeBandsTest.java b/src/test/java/org/apache/commons/compress/harmony/unpack200/NewAttributeBandsTest.java index 1e9deef33..9d02a7244 100644 --- a/src/test/java/org/apache/commons/compress/harmony/unpack200/NewAttributeBandsTest.java +++ b/src/test/java/org/apache/commons/compress/harmony/unpack200/NewAttributeBandsTest.java @@ -20,6 +20,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import java.io.IOException; @@ -53,6 +54,11 @@ public List<?> getLayoutElements() { } } + private MockNewAttributeBands createNewAttributeBands(final String layoutStr) throws IOException, Pack200Exception { + return new MockNewAttributeBands(new MockSegment(), + new AttributeLayout("test", AttributeLayout.CONTEXT_CLASS, layoutStr, 25)); + } + @Test void testEmptyLayout() throws IOException, Pack200Exception { final MockNewAttributeBands newAttributeBands = new MockNewAttributeBands(new MockSegment(), @@ -62,7 +68,17 @@ void testEmptyLayout() throws IOException, Pack200Exception { } @ParameterizedTest - @ValueSource(strings = { "B", "FB", "SB", "H", "FH", "SH", "I", "FI", "SI", "PB", "OB", "OSB", "POB", "PH", "OH", "OSH", "POH", "PI", "OI", "OSI", "POI" }) + @ValueSource(strings = { + // unsigned_int + "B", "H", "I", "V", + // signed_int + "SB", "SH", "SI", "SV", + // bc_index + "PB", "PH", "PI", "PV", "POB", "POH", "POI", "POV", + // bc_offset + "OB", "OH", "OI", "OV", + // flag + "FB", "FH", "FI", "FV"}) void testIntegralLayout(final String layoutStr) throws IOException, Pack200Exception { final MockNewAttributeBands newAttributeBands = new MockNewAttributeBands(new MockSegment(), new AttributeLayout("test", AttributeLayout.CONTEXT_CLASS, layoutStr, 25)); @@ -125,7 +141,7 @@ void testLayoutWithBackwardsCall() throws IOException, Pack200Exception { void testLayoutWithCalls() throws IOException, Pack200Exception { // @formatter:off final MockNewAttributeBands newAttributeBands = new MockNewAttributeBands(new MockSegment(), new AttributeLayout("test", AttributeLayout.CONTEXT_FIELD, - "[NH[(1)]][RSH NH[RUH(1)]][TB(66,67,73,83,90)[KIH](68)[KDH](70)[KFH](74)[KJH](99)[RSH](101)[RSH RUH](115)[RUH](91)[NH[(0)]](64)[RSH[RUH(0)]]()[]]", + "[NH[(1)]][RSHNH[RUH(1)]][TB(66,67,73,83,90)[KIH](68)[KDH](70)[KFH](74)[KJH](99)[RSH](101)[RSHRUH](115)[RUH](91)[NH[(0)]](64)[RSHNH[RUH(0)]]()[]]", 26)); // @formatter:on final List layoutElements = newAttributeBands.getLayoutElements(); @@ -180,6 +196,15 @@ void testReplicationLayout() throws IOException, Pack200Exception { assertEquals("H", fifthElement.getTag()); } + @ParameterizedTest + @ValueSource(strings = {"NH[]", "[]"}) + void testEmptyBodyFails(final String layout) { + final Pack200Exception ex = assertThrows(Pack200Exception.class, () -> createNewAttributeBands(layout)); + assertTrue(ex.getMessage().contains(layout), "Unexpected exception message: " + ex.getMessage()); + final Throwable cause = ex.getCause(); + assertTrue(cause.getMessage().contains("empty"), "Unexpected exception message: " + cause.getMessage()); + } + @Test void testUnionLayout() throws IOException, Pack200Exception { final MockNewAttributeBands newAttributeBands = new MockNewAttributeBands(new MockSegment(),
