Update README.md new mvn clean command (IEP-1560) #1244
Conversation
Before command not working, updated to: mvn clean verify -Djarsigner.skip=true \ -Djdk.xml.maxGeneralEntitySizeLimit=0 \ -Djdk.xml.totalEntitySizeLimit=0
WalkthroughThe Maven build command in the README file was updated to include two additional JVM system properties related to XML entity size limits. No other files or documentation were changed. Changes
Suggested reviewers
Poem
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
README.md (1)
30-32: Replace hard tabs with spaces for code block indentation
The inserted Maven command lines use hard tabs, triggering markdownlint MD010. Replace with spaces to maintain consistent formatting across the code block.
Suggested diff:- mvn clean verify -Djarsigner.skip=true \ - -Djdk.xml.maxGeneralEntitySizeLimit=0 \ - -Djdk.xml.totalEntitySizeLimit=0 + mvn clean verify -Djarsigner.skip=true \ + -Djdk.xml.maxGeneralEntitySizeLimit=0 \ + -Djdk.xml.totalEntitySizeLimit=0🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
30-30: Hard tabs
Column: 1(MD010, no-hard-tabs)
31-31: Hard tabs
Column: 1(MD010, no-hard-tabs)
32-32: Hard tabs
Column: 1(MD010, no-hard-tabs)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
README.md(1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
README.md
30-30: Hard tabs
Column: 1
(MD010, no-hard-tabs)
31-31: Hard tabs
Column: 1
(MD010, no-hard-tabs)
32-32: Hard tabs
Column: 1
(MD010, no-hard-tabs)
github-actions
bot
changed the title
| mvn clean verify -Djarsigner.skip=true | ||
| mvn clean verify -Djarsigner.skip=true \ | ||
| -Djdk.xml.maxGeneralEntitySizeLimit=0 \ | ||
| -Djdk.xml.totalEntitySizeLimit=0 |
There was a problem hiding this comment.
Thanks for the PR. While this can help bypass errors with large XML files, setting these limits to zero disables important protections against XML. These limits are in place to prevent malicious or accidental resource exhaustion
Best practice is to only increase these limits as much as necessary when needed, not to remove them entirely. If we are hitting the default limits, we should set them to a large but finite value that is sufficient for our use case (e.g., -Djdk.xml.totalEntitySizeLimit=200000000 for 200MB), rather than disabling the limit.
For now, I am not merging this change.
Thanks again for your effort and understanding!
|
I regularly use the same build command and haven’t encountered XML entity size limit errors, either locally or on our CI systems. I use java 21. In general, we should avoid disabling these limits unless absolutely necessary, as discussed above. |
2 participants
Before command not working, updated to:
mvn clean verify -Djarsigner.skip=true
-Djdk.xml.maxGeneralEntitySizeLimit=0
-Djdk.xml.totalEntitySizeLimit=0
Description
Please include a summary of the change and which issue is fixed.
Fixes # (IEP-XXX)
Type of change
Please delete options that are not relevant.
How has this been tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Test Configuration:
Dependent components impacted by this PR:
Checklist
Summary by CodeRabbit