[GitHub] [commons-bcel] rjatkins commented on a diff in pull request #147: BCEL-363 Enforce MAX_CP_ENTRIES in ConstantPoolGen and ConstantPool.dump

[GitBox <gi...@apache.org>]

rjatkins commented on code in PR #147:
URL: https://github.com/apache/commons-bcel/pull/147#discussion_r975354533


##########
src/main/java/org/apache/bcel/generic/ConstantPoolGen.java:
##########
@@ -561,9 +561,18 @@ public int addUtf8(final String n) {
      * Resize internal array of constants.
      */
     protected void adjustSize() {
+        // 3 extra spaces are needed as some entries may take 3 slots
+        if (index + 3 >= Const.MAX_CP_ENTRIES + 1) {
+            throw new RuntimeException("The number of constants " + (index + 3)

Review Comment:
   Since we don't have any arguments to check here, I've opted to throw IllegalStateException. This exception seems appropriate, since we would otherwise violate the invariant that the ConstantPoolGen.constants is a valid constant pool.



##########
src/main/java/org/apache/bcel/classfile/ConstantPool.java:
##########
@@ -230,8 +230,15 @@ public ConstantPool copy() {
      * @throws IOException if problem in writeShort or dump
      */
     public void dump(final DataOutputStream file) throws IOException {
-        file.writeShort(constantPool.length);
-        for (int i = 1; i < constantPool.length; i++) {
+        /*
+         * Constants over the size of the constant pool shall not be written out.
+         * This is a redundant measure as the ConstantPoolGen should have already
+         * reported an error back in the situation.
+        */
+        int size = Math.min(constantPool.length, Const.MAX_CP_ENTRIES);

Review Comment:
   Done



-- 
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