You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by GitBox <gi...@apache.org> on 2022/09/20 11:36:50 UTC
[GitHub] [commons-bcel] rjatkins opened a new pull request, #147: BCEL-363 Enforce MAX_CP_ENTRIES in ConstantPoolGen and ConstantPool.dump
rjatkins opened a new pull request, #147:
URL: https://github.com/apache/commons-bcel/pull/147
Reapplies the fix in https://github.com/openjdk/jdk11u/commit/13bf52c8d876528a43be7cb77a1f452d29a21492 but using the Const constant for the constant pool size limit.
--
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
[GitHub] [commons-bcel] garydgregory commented on a diff in pull request #147: BCEL-363 Enforce MAX_CP_ENTRIES in ConstantPoolGen and ConstantPool.dump
Posted by 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
[GitHub] [commons-bcel] rjatkins commented on pull request #147: BCEL-363 Enforce MAX_CP_ENTRIES in ConstantPoolGen and ConstantPool.dump
Posted by GitBox <gi...@apache.org>.
rjatkins commented on PR #147:
URL: https://github.com/apache/commons-bcel/pull/147#issuecomment-1259025705
@garydgregory is there any chance of a release with this fix in it soon? I'd like to use it to patch xalan, and I'd rather not have to release a patched version of BCEL to do it.
--
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
[GitHub] [commons-bcel] rjatkins commented on pull request #147: BCEL-363 Enforce MAX_CP_ENTRIES in ConstantPoolGen and ConstantPool.dump
Posted by GitBox <gi...@apache.org>.
rjatkins commented on PR #147:
URL: https://github.com/apache/commons-bcel/pull/147#issuecomment-1252348768
I've added the requested test coverage. Let me know if you need further changes
--
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
[GitHub] [commons-bcel] rjatkins commented on a diff in pull request #147: BCEL-363 Enforce MAX_CP_ENTRIES in ConstantPoolGen and ConstantPool.dump
Posted by 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
[GitHub] [commons-bcel] garydgregory merged pull request #147: BCEL-363 Enforce MAX_CP_ENTRIES in ConstantPoolGen and ConstantPool.dump
Posted by GitBox <gi...@apache.org>.
garydgregory merged PR #147:
URL: https://github.com/apache/commons-bcel/pull/147
--
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
[GitHub] [commons-bcel] garydgregory commented on pull request #147: BCEL-363 Enforce MAX_CP_ENTRIES in ConstantPoolGen and ConstantPool.dump
Posted by GitBox <gi...@apache.org>.
garydgregory commented on PR #147:
URL: https://github.com/apache/commons-bcel/pull/147#issuecomment-1259339076
@rjatkins
Sure, I'll look into it later this week or this weekend.
--
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
[GitHub] [commons-bcel] garydgregory commented on pull request #147: BCEL-363 Enforce MAX_CP_ENTRIES in ConstantPoolGen and ConstantPool.dump
Posted by GitBox <gi...@apache.org>.
garydgregory commented on PR #147:
URL: https://github.com/apache/commons-bcel/pull/147#issuecomment-1252285218
https://issues.apache.org/jira/browse/BCEL-363
--
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
[GitHub] [commons-bcel] garydgregory commented on pull request #147: BCEL-363 Enforce MAX_CP_ENTRIES in ConstantPoolGen and ConstantPool.dump
Posted by GitBox <gi...@apache.org>.
garydgregory commented on PR #147:
URL: https://github.com/apache/commons-bcel/pull/147#issuecomment-1253700160
@rjatkins merged, ty for your PR!
--
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