On Fri, 28 Apr 2023 05:47:24 GMT, Joe Wang <jo...@openjdk.org> wrote:
>> Add a system property, jdk.xml.config.file, to return the path to a custom >> JAXP configuration file. The current configuration file, jaxp.properties, >> that the JDK supports will become the default configuration file. >> >> CSR: https://bugs.openjdk.org/browse/JDK-8303531 >> >> Tests: XML SQE and JCK tests passed. > > Joe Wang has updated the pull request with a new target base due to a merge > or a rebase. The incremental webrev excludes the unrelated changes brought in > by the merge/rebase. The pull request contains ten additional commits since > the last revision: > > - Merge branch 'master' into JDK-8303530 > - Update based on the review meeting on 4/5. Added (duplicated) definitions > in jaxp.properties; Rewrote Property Precedence with samples; Other updates. > - update javadoc as discussed in the review meeting; add a sample > configuration file jaxp.properties; update impl and tests accordingly. > - continue support stax.properties at the impl level, though dropped from > the spec > - change prefix from jdk to java > - change prefix from jdk to java; rm zip file that accidentally checked in > - update config file description; add a general scope and order section; > move value definition for external properties to class description > - clean up tests; fix copy&paste error. > - 8303530: Add system property for custom JAXP configuration file Regarding the new unit tests, it looks like there are a lot of boilerplate codes. Can they be shared? src/java.xml/share/classes/jdk/xml/internal/SecuritySupport.java line 234: > 232: } > 233: firstTimeCustom = false; > 234: } Is it possible if `firstTime` and `firstTimeCustom` differ? Would it be possible to get rid of `firstTimeCustom` and move this piece into `firstTime` block? src/java.xml/share/classes/jdk/xml/internal/SecuritySupport.java line 253: > 251: return true; > 252: } catch (IOException e) { > 253: // ignore file not found error How could "file not found" occur assuming `SecuritySupport.doesFileExist(f)` returns true? ------------- PR Review: https://git.openjdk.org/jdk/pull/12985#pullrequestreview-1406630394 PR Review Comment: https://git.openjdk.org/jdk/pull/12985#discussion_r1180861488 PR Review Comment: https://git.openjdk.org/jdk/pull/12985#discussion_r1180862384
