Re: RFR: 8330542: Add jaxp-strict.properties in preparation for a secure by default configuration [v10]
On Mon, 20 May 2024 13:03:36 GMT, Sean Mullan wrote: >> In XML parlance, a "processor" is an aggregation of parsers, serializers, >> and other things that contribute to the processing. So I think it could be >> either here, but you have a point and if it stays as "processor" then it >> should link #FacPro where the term is defined. > > It's the wording that doesn't sound right, before this you have "making" > which doesn't sound right with "process". Maybe it needs two sentences. Thanks! Same as Alan, I thought you were talking about "processor" as well on the first glance, then realized you're referring to the parallel statements :-) - PR Review Comment: https://git.openjdk.org/jdk/pull/18831#discussion_r1606995925
Re: RFR: 8330542: Add jaxp-strict.properties in preparation for a secure by default configuration [v10]
On Mon, 20 May 2024 07:13:02 GMT, Alan Bateman wrote: >> Joe Wang has updated the pull request incrementally with one additional >> commit since the last revision: >> >> withdraw changes to jaxp.properties. The configuration process has not >> changed, changing the default configuration would result in many failures >> that test the process. > > src/java.xml/share/conf/jaxp-strict.properties line 17: > >> 15: # java -Djava.xml.config.file=$JAVA_HOME/conf/jaxp-strict.properties >> 16: # >> 17: # The pathname to the configuration file must be valid. If it is not >> absolute, > > I think it would be better to drop this paragraph or else just replace it > with a sentence to say that the java.xml module description specifies the > system property. Thanks Alan. Replaced the paragraph with a reference to the module description. - PR Review Comment: https://git.openjdk.org/jdk/pull/18831#discussion_r1606993156
Re: RFR: 8330542: Add jaxp-strict.properties in preparation for a secure by default configuration [v10]
On Mon, 20 May 2024 12:55:24 GMT, Alan Bateman wrote: >> src/java.xml/share/classes/module-info.java line 444: >> >>> 442: * >>> 443: * Deploying with this configuration prevents processors from >>> unknowingly making >>> 444: * outbound network connections to fetch DTDs, or process XML that >>> makes use of >> >> s/process/processing/ > > In XML parlance, a "processor" is an aggregation of parsers, serializers, and > other things that contribute to the processing. So I think it could be either > here, but you have a point and if it stays as "processor" then it should link > #FacPro where the term is defined. It's the wording that doesn't sound right, before this you have "making" which doesn't sound right with "process". Maybe it needs two sentences. - PR Review Comment: https://git.openjdk.org/jdk/pull/18831#discussion_r1606754542
Re: RFR: 8330542: Add jaxp-strict.properties in preparation for a secure by default configuration [v10]
On Mon, 20 May 2024 12:48:01 GMT, Sean Mullan wrote: >> Joe Wang has updated the pull request incrementally with one additional >> commit since the last revision: >> >> withdraw changes to jaxp.properties. The configuration process has not >> changed, changing the default configuration would result in many failures >> that test the process. > > src/java.xml/share/classes/module-info.java line 444: > >> 442: * >> 443: * Deploying with this configuration prevents processors from >> unknowingly making >> 444: * outbound network connections to fetch DTDs, or process XML that >> makes use of > > s/process/processing/ In XML parlance, a "processor" is an aggregation of parsers, serializers, and other things that contribute to the processing. So I think it could be either here, but you have a point and if it stays as "processor" then it should link #FacPro where the term is defined. - PR Review Comment: https://git.openjdk.org/jdk/pull/18831#discussion_r1606745477
Re: RFR: 8330542: Add jaxp-strict.properties in preparation for a secure by default configuration [v10]
On Sun, 19 May 2024 05:01:32 GMT, Joe Wang wrote: >> 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. > > Joe Wang has updated the pull request incrementally with one additional > commit since the last revision: > > withdraw changes to jaxp.properties. The configuration process has not > changed, changing the default configuration would result in many failures > that test the process. src/java.xml/share/classes/module-info.java line 444: > 442: * > 443: * Deploying with this configuration prevents processors from > unknowingly making > 444: * outbound network connections to fetch DTDs, or process XML that makes > use of s/process/processing/ - PR Review Comment: https://git.openjdk.org/jdk/pull/18831#discussion_r1606737006
Re: RFR: 8330542: Add jaxp-strict.properties in preparation for a secure by default configuration [v10]
On Sun, 19 May 2024 05:01:32 GMT, Joe Wang wrote: >> 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. > > Joe Wang has updated the pull request incrementally with one additional > commit since the last revision: > > withdraw changes to jaxp.properties. The configuration process has not > changed, changing the default configuration would result in many failures > that test the process. Looks good, just one comment on on the jaxp-strict.properties file text. src/java.xml/share/conf/jaxp-strict.properties line 17: > 15: # java -Djava.xml.config.file=$JAVA_HOME/conf/jaxp-strict.properties > 16: # > 17: # The pathname to the configuration file must be valid. If it is not > absolute, I think it would be better to drop this paragraph or else just replace it with a sentence to say that the java.xml module description specifies the system property. - Marked as reviewed by alanb (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/18831#pullrequestreview-2065520089 PR Review Comment: https://git.openjdk.org/jdk/pull/18831#discussion_r1606350445
Re: RFR: 8330542: Add jaxp-strict.properties in preparation for a secure by default configuration [v10]
> 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 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. Joe Wang has updated the pull request incrementally with one additional commit since the last revision: withdraw changes to jaxp.properties. The configuration process has not changed, changing the default configuration would result in many failures that test the process. - Changes: - all: https://git.openjdk.org/jdk/pull/18831/files - new: https://git.openjdk.org/jdk/pull/18831/files/2ee2c7ca..dfc965c6 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=18831&range=09 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=18831&range=08-09 Stats: 10 lines in 2 files changed: 0 ins; 0 del; 10 mod Patch: https://git.openjdk.org/jdk/pull/18831.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18831/head:pull/18831 PR: https://git.openjdk.org/jdk/pull/18831
