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

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

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


##########
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:
   Use `final` where you can.



##########
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:
   Throwing `RuntimeException` is an anti-pattern IMO, using `IllegalArgumentException` or `IllegalStateException` would be better.



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