jdk
jdk copied to clipboard
openjdk
8330542: Add jaxp-strict.properties in preparation for a secure by default configuration
Add two sample configuration files:
jaxp-strict.properties: used to set strict configuration, stricter than jaxp.properties in previous versions such as JDK 22
jaxp-compat.properties: used to regain compatibility from any more restricted configuration than previous versions such as JDK 22
Updated on 5/16/2024
Design change: The design is changed to include in the JDK two configuration files that are the default jaxp.properties and jaxp-strict.properties, instead of three, dropping jaxp-compat.properties.
Updated on 5/18/2024
Withdraw changes to jaxp.properties. The original idea was to match jaxp-strict.properties with regard to the properties. However, that change impact the configuration process, resulting in tests that verify the process to fail.
Progress
- [x] Change must be properly reviewed (1 review required, with at least 1 Reviewer)
- [x] Change must not contain extraneous whitespace
- [x] Commit message must refer to an issue
- [ ] Change requires CSR request JDK-8331016 to be approved
Issues
- JDK-8330542: Add jaxp-strict.properties in preparation for a secure by default configuration (Enhancement - P4)
- JDK-8331016: Add jaxp-strict.properties in preparation for a secure by default configuration (CSR)
Reviewers
- Lance Andersen (@LanceAndersen - Reviewer) ⚠️ Review applies to 019c2aee
- Erik Joelsson (@erikj79 - Reviewer) ⚠️ Review applies to 2ee2c7ca
Reviewing
Using git
Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/18831/head:pull/18831
$ git checkout pull/18831
Update a local copy of the PR:
$ git checkout pull/18831
$ git pull https://git.openjdk.org/jdk.git pull/18831/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 18831
View PR using the GUI difftool:
$ git pr show -t 18831
Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/18831.diff
Webrev
:wave: Welcome back joehw! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.
![bridgekeeper[bot] avatar](https://avatars.githubusercontent.com/u/41768318?v=4 "Published by a pub.dev verified publisher"){kind=small}
@JoeWang-Java This change now passes all automated pre-integration checks.
ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.
After integration, the commit message for the final commit will be:
8330542: Template for Creating Strict JAXP Configuration File
Reviewed-by: lancea, erikj, alanb, ihse, mullan, naoto
You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.
At the time when this comment was updated there had been 512 new commits pushed to the master branch:
- 0f3e2cc334e5926d53bbbce22e4a6bfeb2752140: 8331670: Deprecate the Memory-Access Methods in sun.misc.Unsafe for Removal
- 51ae08f72b879bc611177ea643cd88e36185d9e8: 8333093: Incorrect comment in zAddress_aarch64.cpp
- 4754f059f99a426cc8c5d94b0809e79d563ffc2e: 8333035: Parallel: Remove ParMarkBitMap::IterationStatus
- 87a06b6ce41f8623d9111b4e41c72f0ddf842acd: 8325805: Compiler Implementation for Flexible Constructor Bodies (Second Preview)
- e708d135e3af7e0652cdbb680388a0735582ba74: 8332064: Implementation of Structured Concurrency (Third Preview)
- 7b52d0acfc7d6083b407efa0877c139e9837f86b: 8332265: RISC-V: Materialize pointers faster by using a temp register
- aa4c83a5bfe146714a46fb454aafc7393d2d8453: 8332505: JEP 457: ClassRemapper forgets to remap bootstrap method references
- cabe337400a0bd61d73bf3ca66e16266267299c7: 8331921: Hotspot assembler files should use common logic to setup exported functions
- 2edb6d98133d8bd6dc4527c7497c460283fdc53e: 8330386: Replace Opaque4Node of Initialized Assertion Predicate with new OpaqueInitializedAssertionPredicateNode
- 1850914380655ef3d08614a5656e7cc23478f38f: 8332864: Parallel: Merge ParMarkBitMapClosure into MoveAndUpdateClosure
- ... and 502 more: https://git.openjdk.org/jdk/compare/064628471b83616b4463baa78618d1b7a66d0c7c...master
As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.
➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.
@JoeWang-Java The following label will be automatically applied to this pull request:
core-libs
When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.
Webrevs
- 14: Full - Incremental (abc1e88b)
- 13: Full - Incremental (714095d1)
- 12: Full - Incremental (0de8ad69)
- 11: Full - Incremental (dd7f6239)
- 10: Full - Incremental (55a86db3)
- 09: Full - Incremental (dfc965c6)
- 08: Full - Incremental (2ee2c7ca)
- 07: Full - Incremental (cf4df792)
- 06: Full - Incremental (f3af4ae9)
- 05: Full - Incremental (af351a4d)
- 04: Full - Incremental (93b66312)
- 03: Full - Incremental (019c2aee)
- 02: Full - Incremental (e6386015)
- 01: Full - Incremental (98fcc3ef)
- 00: Full (e36e5fd4)
Just some context here. The JDK has been on a path for many releases to make XML processing more secure by default. At some point there will be a proposal/JEP to flip the switch, something that may create an upgrade challenge for some applications and deployments that haven't embraced the various security features and configuration added over the last 10+ years. The addition of conf/jaxp-strict.properties allows deployments to test more the secure/strict behavior in preparation for a possible future where strict is the default. As examples: trying this out may help identify processing XML that (perhaps unknowingly) makes outbound network connections to fetch DTD, or processing XML that relies on extension functions. So I think very useful to have this configuration available in a shipping JDK but it may need a write-up/JEP before anyone knows about this.
Thanks Alan, Lance! I added description of the three config files to both files so that readers can find answers reading any one of them. Also added a release note: https://bugs.openjdk.org/browse/JDK-8330605
It might just be me, but the word "sample" is too related to programming examples that it makes this feel like something that users may not take seriously. But I think this is something that you really want users to take seriously and try out. Perhaps just drop the word "sample"?
A few other comments/questions:
Does this need a CSR since you are adding new property files?
Are there any tests to ensure the property files are working correctly?
Also, how does one try out these property files? Is there a system property that needs to be set? Can you add more details to the RN on that?
A few other comments/questions:
Does this need a CSR since you are adding new property files?
Not sure it does, but Joe will follow up with Joe Darcy
Are there any tests to ensure the property files are working correctly?
There are tests that Joe added as part of the JDK 22 work for custom config files
Also, how does one try out these property files? Is there a system property that needs to be set? Can you add more details to the RN on that?
java -Djava.xml.config.file=$JAVA_HOME/conf/jaxp-compat.properties
The property was added in JDK 22 see: https://docs.oracle.com/en/java/javase/22/docs/api/java.xml/module-summary.html#Conf_CF_SP
Also, how does one try out these property files? Is there a system property that needs to be set? Can you add more details to the RN on that?
java -Djava.xml.config.file=$JAVA_HOME/conf/jaxp-compat.properties
The property was added in JDK 22 see: https://docs.oracle.com/en/java/javase/22/docs/api/java.xml/module-summary.html#Conf_CF_SP
Sounds good, can you add an example to the RN using the above system property?
Sounds good, can you add an example to the RN using the above system property?
Added. Thanks.
Adding jaxp-strict.properties make sense as it allows developers to identify issues that will arise in the future when XML processing is secure by default. If they deploy with -Djava.xml.config.file=jaxp-strict.properties, and jaxp-strict.properties is removed as part of moving to secure by default, then it should be okay too as the defaults will be strict.
I'm less sure about including jaxp-compat.properties in JDK 23. That's the config file to get temporary relief while you work through the issues with existing code or deployments that break when XML processing is secure by default. Adding in the JDK 23 sends the message that you can "prepare" your command line in advance, which I don't think should be a goal here.
Update: the plan has changed from providing a configuration file to a template. Module description, test and make file are updated accordingly. Thanks Erik for the new make file!
I would recommend naming the new file jaxp-strict.properties.template instead. This would follow the pattern we have used in the JDK, and I think it is much better at providing clarify as to what this file actually is -- a template for a .properties file.
Magnus's suggestion for the suffix to be .properties.template makes sense, consistent with the one .template that the JDK currently includes in the run-time image.
Overall this looks okay, I'm happy that the implNote update is removed from the proposal as it read too much like the introducing a new supported interface and would have been confusing to have two configurations in the conf directory.
Thanks Magnus, Alan. Renamed the template to jaxp-strict.properties.template.
Going to push as commit 91caec07cb2e4d98d4366f5627f55834282caa94.
Since your change was applied there have been 514 commits pushed to the master branch:
- da6aa2a86c86ba5fce747b36dcb2d6001cfcc44e: 8332849: Update doc/testing.{md,html} (spelling and stale information)
- b8f2ec9091f9f7e5f4611991d04dd8aa113b94fd: 8195675: Call to insertText with single character from custom Input Method ignored
- 0f3e2cc334e5926d53bbbce22e4a6bfeb2752140: 8331670: Deprecate the Memory-Access Methods in sun.misc.Unsafe for Removal
- 51ae08f72b879bc611177ea643cd88e36185d9e8: 8333093: Incorrect comment in zAddress_aarch64.cpp
- 4754f059f99a426cc8c5d94b0809e79d563ffc2e: 8333035: Parallel: Remove ParMarkBitMap::IterationStatus
- 87a06b6ce41f8623d9111b4e41c72f0ddf842acd: 8325805: Compiler Implementation for Flexible Constructor Bodies (Second Preview)
- e708d135e3af7e0652cdbb680388a0735582ba74: 8332064: Implementation of Structured Concurrency (Third Preview)
- 7b52d0acfc7d6083b407efa0877c139e9837f86b: 8332265: RISC-V: Materialize pointers faster by using a temp register
- aa4c83a5bfe146714a46fb454aafc7393d2d8453: 8332505: JEP 457: ClassRemapper forgets to remap bootstrap method references
- cabe337400a0bd61d73bf3ca66e16266267299c7: 8331921: Hotspot assembler files should use common logic to setup exported functions
- ... and 504 more: https://git.openjdk.org/jdk/compare/064628471b83616b4463baa78618d1b7a66d0c7c...master
Your commit was automatically rebased without conflicts.
@JoeWang-Java Pushed as commit 91caec07cb2e4d98d4366f5627f55834282caa94.
:bulb: You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.