RFR: 8169772: [JAXP] XALAN: Transformation of DOM with null valued text node causes NPE
Hi, please review another XALAN fix. The Serializer should be able to handle text nodes with null input. There has already been some discussion here: http://mail.openjdk.java.net/pipermail/core-libs-dev/2016-November/044567.html Bug: https://bugs.openjdk.java.net/browse/JDK-8169772 Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8169772.0/ The actual fix is in ToUnknownStream.java, method "public void characters(String chars) throws SAXException". I don't know if one should even directly return after chars being null or of size() 0. The rest of this change is cleanups and a test case. Thanks for reviewing. Best regards Christoph
RE: RFR (JAXP) 8158619: Very large CDATA section in XML document causes OOME
Hi Joe,
this looks good and contains nice cleanups (some of them I have as well in my
changes for 8169631 :) ).
Some formatting nits:
src/java.xml/share/classes/com/sun/org/apache/xalan/internal/xsltc/compiler/XSLTC.java:
line 975 map.put(className+".class", attr);
spaces left and right of '+'
src/java.xml/share/classes/com/sun/org/apache/xalan/internal/xsltc/trax/TemplatesImpl.java
168 private final Map>
_loadedExternalExtensionFunctions;
175 TransletClassLoader(ClassLoader parent,Map> mapEF)
{
spaces in Map declaration
src/java.xml/share/classes/com/sun/org/apache/xalan/internal/xsltc/trax/TransformerFactoryImpl.java
288 public Map> getExternalExtensionsMap() {
spaces
src/java.xml/share/classes/com/sun/org/apache/xerces/internal/impl/xs/XMLSchemaLoader.java
700 LocationArray la = (locations.get(XMLSymbols.EMPTY_STRING));
Braces are unneeded
354 null, null, null, null, JdkXmlUtils.CDATA_CHUNK_SIZE_DEFAULT};
Add 2 more spaces in the beginning and a space before the closing }
Best regards
Christoph
> -Original Message-
> From: core-libs-dev [mailto:core-libs-dev-boun...@openjdk.java.net] On Behalf
> Of Joe Wang
> Sent: Mittwoch, 16. November 2016 23:13
> To: core-libs-dev@openjdk.java.net
> Subject: RFR (JAXP) 8158619: Very large CDATA section in XML document
> causes OOME
>
> Hi,
>
> Please review an enhancement adding a property to allow for specifying
> the chunk size of CDATA.
>
> JBS: https://bugs.openjdk.java.net/browse/JDK-8158619
> webrevs: http://cr.openjdk.java.net/~joehw/jdk9/8158619/webrev/
>
> Thanks,
> Joe
RE: RFR: 8169772: [JAXP] XALAN: Transformation of DOM with null valued text node causes NPE
Hi Joe, thanks for the feedback. I've created a new version of the webrev working in your suggestions, adding some further formatting cleanups in the files and I also moved a small refactoring in TransformerTest.java to this changeset. http://cr.openjdk.java.net/~clanger/webrevs/8169772.1/ >From my end this one is ready for pushing - waiting for your final go. Best regards Christoph From: Joe Wang [mailto:huizhe.w...@oracle.com] Sent: Freitag, 18. November 2016 07:36 To: Langer, Christoph Cc: core-libs-dev@openjdk.java.net Subject: Re: RFR: 8169772: [JAXP] XALAN: Transformation of DOM with null valued text node causes NPE Looks fine. License header: in general, don't change / add Year if there's no material change, removing the legacy $Id field in EmptySerializer.java for example, does not constitute a change to the code, so just keep the original year (see below). The initial years for the classes: EmptySerializer.java 2012 SerializerBase.java 2012 ToSAXHandler.java none (meaning if you make changes to this class, just add 2016) ToStream.java 2006 ToUnknownStream.java 2007 XSLOutputAttributes.java none (so keep the original "DO NOT REMOVE OR ALTER!" block) Thanks, Joe On 11/16/16, 6:22 AM, Langer, Christoph wrote: Hi, please review another XALAN fix. The Serializer should be able to handle text nodes with null input. There has already been some discussion here: http://mail.openjdk.java.net/pipermail/core-libs-dev/2016-November/044567.html Bug: https://bugs.openjdk.java.net/browse/JDK-8169772 Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8169772.0/ The actual fix is in ToUnknownStream.java, method "public void characters(String chars) throws SAXException". I don't know if one should even directly return after chars being null or of size() 0. The rest of this change is cleanups and a test case. Thanks for reviewing. Best regards Christoph
RE: RFR: 8169772: [JAXP] XALAN: Transformation of DOM with null valued text node causes NPE
Hi Joe, Yep, I'll conduct the testing before pushing. With the "Expected Result", you mean some comment for the test method, right? Thanks Christoph From: Joe Wang [mailto:huizhe.w...@oracle.com] Sent: Freitag, 18. November 2016 20:00 To: Langer, Christoph Cc: core-libs-dev@openjdk.java.net Subject: Re: RFR: 8169772: [JAXP] XALAN: Transformation of DOM with null valued text node causes NPE Looks good, Christoph. I assume you'll do an all-test run (all in jaxp/test) before pushing. No need for further review, but it'd be nice to add an "Expected result" for the new test testBug8169772, with/without the fix (e.g. NPE). Best regards, Joe On 11/18/16, 4:38 AM, Langer, Christoph wrote: Hi Joe, thanks for the feedback. I've created a new version of the webrev working in your suggestions, adding some further formatting cleanups in the files and I also moved a small refactoring in TransformerTest.java to this changeset. http://cr.openjdk.java.net/~clanger/webrevs/8169772.1/<http://cr.openjdk.java.net/%7Eclanger/webrevs/8169772.1/> >From my end this one is ready for pushing - waiting for your final go. Best regards Christoph From: Joe Wang [mailto:huizhe.w...@oracle.com] Sent: Freitag, 18. November 2016 07:36 To: Langer, Christoph <mailto:christoph.lan...@sap.com> Cc: core-libs-dev@openjdk.java.net<mailto:core-libs-dev@openjdk.java.net> Subject: Re: RFR: 8169772: [JAXP] XALAN: Transformation of DOM with null valued text node causes NPE Looks fine. License header: in general, don't change / add Year if there's no material change, removing the legacy $Id field in EmptySerializer.java for example, does not constitute a change to the code, so just keep the original year (see below). The initial years for the classes: EmptySerializer.java 2012 SerializerBase.java 2012 ToSAXHandler.java none (meaning if you make changes to this class, just add 2016) ToStream.java 2006 ToUnknownStream.java 2007 XSLOutputAttributes.java none (so keep the original "DO NOT REMOVE OR ALTER!" block) Thanks, Joe On 11/16/16, 6:22 AM, Langer, Christoph wrote: Hi, please review another XALAN fix. The Serializer should be able to handle text nodes with null input. There has already been some discussion here: http://mail.openjdk.java.net/pipermail/core-libs-dev/2016-November/044567.html Bug: https://bugs.openjdk.java.net/browse/JDK-8169772 Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8169772.0/<http://cr.openjdk.java.net/%7Eclanger/webrevs/8169772.0/> The actual fix is in ToUnknownStream.java, method "public void characters(String chars) throws SAXException". I don't know if one should even directly return after chars being null or of size() 0. The rest of this change is cleanups and a test case. Thanks for reviewing. Best regards Christoph
RE: RFR: 8169631: [JAXP] XALAN: transformation of XML via namespace-unaware SAX input yields a different result than namespace-unaware DOM input
Hi Joe, thanks for the feedback. I've now understood the testcase that you've sent over and the reason that it is reporting failure after my fix is that the output of its transform operation is rather correct now. And before it was wrong. :) The test is comparing the actual result to a "golden" result file in the end and both of these were not looking healthy so far. The reason is that your test is using a namespace unaware SAX Parser as input. With the current JDK XALAN, you could already modify your smoketest to use a namespace aware parser. E.g. replace lines 82// Use the JAXP way to get an XMLReader 83XMLReader reader = SAXParserFactory.newInstance().newSAXParser().getXMLReader(); with 82// Use the JAXP way to get an XMLReader 83SAXParserFactory spf = SAXParserFactory.newInstance(); 84spf.setNamespaceAware(true); 85XMLReader reader = spf.newSAXParser().getXMLReader(); ...and you would already get correct results that also DOM input or Stream Input would yield. So, are there other concerns/issues with this fix? Do you want me to include a transformation operation like the one that your SmokeTest does to TransformerTest which would illustrate the problem with namespace unaware SAX input data? Best regards Christoph > -Original Message- > From: Joe Wang [mailto:huizhe.w...@oracle.com] > Sent: Freitag, 18. November 2016 05:53 > To: Langer, Christoph > Cc: core-libs-dev@openjdk.java.net > Subject: Re: RFR: 8169631: [JAXP] XALAN: transformation of XML via > namespace-unaware SAX input yields a different result than namespace- > unaware DOM input > > > > On 11/14/16, 11:43 PM, Langer, Christoph wrote: > > Hi Joe, > > > > thanks for looking. > > > > Can you let me know which smoke test is failing? I didn't see issues so far > > - I > was merely running the jtreg unittests for transformer. > > I sent the test to your mailbox. > > > > I stepped back from replacing Vector with ArrayList for m_prefixMappings > because the code is using methods indexOf() with a start index and setSize() > for > which ArrayList has no direct matchings. One could, for sure, add some other > coding, e.g. use ArrayList's subList() method for the index based search - > but I > wouldn't want to run the risk of adding a regression here just because I > modified the code and did not well test it. But if you insist, I could have > another > look. > > Ok, that's fine. subList would do, but setSize may need a bit more work. > > Best, > Joe > > > > Best regards > > Christoph > > > >> -Original Message- > >> From: Joe Wang [mailto:huizhe.w...@oracle.com] > >> Sent: Dienstag, 15. November 2016 03:23 > >> To: Langer, Christoph > >> Cc: core-libs-dev@openjdk.java.net > >> Subject: Re: RFR: 8169631: [JAXP] XALAN: transformation of XML via > >> namespace-unaware SAX input yields a different result than namespace- > >> unaware DOM input > >> > >> Hi Christoph, > >> > >> Not all tests have finished yet, but there's at least one failure in the > >> smoke test. I'll get to the details when I have time. > >> > >> Any reason why m_prefixMappings can not be replaced with ArrayList? > >> > >> Thanks, > >> Joe > >> > >> On 11/14/16, 6:10 AM, Langer, Christoph wrote: > >>> Hi, > >>> > >>> please review this fix for bug 8169631. > >>> > >>> Bug: https://bugs.openjdk.java.net/browse/JDK-8169631 > >>> Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8169631.0/ > >>> > >>> When XALAN is handling namespace unaware input, it behaves differently > >> while using SAX input compared to DOM input. > >>> With both input source types, the class > >> com.sun.org.apache.xml.internal.dtm.ref.sax2dtm.SAX2DTM2 converts SAX > >> input into a DTM representation for processing by the XALAN transformer. > Its > >> method startElement takes URI, localname and qName as attribute. In case > of > >> missing feature namespaces, startElement and localname can be empty. > >> However, the function uses the localname value for the call to > >> m_expandedNameTable.getExpandedTypeID() and further processing. In the > >> case where only qName has data, this leads to issues. > >>> When using DOM input, the class > >> com.sun.org.apache.xalan.internal.xsltc.trax.DOM2SAX converts the DOM > input > >> into SAX input. In the case of empty localname, it fills localname with > >> qnam
Issues running JAXP jtreg tests ("java.lang.RuntimePermission" "accessDeclaredMembers")
Hi,
I'm currently struggling while running jtreg tests for the jaxp depot.
There are several tests that fail with the same symptom. I always get
exceptions like:
java.security.AccessControlException: access denied
("java.lang.RuntimePermission" "accessDeclaredMembers")
at
java.base/java.security.AccessControlContext.checkPermission(AccessControlContext.java:471)
at
java.base/java.security.AccessController.checkPermission(AccessController.java:894)
at
java.base/java.lang.SecurityManager.checkPermission(SecurityManager.java:548)
at java.base/java.lang.Class.checkMemberAccess(Class.java:2595)
at java.base/java.lang.Class.getDeclaredMethods(Class.java:2162)
at org.testng.internal.ClassHelper.extractMethods(ClassHelper.java:217)
at
org.testng.internal.ClassHelper.getAvailableMethods(ClassHelper.java:182)
at org.testng.internal.Parameters.findDataProvider(Parameters.java:323)
at org.testng.internal.Parameters.findDataProvider(Parameters.java:259)
at org.testng.internal.Parameters.handleParameters(Parameters.java:419)
at org.testng.internal.Invoker.handleParameters(Invoker.java:1274)
at org.testng.internal.Invoker.createParameters(Invoker.java:989)
at org.testng.internal.Invoker.invokeTestMethods(Invoker.java:1079)
at
org.testng.internal.TestMethodWorker.invokeTestMethods(TestMethodWorker.java:129)
at org.testng.internal.TestMethodWorker.run(TestMethodWorker.java:112)
at org.testng.TestRunner.privateRun(TestRunner.java:782)
at org.testng.TestRunner.run(TestRunner.java:632)
at org.testng.SuiteRunner.runTest(SuiteRunner.java:366)
at org.testng.SuiteRunner.runSequentially(SuiteRunner.java:361)
at org.testng.SuiteRunner.privateRun(SuiteRunner.java:319)
at org.testng.SuiteRunner.run(SuiteRunner.java:268)
at org.testng.SuiteRunnerWorker.runSuite(SuiteRunnerWorker.java:52)
at org.testng.SuiteRunnerWorker.run(SuiteRunnerWorker.java:86)
at org.testng.TestNG.runSuitesSequentially(TestNG.java:1244)
at org.testng.TestNG.runSuitesLocally(TestNG.java:1169)
at org.testng.TestNG.run(TestNG.java:1064)
at
com.sun.javatest.regtest.TestNGAction$TestNGRunner.main(TestNGAction.java:224)
at
com.sun.javatest.regtest.TestNGAction$TestNGRunner.main(TestNGAction.java:188)
at
java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at
java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at
java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.base/java.lang.reflect.Method.invoke(Method.java:537)
at
com.sun.javatest.regtest.agent.MainWrapper$MainThread.run(MainWrapper.java:110)
at java.base/java.lang.Thread.run(Thread.java:844)
For instance the test javax/xml/jaxp/unittest/transform/XSLTFunctionsTest.java
fails like this.
It's calling "testng -DrunSecMngr=true" and obviously some important permission
for testing is missing with that.
I'm using most current jtreg (with testng-6.9.10.jar)
Thanks for any help.
Best regards
Christoph
RE: Issues running JAXP jtreg tests ("java.lang.RuntimePermission" "accessDeclaredMembers")
Hi Chris,
thanks for this hint. However, we've already seen this change and rebuilt jtreg
with the latest jtreg repo. But it doesn't change a thing.
Also, the download from
https://adopt-openjdk.ci.cloudbees.com/job/jtreg/lastSuccessfulBuild/artifact/
where I would suppose latest jtreg sources were used, don't help.
Am I missing something?
Best regards
Christoph
> -Original Message-
> From: Chris Hegarty [mailto:chris.hega...@oracle.com]
> Sent: Dienstag, 22. November 2016 10:08
> To: Langer, Christoph
> Cc: core-libs-dev@openjdk.java.net; code-tools-...@openjdk.java.net; jtreg-
> u...@openjdk.java.net
> Subject: Re: Issues running JAXP jtreg tests ("java.lang.RuntimePermission"
> "accessDeclaredMembers")
>
> Hi Christoph,
>
> Can you please ensure that your build of jtreg contains the fix for 7901792
> [1].
> 7901792 grants /lib/testng.jar all permissions.
>
> -Chris.
>
> [1] https://bugs.openjdk.java.net/browse/CODETOOLS-7901792
>
> > On 22 Nov 2016, at 08:38, Langer, Christoph
> wrote:
> >
> > Hi,
> >
> > I'm currently struggling while running jtreg tests for the jaxp depot.
> >
> > There are several tests that fail with the same symptom. I always get
> exceptions like:
> > java.security.AccessControlException: access denied
> ("java.lang.RuntimePermission" "accessDeclaredMembers")
> >at
> java.base/java.security.AccessControlContext.checkPermission(AccessControlCo
> ntext.java:471)
> >at
> java.base/java.security.AccessController.checkPermission(AccessController.java
> :894)
> >at
> java.base/java.lang.SecurityManager.checkPermission(SecurityManager.java:5
> 48)
> >at java.base/java.lang.Class.checkMemberAccess(Class.java:2595)
> >at java.base/java.lang.Class.getDeclaredMethods(Class.java:2162)
> >at
> > org.testng.internal.ClassHelper.extractMethods(ClassHelper.java:217)
> >at
> org.testng.internal.ClassHelper.getAvailableMethods(ClassHelper.java:182)
> >at
> > org.testng.internal.Parameters.findDataProvider(Parameters.java:323)
> >at
> > org.testng.internal.Parameters.findDataProvider(Parameters.java:259)
> >at
> org.testng.internal.Parameters.handleParameters(Parameters.java:419)
> >at org.testng.internal.Invoker.handleParameters(Invoker.java:1274)
> >at org.testng.internal.Invoker.createParameters(Invoker.java:989)
> >at org.testng.internal.Invoker.invokeTestMethods(Invoker.java:1079)
> >at
> org.testng.internal.TestMethodWorker.invokeTestMethods(TestMethodWorker.
> java:129)
> >at
> org.testng.internal.TestMethodWorker.run(TestMethodWorker.java:112)
> >at org.testng.TestRunner.privateRun(TestRunner.java:782)
> >at org.testng.TestRunner.run(TestRunner.java:632)
> >at org.testng.SuiteRunner.runTest(SuiteRunner.java:366)
> >at org.testng.SuiteRunner.runSequentially(SuiteRunner.java:361)
> >at org.testng.SuiteRunner.privateRun(SuiteRunner.java:319)
> >at org.testng.SuiteRunner.run(SuiteRunner.java:268)
> >at org.testng.SuiteRunnerWorker.runSuite(SuiteRunnerWorker.java:52)
> >at org.testng.SuiteRunnerWorker.run(SuiteRunnerWorker.java:86)
> >at org.testng.TestNG.runSuitesSequentially(TestNG.java:1244)
> >at org.testng.TestNG.runSuitesLocally(TestNG.java:1169)
> >at org.testng.TestNG.run(TestNG.java:1064)
> >at
> com.sun.javatest.regtest.TestNGAction$TestNGRunner.main(TestNGAction.java:
> 224)
> >at
> com.sun.javatest.regtest.TestNGAction$TestNGRunner.main(TestNGAction.java:
> 188)
> >at
> java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native
> Method)
> >at
> java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethod
> AccessorImpl.java:62)
> >at
> java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(Delegatin
> gMethodAccessorImpl.java:43)
> >at java.base/java.lang.reflect.Method.invoke(Method.java:537)
> >at
> com.sun.javatest.regtest.agent.MainWrapper$MainThread.run(MainWrapper.j
> ava:110)
> >at java.base/java.lang.Thread.run(Thread.java:844)
> >
> > For instance the test
> javax/xml/jaxp/unittest/transform/XSLTFunctionsTest.java fails like this.
> >
> > It's calling "testng -DrunSecMngr=true" and obviously some important
> permission for testing is missing with that.
> >
> > I'm using most current jtreg (with testng-6.9.10.jar)
> >
> >
> > Thanks for any help.
> >
> >
> > Best regards
> > Christoph
RE: Issues running JAXP jtreg tests ("java.lang.RuntimePermission" "accessDeclaredMembers")
Yes, please find it here:
http://cr.openjdk.java.net/~clanger/jtreg/XSLTFunctionsTest.jtr
> -Original Message-
> From: Chris Hegarty [mailto:chris.hega...@oracle.com]
> Sent: Dienstag, 22. November 2016 11:03
> To: Langer, Christoph
> Cc: core-libs-dev@openjdk.java.net; code-tools-...@openjdk.java.net; jtreg-
> u...@openjdk.java.net
> Subject: Re: Issues running JAXP jtreg tests ("java.lang.RuntimePermission"
> "accessDeclaredMembers")
>
>
> > On 22 Nov 2016, at 09:43, Langer, Christoph
> wrote:
> >
> > Hi Chris,
> >
> > thanks for this hint. However, we've already seen this change and rebuilt
> jtreg with the latest jtreg repo. But it doesn't change a thing.
> >
> > Also, the download from https://adopt-
> openjdk.ci.cloudbees.com/job/jtreg/lastSuccessfulBuild/artifact/ where I would
> suppose latest jtreg sources were used, don't help.
> >
> > Am I missing something?
>
> Is it possible to post, or upload to cr.o.j.n, the jtr of the failing test?
>
> -Chris.
>
> > Best regards
> > Christoph
> >
> >> -Original Message-
> >> From: Chris Hegarty [mailto:chris.hega...@oracle.com]
> >> Sent: Dienstag, 22. November 2016 10:08
> >> To: Langer, Christoph
> >> Cc: core-libs-dev@openjdk.java.net; code-tools-...@openjdk.java.net; jtreg-
> >> u...@openjdk.java.net
> >> Subject: Re: Issues running JAXP jtreg tests ("java.lang.RuntimePermission"
> >> "accessDeclaredMembers")
> >>
> >> Hi Christoph,
> >>
> >> Can you please ensure that your build of jtreg contains the fix for 7901792
> [1].
> >> 7901792 grants /lib/testng.jar all permissions.
> >>
> >> -Chris.
> >>
> >> [1] https://bugs.openjdk.java.net/browse/CODETOOLS-7901792
> >>
> >>> On 22 Nov 2016, at 08:38, Langer, Christoph
> >> wrote:
> >>>
> >>> Hi,
> >>>
> >>> I'm currently struggling while running jtreg tests for the jaxp depot.
> >>>
> >>> There are several tests that fail with the same symptom. I always get
> >> exceptions like:
> >>> java.security.AccessControlException: access denied
> >> ("java.lang.RuntimePermission" "accessDeclaredMembers")
> >>> at
> >>
> java.base/java.security.AccessControlContext.checkPermission(AccessControlCo
> >> ntext.java:471)
> >>> at
> >>
> java.base/java.security.AccessController.checkPermission(AccessController.java
> >> :894)
> >>> at
> >>
> java.base/java.lang.SecurityManager.checkPermission(SecurityManager.java:5
> >> 48)
> >>> at java.base/java.lang.Class.checkMemberAccess(Class.java:2595)
> >>> at java.base/java.lang.Class.getDeclaredMethods(Class.java:2162)
> >>> at
> org.testng.internal.ClassHelper.extractMethods(ClassHelper.java:217)
> >>> at
> >> org.testng.internal.ClassHelper.getAvailableMethods(ClassHelper.java:182)
> >>> at
> org.testng.internal.Parameters.findDataProvider(Parameters.java:323)
> >>> at
> org.testng.internal.Parameters.findDataProvider(Parameters.java:259)
> >>> at
> >> org.testng.internal.Parameters.handleParameters(Parameters.java:419)
> >>> at org.testng.internal.Invoker.handleParameters(Invoker.java:1274)
> >>> at org.testng.internal.Invoker.createParameters(Invoker.java:989)
> >>> at org.testng.internal.Invoker.invokeTestMethods(Invoker.java:1079)
> >>> at
> >>
> org.testng.internal.TestMethodWorker.invokeTestMethods(TestMethodWorker.
> >> java:129)
> >>> at
> >> org.testng.internal.TestMethodWorker.run(TestMethodWorker.java:112)
> >>> at org.testng.TestRunner.privateRun(TestRunner.java:782)
> >>> at org.testng.TestRunner.run(TestRunner.java:632)
> >>> at org.testng.SuiteRunner.runTest(SuiteRunner.java:366)
> >>> at org.testng.SuiteRunner.runSequentially(SuiteRunner.java:361)
> >>> at org.testng.SuiteRunner.privateRun(SuiteRunner.java:319)
> >>> at org.testng.SuiteRunner.run(SuiteRunner.java:268)
> >>> at org.testng.SuiteRunnerWorker.runSuite(SuiteRunnerWorker.java:52)
> >>> at org.testng.SuiteRunnerWorker.run(SuiteRunnerWorker.java:86)
> >>> at org.testng.TestNG.runSuitesSequentially(TestNG.java:1244)
> >>>
RE: Issues running JAXP jtreg tests ("java.lang.RuntimePermission" "accessDeclaredMembers")
Hi Daniel,
I checked your suggestion with the paths and moved my jtreg to a local disk -
but nothing changed.
I don't see any policy that is being manipulated. I found out, by adding some
debug printing code, that inside jtreg, the addGrantEntry() method is not
called at all... Is some policy file missing?
Thanks
Christoph
> -Original Message-
> From: Daniel Fuchs [mailto:daniel.fu...@oracle.com]
> Sent: Dienstag, 22. November 2016 12:25
> To: Langer, Christoph ; Chris Hegarty
>
> Cc: code-tools-...@openjdk.java.net; core-libs-dev@openjdk.java.net; jtreg-
> u...@openjdk.java.net
> Subject: Re: Issues running JAXP jtreg tests ("java.lang.RuntimePermission"
> "accessDeclaredMembers")
>
> Hi Christoph,
>
> Is there anything funny with the place jtreg is installed?
> like:
>- path contains whitespaces
>- path is accessible through links or mount points...
>
> It seems clear that the issue here is that testng classes are
> missing some permissions, so I was wondering whether that could
> be caused by the actual path to testng.jar not matching the
> path injected in the policy file.
>
> I'm using jtreg 4.2 fcs b03, and have no issues with the jaxp tests:
>
> $ cd jaxp/tests
> $ rm -r JT*
> $ jtreg -verbose:summary -ignore:quiet -jdk
> ../../build/macosx-x86_64-normal-server-release/images/jdk javax/
>
> => the only test that fails is
> javax/xml/jaxp/isolatedjdk/catalog/PropertiesTest.sh, but that's
> expected (it's in the ProblemList.txt).
>
> The other thing to take care of, is not to run two jtreg process
> concurrently if they point to the same JT* directories. If you do
> that then you might experience weird failures with permissions
> issues (it seems to mess the policy files).
>
> best regards,
>
> -- daniel
>
> On 22/11/16 10:52, Langer, Christoph wrote:
> > Yes, please find it here:
> http://cr.openjdk.java.net/~clanger/jtreg/XSLTFunctionsTest.jtr
> >
> >
> >> -Original Message-
> >> From: Chris Hegarty [mailto:chris.hega...@oracle.com]
> >> Sent: Dienstag, 22. November 2016 11:03
> >> To: Langer, Christoph
> >> Cc: core-libs-dev@openjdk.java.net; code-tools-...@openjdk.java.net; jtreg-
> >> u...@openjdk.java.net
> >> Subject: Re: Issues running JAXP jtreg tests ("java.lang.RuntimePermission"
> >> "accessDeclaredMembers")
> >>
> >>
> >>> On 22 Nov 2016, at 09:43, Langer, Christoph
> >> wrote:
> >>>
> >>> Hi Chris,
> >>>
> >>> thanks for this hint. However, we've already seen this change and rebuilt
> >> jtreg with the latest jtreg repo. But it doesn't change a thing.
> >>>
> >>> Also, the download from https://adopt-
> >> openjdk.ci.cloudbees.com/job/jtreg/lastSuccessfulBuild/artifact/ where I
> would
> >> suppose latest jtreg sources were used, don't help.
> >>>
> >>> Am I missing something?
> >>
> >> Is it possible to post, or upload to cr.o.j.n, the jtr of the failing test?
> >>
> >> -Chris.
> >>
> >>> Best regards
> >>> Christoph
> >>>
> >>>> -Original Message-
> >>>> From: Chris Hegarty [mailto:chris.hega...@oracle.com]
> >>>> Sent: Dienstag, 22. November 2016 10:08
> >>>> To: Langer, Christoph
> >>>> Cc: core-libs-dev@openjdk.java.net; code-tools-...@openjdk.java.net;
> jtreg-
> >>>> u...@openjdk.java.net
> >>>> Subject: Re: Issues running JAXP jtreg tests
> ("java.lang.RuntimePermission"
> >>>> "accessDeclaredMembers")
> >>>>
> >>>> Hi Christoph,
> >>>>
> >>>> Can you please ensure that your build of jtreg contains the fix for
> >>>> 7901792
> >> [1].
> >>>> 7901792 grants /lib/testng.jar all permissions.
> >>>>
> >>>> -Chris.
> >>>>
> >>>> [1] https://bugs.openjdk.java.net/browse/CODETOOLS-7901792
> >>>>
> >>>>> On 22 Nov 2016, at 08:38, Langer, Christoph
>
> >>>> wrote:
> >>>>>
> >>>>> Hi,
> >>>>>
> >>>>> I'm currently struggling while running jtreg tests for the jaxp depot.
> >>>>>
> >>>>> There are several tests that fail with the same symptom. I always get
> >>>> exceptions like:
> >&
RE: [RFR] 8160768: Add capability to custom resolve host/domain names within the default JDNI LDAP provider
Hi Rob, a little style nit: Is it really a good idea to use import java.util.*; in src/java.naming/share/classes/com/sun/jndi/ldap/LdapCtxFactory.java? I thought one is supposed to only use qualified exports nowadays (with all the IDE support). Best regards Christoph -Original Message- From: core-libs-dev [mailto:core-libs-dev-boun...@openjdk.java.net] On Behalf Of Rob McKenna Sent: Mittwoch, 6. Dezember 2017 19:25 To: vyom tewari Cc: Osipov, Michael ; core-libs-dev@openjdk.java.net Subject: Re: [RFR] 8160768: Add capability to custom resolve host/domain names within the default JDNI LDAP provider Thanks Vyom, these should be fixed in: http://cr.openjdk.java.net/~robm/8160768/webrev.07/ Comments inline: On 06/12/17 22:14, vyom tewari wrote: > Hi Rob, > > Please find below comments. > > DefaultLdapDnsProvider.java > > line 35: convert it to for-each code will be more readable. > > LdapDnsProviderService.java > > line 21: constant name does not follow Java naming convention, there are > other places as well please fix it. > > getInstance() is already synchronized why you need another "synchronized" > block ? Ah, neglected to remove the outer synchronized keyword, thanks. > > Line 70: Please use result.getEndpoints().isEmpty() in place of > result.getEndpoints().size() == 0 > > LdapDnsProvider.java > > constructor LdapDnsProvider() calls LdapDnsProvider(Void unused) which does > nothing, can you explain why LdapDnsProvider() calls LdapDnsProvider(Void > unused) ? I've copied this pattern from the System.LoggerFinder api and I had forgotten what it was for too: http://www.oracle.com/technetwork/java/seccodeguide-139067.html#7 Section 7.3. > > LdapDnsProviderResult.java > > Private field domainName & endpoints both can be final. > > LdapDnsProviderTest.java > > Fix the tag Order it is not correct. correct order is as follows. > > /** > * @test > * @bug 8160768 > * @summary ctx provider tests for ldap > * @modules java.naming/com.sun.jndi.ldap > * @compile dnsprovider/TestDnsProvider.java > * @run main/othervm LdapDnsProviderTest > * @run main/othervm LdapDnsProviderTest nosm > * @run main/othervm LdapDnsProviderTest smnodns > * @run main/othervm LdapDnsProviderTest smdns > */ > > Line 82,83 you don't need System.out.println(e); e.printStackTrace(); I'm going to leave these in as I've had a few failing tests in the past where I've needed to push diagnostic changes like this to determine the root cause. -Rob > > Line 70: you don't need this try block > > Line 96 : constant name does not follow Java naming convention > > Line 105: you can use try-with-resources this will avoid some boilerplate. > > Thanks, > Vyom > > On Tuesday 05 December 2017 11:18 PM, Rob McKenna wrote: > >As this enhancement is limited to JDK10 this frees us up to explore a > >different approach. > > > >http://cr.openjdk.java.net/~robm/8160768/webrev.06/ > > > >In this webrev we're using the Service Provider Interface to load an > >implementation of the new LdapDnsProvider class. Implementations of this > >class are responsible for resolving DNS endpoints for a given ldap url > >should the default implementation be insufficient in a particular > >environment. > > > >Note: if a security manager is installed the ldapDnsProvider > >RuntimePermission must be granted. > > > > -Rob > > >
RE: Oracle Java 8u161 regression in XML Schema Factory
Hi Bernd, would your test still fail with system property "-Djdk.xml.overrideDefaultParser=true"? I debugged a similar issue (if not the same) today and found that with change http://hg.openjdk.java.net/jdk8u/jdk8u-dev/jaxp/rev/08a44c164993 the default behavior for Xalan changed a bit. Before that change, the services mechanism was enabled to load the SAX and DOM implementations for the use of Xalan. E.g. when TransformerFactory.newInstance() was called, the services mechanism was used and in the TransformerFactory (at least if it was the default JDK implementation), the property to use the services mechanism was set as well - wich was used to retrieve SAX and DOM parser implementations. And what hit me even more was that SAX2DOM had its private field "DocumentBuilderFactory _factory" automatically initialized by a call to DocumentBuilderFactory.newInstance() which in turn would resolve any custom DBF implementation if it were on classpath. Now, with the new implementation, the _factory in SAX2DOM will be initialized with the "overrideDefaultParser " setting coming from the Transformer Factory. So, by setting -Djdk.xml.overrideDefaultParser=true, you will restore the old behavior. @Joe: Is there some documentation for this change in default behavior that came with JDK8u161? I assume it is for higher security and cannot be reverted (e.g. by setting the feature default for Djdk.xml.overrideDefaultParser to true). Best regards Christoph > -Original Message- > From: core-libs-dev [mailto:core-libs-dev-boun...@openjdk.java.net] On > Behalf Of Bernd > Sent: Dienstag, 13. Februar 2018 22:55 > To: OpenJDK Dev list > Subject: Re: Oracle Java 8u161 regression in XML Schema Factory > > Hello, > > > 2018-01-25 17:41 GMT+01:00 Seán Coffey : > > > > > Classes nearer to those below were touched via JDK-8186080: Transform > XML > > interfaces > > http://hg.openjdk.java.net/jdk8u/jdk8u/jdk/rev/cb84156d54b2 > > http://hg.openjdk.java.net/jdk8u/jdk8u/jaxp/rev/08a44c164993 > > > > This may be connected with some tools trying to redefine the classes > > perhaps. Needs more investigating. Perhaps the XMLSchemaLoader > changes are > > a factor ? > > > > I have ben able to extract a minimal reproducer. It is not related to > XMLUnit, only to powermock. If it instruments com.sun but not javax.xml > (and other combinations) then it fails. > > For details see the readme in this maven project: > > https://github.com/ecki/reproduce-schemaex > > I also found a way to make it work with both versions, so its no longer an > issue for me, but there is definitely some changes (which might also be > triggered in AppServers or OSGi containers with partially reconfigured > implementations. Not sure if you want to investigate deeper). > > Gruss > Bernd > > > Here is the stacktrace anyway: > >> > >> com.sun.org.apache.xerces.internal.impl.dv.DVFactoryException: Schema > >> factory class > >> com.sun.org.apache.xerces.internal.impl.dv.xs.SchemaDVFactoryImpl > does > >> not > >> extend from SchemaDVFactory. > >> at > >> com.sun.org.apache.xerces.internal.impl.dv.SchemaDVFactory. > >> getInstance(SchemaDVFactory.java:75) > >> at > >> com.sun.org.apache.xerces.internal.impl.dv.SchemaDVFactory. > >> getInstance(SchemaDVFactory.java:57) > >> at > >> com.sun.org.apache.xerces.internal.impl.xs.XMLSchemaLoader. > >> reset(XMLSchemaLoader.java:1024) > >> at > >> com.sun.org.apache.xerces.internal.impl.xs.XMLSchemaLoader. > >> loadGrammar(XMLSchemaLoader.java:556) > >> at > >> com.sun.org.apache.xerces.internal.impl.xs.XMLSchemaLoader. > >> loadGrammar(XMLSchemaLoader.java:535) > >> at > >> com.sun.org.apache.xerces.internal.jaxp.validation.XMLSchema > >> Factory.newSchema(XMLSchemaFactory.java:254) > >> at javax.xml.validation.SchemaFactory.newSchema(SchemaFactory. > >> java:638) > >> at javax.xml.validation.SchemaFactory.newSchema(SchemaFactory. > >> java:654) > >> at > >> com.seeburger.api.test.helpers.BuilderTestHelper.getCRSchema > >> Validator(BuilderTestHelper.java:57) > >> at > >> com.seeburger.api.test.helpers.BuilderTestHelper.validateAnd > >> Compare(BuilderTestHelper.java:73) > >> at > >> com.seeburger.api.test.KSMBuilderTest.testDeletePGP(KSMBuild > >> erTest.java:196) > >> at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) > >> at > >> sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAcce > >> ssorImpl.java:62) > >> at > >> sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMe > >> thodAccessorImpl.java:43) > >> at java.lang.reflect.Method.invoke(Method.java:498) > >> at org.junit.internal.runners.TestMethod.invoke(TestMethod.java:68) > >> at > >> org.powermock.modules.junit4.internal.impl.PowerMockJUnit44R > >> unnerDelegateImpl$PowerMockJUnit44MethodRunner.runTestMethod > >> (PowerMockJUnit44RunnerDelegateImpl.java:310) > >> at org.junit.internal.runners.MethodRoadie$2.run(MethodRoad
RFR (XS): 8198539: Cleanup of unused imports in java/util/jar/Attributes.java (java.base) and JdpController.java (jdk.management.agent)
Hi, please review a simple import cleanup fix for java/util/jar/Attributes.java and sun/management/jdp/JdpController.java. Bug: https://bugs.openjdk.java.net/browse/JDK-8198539 Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8198539.0/ Thanks and best regards Christoph
RE: Oracle Java 8u161 regression in XML Schema Factory
Hi Bernd, ok, your issue must be something different then. Best regards Christoph > -Original Message- > From: core-libs-dev [mailto:core-libs-dev-boun...@openjdk.java.net] On > Behalf Of Bernd Eckenfels > Sent: Mittwoch, 21. Februar 2018 20:01 > To: OpenJDK Dev list > Subject: Re: Oracle Java 8u161 regression in XML Schema Factory > > Hallo Christoph, > > Yes the Tests fail with -Djdk.xml.overrideDefaultParser=true (and false) as > well. > > Gruss > Bernd > -- > http://bernd.eckenfels.net > > Von: Langer, Christoph > Gesendet: Mittwoch, 21. Februar 2018 16:16 > An: Bernd; OpenJDK Dev list; huizhe.w...@oracle.com > Betreff: RE: Oracle Java 8u161 regression in XML Schema Factory > > Hi Bernd, > > would your test still fail with system property "- > Djdk.xml.overrideDefaultParser=true"? > > I debugged a similar issue (if not the same) today and found that with change > http://hg.openjdk.java.net/jdk8u/jdk8u-dev/jaxp/rev/08a44c164993 the > default behavior for Xalan changed a bit. > > Before that change, the services mechanism was enabled to load the SAX > and DOM implementations for the use of Xalan. E.g. when > TransformerFactory.newInstance() was called, the services mechanism was > used and in the TransformerFactory (at least if it was the default JDK > implementation), the property to use the services mechanism was set as > well - wich was used to retrieve SAX and DOM parser implementations. > > And what hit me even more was that SAX2DOM had its private field > "DocumentBuilderFactory _factory" automatically initialized by a call to > DocumentBuilderFactory.newInstance() which in turn would resolve any > custom DBF implementation if it were on classpath. Now, with the new > implementation, the _factory in SAX2DOM will be initialized with the > "overrideDefaultParser " setting coming from the Transformer Factory. So, > by setting -Djdk.xml.overrideDefaultParser=true, you will restore the old > behavior. > > @Joe: Is there some documentation for this change in default behavior that > came with JDK8u161? I assume it is for higher security and cannot be > reverted (e.g. by setting the feature default for > Djdk.xml.overrideDefaultParser to true). > > Best regards > Christoph > > > -Original Message- > > From: core-libs-dev [mailto:core-libs-dev-boun...@openjdk.java.net] On > > Behalf Of Bernd > > Sent: Dienstag, 13. Februar 2018 22:55 > > To: OpenJDK Dev list > > Subject: Re: Oracle Java 8u161 regression in XML Schema Factory > > > > Hello, > > > > > > 2018-01-25 17:41 GMT+01:00 Seán Coffey : > > > > > > > > Classes nearer to those below were touched via JDK-8186080: Transform > > XML > > > interfaces > > > http://hg.openjdk.java.net/jdk8u/jdk8u/jdk/rev/cb84156d54b2 > > > http://hg.openjdk.java.net/jdk8u/jdk8u/jaxp/rev/08a44c164993 > > > > > > This may be connected with some tools trying to redefine the classes > > > perhaps. Needs more investigating. Perhaps the XMLSchemaLoader > > changes are > > > a factor ? > > > > > > > I have ben able to extract a minimal reproducer. It is not related to > > XMLUnit, only to powermock. If it instruments com.sun but not javax.xml > > (and other combinations) then it fails. > > > > For details see the readme in this maven project: > > > > https://github.com/ecki/reproduce-schemaex > > > > I also found a way to make it work with both versions, so its no longer an > > issue for me, but there is definitely some changes (which might also be > > triggered in AppServers or OSGi containers with partially reconfigured > > implementations. Not sure if you want to investigate deeper). > > > > Gruss > > Bernd > > > > > > Here is the stacktrace anyway: > > >> > > >> com.sun.org.apache.xerces.internal.impl.dv.DVFactoryException: > Schema > > >> factory class > > >> com.sun.org.apache.xerces.internal.impl.dv.xs.SchemaDVFactoryImpl > > does > > >> not > > >> extend from SchemaDVFactory. > > >> at > > >> com.sun.org.apache.xerces.internal.impl.dv.SchemaDVFactory. > > >> getInstance(SchemaDVFactory.java:75) > > >> at > > >> com.sun.org.apache.xerces.internal.impl.dv.SchemaDVFactory. > > >> getInstance(SchemaDVFactory.java:57) > > >> at > > >> com.sun.org.apache.xerces.internal.impl.xs.XMLSchemaLoader. > > >> reset(XMLSchemaLoader.java:1024) > > >> at
RE: Oracle Java 8u161 regression in XML Schema Factory
Hi Joe, thanks for the clarification. It would be good to have a place of documentation where one could refer customers to. Thanks Christoph > -Original Message- > From: Joe Wang [mailto:huizhe.w...@oracle.com] > Sent: Mittwoch, 21. Februar 2018 21:50 > To: Langer, Christoph > Cc: Bernd ; OpenJDK Dev list d...@openjdk.java.net> > Subject: Re: Oracle Java 8u161 regression in XML Schema Factory > > > > > > @Joe: Is there some documentation for this change in default behavior > that came with JDK8u161? I assume it is for higher security and cannot be > reverted (e.g. by setting the feature default for > Djdk.xml.overrideDefaultParser to true). > > It is indeed. It was a customer's request. Customers' complaints were > that a factory created through the official API could in many cases, > unknowingly to the customers, load 3rd party parsers that didn't > necessarily implement the security features added since JDK7u40 and 8. > For customers, this behavior was a security concern. It was particularly > inconvenient to users who might have some 3rd party libraries that just > happen to be in their environment. > > This change was not mentioned in the release notes. I'll check whether > we could find a right place for a note of this change. The 8u161 release > notes would have been a good place to do so. > > Best, > Joe > > > > > Best regards > > Christoph > > > >> -Original Message- > >> From: core-libs-dev [mailto:core-libs-dev-boun...@openjdk.java.net] On > >> Behalf Of Bernd > >> Sent: Dienstag, 13. Februar 2018 22:55 > >> To: OpenJDK Dev list > >> Subject: Re: Oracle Java 8u161 regression in XML Schema Factory > >> > >> Hello, > >> > >> > >> 2018-01-25 17:41 GMT+01:00 Seán Coffey: > >> > >>> Classes nearer to those below were touched via JDK-8186080: Transform > >> XML > >>> interfaces > >>> http://hg.openjdk.java.net/jdk8u/jdk8u/jdk/rev/cb84156d54b2 > >>> http://hg.openjdk.java.net/jdk8u/jdk8u/jaxp/rev/08a44c164993 > >>> > >>> This may be connected with some tools trying to redefine the classes > >>> perhaps. Needs more investigating. Perhaps the XMLSchemaLoader > >> changes are > >>> a factor ? > >>> > >> I have ben able to extract a minimal reproducer. It is not related to > >> XMLUnit, only to powermock. If it instruments com.sun but not javax.xml > >> (and other combinations) then it fails. > >> > >> For details see the readme in this maven project: > >> > >> https://github.com/ecki/reproduce-schemaex > >> > >> I also found a way to make it work with both versions, so its no longer an > >> issue for me, but there is definitely some changes (which might also be > >> triggered in AppServers or OSGi containers with partially reconfigured > >> implementations. Not sure if you want to investigate deeper). > >> > >> Gruss > >> Bernd > >> > >> > >> Here is the stacktrace anyway: > >>>> com.sun.org.apache.xerces.internal.impl.dv.DVFactoryException: > Schema > >>>> factory class > >>>> com.sun.org.apache.xerces.internal.impl.dv.xs.SchemaDVFactoryImpl > >> does > >>>> not > >>>> extend from SchemaDVFactory. > >>>> at > >>>> com.sun.org.apache.xerces.internal.impl.dv.SchemaDVFactory. > >>>> getInstance(SchemaDVFactory.java:75) > >>>> at > >>>> com.sun.org.apache.xerces.internal.impl.dv.SchemaDVFactory. > >>>> getInstance(SchemaDVFactory.java:57) > >>>> at > >>>> com.sun.org.apache.xerces.internal.impl.xs.XMLSchemaLoader. > >>>> reset(XMLSchemaLoader.java:1024) > >>>> at > >>>> com.sun.org.apache.xerces.internal.impl.xs.XMLSchemaLoader. > >>>> loadGrammar(XMLSchemaLoader.java:556) > >>>> at > >>>> com.sun.org.apache.xerces.internal.impl.xs.XMLSchemaLoader. > >>>> loadGrammar(XMLSchemaLoader.java:535) > >>>> at > >>>> com.sun.org.apache.xerces.internal.jaxp.validation.XMLSchema > >>>> Factory.newSchema(XMLSchemaFactory.java:254) > >>>> at javax.xml.validation.SchemaFactory.newSchema(SchemaFactory. > >>>> java:638) > >>>> at javax.xml.validation.SchemaFactory.newSchema(SchemaFactory. &
RE: RFR (XS): 8198539: Cleanup of unused imports in java/util/jar/Attributes.java (java.base) and JdpController.java (jdk.management.agent)
Thanks, Thomas. From: Thomas Stüfe [mailto:thomas.stu...@gmail.com] Sent: Donnerstag, 22. Februar 2018 09:54 To: Langer, Christoph Cc: Java Core Libs ; serviceability-...@openjdk.java.net; andrew_m_leon...@uk.ibm.com Subject: Re: RFR (XS): 8198539: Cleanup of unused imports in java/util/jar/Attributes.java (java.base) and JdpController.java (jdk.management.agent) Hi Christoph, Looks fine. .. Thomas On Feb 22, 2018 09:42, "Langer, Christoph" mailto:christoph.lan...@sap.com>> wrote: Hi, please review a simple import cleanup fix for java/util/jar/Attributes.java and sun/management/jdp/JdpController.java. Bug: https://bugs.openjdk.java.net/browse/JDK-8198539 Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8198539.0/ Thanks and best regards Christoph
RE: Oracle Java 8u161 regression in XML Schema Factory
Thanks, Joe. > -Original Message- > From: Joe Wang [mailto:huizhe.w...@oracle.com] > Sent: Donnerstag, 1. März 2018 20:11 > To: Langer, Christoph > Cc: OpenJDK Dev list > Subject: Re: Oracle Java 8u161 regression in XML Schema Factory > > Hi Christoph and all, > > Just wanted to let you know that we're in progress to update the release > notes for 6u181/7u171/8u161 with a doc for this change. > > Thanks, > Joe > > On 2/22/2018 12:47 AM, Langer, Christoph wrote: > > Hi Joe, > > > > thanks for the clarification. It would be good to have a place of > documentation where one could refer customers to. > > > > Thanks > > Christoph > > > >> -Original Message- > >> From: Joe Wang [mailto:huizhe.w...@oracle.com] > >> Sent: Mittwoch, 21. Februar 2018 21:50 > >> To: Langer, Christoph > >> Cc: Bernd ; OpenJDK Dev list >> d...@openjdk.java.net> > >> Subject: Re: Oracle Java 8u161 regression in XML Schema Factory > >> > >> > >>> @Joe: Is there some documentation for this change in default behavior > >> that came with JDK8u161? I assume it is for higher security and cannot be > >> reverted (e.g. by setting the feature default for > >> Djdk.xml.overrideDefaultParser to true). > >> > >> It is indeed. It was a customer's request. Customers' complaints were > >> that a factory created through the official API could in many cases, > >> unknowingly to the customers, load 3rd party parsers that didn't > >> necessarily implement the security features added since JDK7u40 and 8. > >> For customers, this behavior was a security concern. It was particularly > >> inconvenient to users who might have some 3rd party libraries that just > >> happen to be in their environment. > >> > >> This change was not mentioned in the release notes. I'll check whether > >> we could find a right place for a note of this change. The 8u161 release > >> notes would have been a good place to do so. > >> > >> Best, > >> Joe > >> > >>> Best regards > >>> Christoph > >>> > >>>> -Original Message- > >>>> From: core-libs-dev [mailto:core-libs-dev-boun...@openjdk.java.net] > On > >>>> Behalf Of Bernd > >>>> Sent: Dienstag, 13. Februar 2018 22:55 > >>>> To: OpenJDK Dev list > >>>> Subject: Re: Oracle Java 8u161 regression in XML Schema Factory > >>>> > >>>> Hello, > >>>> > >>>> > >>>> 2018-01-25 17:41 GMT+01:00 Seán Coffey: > >>>> > >>>>> Classes nearer to those below were touched via JDK-8186080: > Transform > >>>> XML > >>>>> interfaces > >>>>> http://hg.openjdk.java.net/jdk8u/jdk8u/jdk/rev/cb84156d54b2 > >>>>> http://hg.openjdk.java.net/jdk8u/jdk8u/jaxp/rev/08a44c164993 > >>>>> > >>>>> This may be connected with some tools trying to redefine the classes > >>>>> perhaps. Needs more investigating. Perhaps the XMLSchemaLoader > >>>> changes are > >>>>> a factor ? > >>>>> > >>>> I have ben able to extract a minimal reproducer. It is not related to > >>>> XMLUnit, only to powermock. If it instruments com.sun but not > javax.xml > >>>> (and other combinations) then it fails. > >>>> > >>>> For details see the readme in this maven project: > >>>> > >>>> https://github.com/ecki/reproduce-schemaex > >>>> > >>>> I also found a way to make it work with both versions, so its no longer > an > >>>> issue for me, but there is definitely some changes (which might also be > >>>> triggered in AppServers or OSGi containers with partially reconfigured > >>>> implementations. Not sure if you want to investigate deeper). > >>>> > >>>> Gruss > >>>> Bernd > >>>> > >>>> > >>>> Here is the stacktrace anyway: > >>>>>> com.sun.org.apache.xerces.internal.impl.dv.DVFactoryException: > >> Schema > >>>>>> factory class > >>>>>> > com.sun.org.apache.xerces.internal.impl.dv.xs.SchemaDVFactoryImpl > >>>> does > >>>>>> not > >>>&g
RE: RFR 8199464 [11] Remove remaining vestiges of Java_sun_reflect_Reflection_getCallerClass
Looks good, Chris. > -Original Message- > From: core-libs-dev [mailto:core-libs-dev-boun...@openjdk.java.net] On > Behalf Of Chris Hegarty > Sent: Mittwoch, 14. März 2018 13:57 > To: build-dev ; Core-Libs-Dev d...@openjdk.java.net> > Subject: RFR 8199464 [11] Remove remaining vestiges of > Java_sun_reflect_Reflection_getCallerClass > > This is a review request to remove remaining vestiges of > Java_sun_reflect_Reflection_getCallerClass. > > JDK-8179424 removed terminally deprecated > jdk.unsupported/sun.reflect.Reflection.getCallerClass(int), these > references are to the no-args getCallerClass that was removed a long > time ago. These should be cleaned up anyway. > > > diff --git a/make/mapfiles/libjava/reorder-sparc > b/make/mapfiles/libjava/reorder-sparc > --- a/make/mapfiles/libjava/reorder-sparc > +++ b/make/mapfiles/libjava/reorder-sparc > @@ -26,7 +26,6 @@ > text: .text%Java_java_io_FileDescriptor_initIDs; > text: .text%Java_java_io_FileOutputStream_initIDs; > text: .text%Java_java_lang_System_setIn0; > -text: .text%Java_sun_reflect_Reflection_getCallerClass__; > text: .text%Java_java_lang_Class_forName0; > text: .text%Java_java_lang_Object_getClass; > text: .text%Java_sun_reflect_Reflection_getClassAccessFlags; > diff --git a/make/mapfiles/libjava/reorder-sparcv9 > b/make/mapfiles/libjava/reorder-sparcv9 > --- a/make/mapfiles/libjava/reorder-sparcv9 > +++ b/make/mapfiles/libjava/reorder-sparcv9 > @@ -25,7 +25,6 @@ > text: .text%Java_java_io_FileDescriptor_initIDs; > text: .text%Java_java_io_FileOutputStream_initIDs; > text: .text%Java_java_lang_System_setIn0; > -text: .text%Java_sun_reflect_Reflection_getCallerClass__; > text: .text%Java_java_lang_Class_forName0; > text: .text%Java_java_lang_String_intern; > text: .text%Java_java_lang_StringUTF16_isBigEndian; > diff --git a/make/mapfiles/libjava/reorder-x86 > b/make/mapfiles/libjava/reorder-x86 > --- a/make/mapfiles/libjava/reorder-x86 > +++ b/make/mapfiles/libjava/reorder-x86 > @@ -26,7 +26,6 @@ > text: .text%Java_java_io_FileDescriptor_initIDs; > text: .text%Java_java_io_FileOutputStream_initIDs; > text: .text%Java_java_lang_System_setIn0; > -text: .text%Java_sun_reflect_Reflection_getCallerClass__; > text: .text%Java_java_lang_Class_forName0; > text: .text%Java_java_lang_String_intern; > text: .text%Java_java_lang_StringUTF16_isBigEndian; > > > -Chris.
RE: RFR : 8202322: AIX: symbol visibility flags not support on xlc 12.1
Hi Matthias, to me the change in principal looks good. I'm wondering if it is possible to do a comparison like xlc < 13 (e.g. extract major number before the first dot, then compare numerically) - but maybe it is too complicated and the current single version compare suits our needs ? Best regards Christoph From: Baesken, Matthias Sent: Donnerstag, 26. April 2018 16:14 To: 'build-...@openjdk.java.net' ; ppc-aix-port-...@openjdk.java.net; core-libs-dev@openjdk.java.net Cc: Langer, Christoph ; Simonis, Volker Subject: RFR : 8202322: AIX: symbol visibility flags not support on xlc 12.1 Hello , could you please review this small adjustment to the symbol visibility compilation settings on AIX ? Currently we use XLC 12.1 to compile JDK on AIX . However XLC 12.1 does not support the "-qvisibility=hidden" setting currently set on AIX. It was introduced with XLC 13.1 . Christoph found some info about it here : https://www.ibm.com/developerworks/aix/library/au-aix-symbol-visibility-part2/index.html Setting it only generates hundreds of warnings in the build log , warnings look like this : XlC12.1 bash-4.4$ xlC -qversion IBM XL C/C++ for AIX, V12.1 (5765-J02, 5725-C72) Version: 12.01..0019 bash-4.4$ xlC -qvisibility=hidden sizeof.c -o sizeof_aixxlc 1506-173 (W) Option visibility=hidden is not valid. Enter xlC for list of valid options. Compare to XLC13.1 bash-3.00$ xlC -qversion IBM XL C/C++ for AIX, V13.1 (5725-C72, 5765-J07) Version: 13.01..0008 bash-3.00$ xlC -qvisibility=default sizeof.c -o sizeof_aixxlc bash-3.00$ xlC -qvisibility=hidden sizeof.c -o sizeof_aixxlc So it is better to avoid setting these flags when using xlc12.1 . Please review : Bug : https://bugs.openjdk.java.net/browse/JDK-8202322 Change : http://cr.openjdk.java.net/~mbaesken/webrevs/8202322/ Best regards, Matthias
RE: RFR : 8202322: AIX: symbol visibility flags not support on xlc 12.1
Hi Thomas, Look at this blog: https://www.ibm.com/developerworks/aix/library/au-aix-symbol-visibility-part2/index.html if I understand it correctly, the xlc 12 default behavior should be like what we'd expect from -qvisibility=hidden. Best regards Christoph > -Original Message- > From: build-dev [mailto:build-dev-boun...@openjdk.java.net] On Behalf Of > Thomas Stüfe > Sent: Freitag, 27. April 2018 06:55 > To: Volker Simonis ; Baesken, Matthias > > Cc: Simonis, Volker ; ppc-aix-port- > d...@openjdk.java.net; core-libs-dev@openjdk.java.net; build- > d...@openjdk.java.net > Subject: Re: RFR : 8202322: AIX: symbol visibility flags not support on xlc > 12.1 > > Hi, > > This was added by "8200178: Remove mapfiles for JDK native libraries". > But if the flag is not accepted, what is the default behavior? Do we > now export everything? > > I'd like to understand this first before removing the flag to get rid > of the warnings. > > Best Regards, Thomas > > On Thu, Apr 26, 2018 at 5:16 PM, Volker Simonis > wrote: > > Hi Matthias, > > > > after Bhaktavatsal Reddy's report about the problems with > > "-qvisibility" with xlC 13 and taking into account that we can't test > > this anyway because we don't currently have xlC 13 > > on our machines I think it would be best to completely remove this > > option for now on AIX. Once we get xlC 13 we can revisit the issue. > > > > Thanks, > > Volker > > > > > > On Thu, Apr 26, 2018 at 4:59 PM, Bhaktavatsal R Maram > > wrote: > >> Hi Matthias, > >> > >> At this point, I think we can remove the flag as you found that it is not > supported in XLC < 13. And with XLC 13, it require more work to use this flag. > >> > >> Thanks, > >> Bhaktavatsal Reddy > >> > >> > >> > >> -"Baesken, Matthias" wrote: - > >> To: "Langer, Christoph" , "'build- > d...@openjdk.java.net'" , "ppc-aix-port- > d...@openjdk.java.net" , "core-libs- > d...@openjdk.java.net" > >> From: "Baesken, Matthias" > >> Date: 04/26/2018 08:23PM > >> Cc: "Simonis, Volker" , Bhaktavatsal R Maram > > >> Subject: RE: RFR : 8202322: AIX: symbol visibility flags not support on xlc > 12.1 > >> > >> > >> Hello Christoph, I think all XLC versions < 12.1 are unsupported > >> (and > probably not working anyway) for the OpenJDK build . > >> I am only aware of XLC versions 12.1 and 13.1which work / in > >> case of > 13.1 “might” work for OpenJDK compilation . > >> And for 12.1 I want to remove the flags . > >> > >> ( waiting for the feedback of Bhaktavatsal Reddy , in case he > >> prefers it > I remove them for all xlC versions including 13.1 ) > >> > >> Best regards, Matthias > >> > >> > >> > >> > >> > >> > >> From: Langer, Christoph > >> Sent: Donnerstag, 26. April 2018 16:38 > >> To: Baesken, Matthias ; 'build- > d...@openjdk.java.net' ; ppc-aix-port- > d...@openjdk.java.net; core-libs-dev@openjdk.java.net > >> Cc: Simonis, Volker > >> Subject: RE: RFR : 8202322: AIX: symbol visibility flags not support on > >> xlc > 12.1 > >> > >> Hi Matthias, > >> > >> to me the change in principal looks good. > >> > >> I’m wondering if it is possible to do a comparison like xlc < 13 (e.g. > >> extract > major number before the first dot, then compare numerically) – but maybe it > is too complicated and the current single version compare suits our needs ? > >> > >> Best regards > >> Christoph > >> > >> > >> > >> > >> From: Baesken, Matthias > >> Sent: Donnerstag, 26. April 2018 16:14 > >> To: 'build-...@openjdk.java.net' ; ppc- > aix-port-...@openjdk.java.net; core-libs-dev@openjdk.java.net > >> Cc: Langer, Christoph ; Simonis, Volker > > >> Subject: RFR : 8202322: AIX: symbol visibility flags not support on xlc > >> 12.1 > >> > >> Hello , could you please review this small adjustment to the symbol > visibility compilation settings on AIX ? > >> Currently we use XLC 12.1 to compile JDK on AIX . > >> > >> However XLC 12.1 does not support the “-qvisibility=hidden” setting > currently set on AIX. > >> I
RE: RFR : 8202322: AIX: symbol visibility flags not support on xlc 12.1
Hi Thomas, let me cite one section from the article: -- Visibility attribute and backward compatibility on AIX As we know from the previous article, on AIX, symbols are not visible by default unless we export them at the linking stage, either manually or with the help of CreateExportList. However, on Linux, symbols are, by default, with export (namely default) visibility. This brings a gap between the AIX visibility attribute and the GNU visibility attribute. To be backward compatible, on AIX, XL C/C++ would not set all the symbols to be exported like Linux. It might consider symbol without any visibility setting to be an unspecific visibility, which aligns with an old AIX implementation. For such symbols, AIX compiler, linker, and related tools would handle it as before. However, unspecific visibility does not mean that the symbol is internal or invisible at all. It is just a visibility that is specially designed to keep the compatibility. ... -- It says in the first sentence: " As we know from the previous article, on AIX, symbols are not visible by default unless we export them at the linking stage, either manually or with the help of CreateExportList". I guess that is why I was under the impression that with xlc12 symbols would not be visible... Best regards Christoph > -Original Message- > From: Thomas Stüfe [mailto:thomas.stu...@gmail.com] > Sent: Freitag, 27. April 2018 09:21 > To: Langer, Christoph > Cc: Volker Simonis ; Baesken, Matthias > ; Simonis, Volker ; > ppc-aix-port-...@openjdk.java.net; core-libs-dev@openjdk.java.net; build- > d...@openjdk.java.net > Subject: Re: RFR : 8202322: AIX: symbol visibility flags not support on xlc > 12.1 > > Hi Christoph > > On Fri, Apr 27, 2018 at 8:07 AM, Langer, Christoph > wrote: > > Hi Thomas, > > > > Look at this blog: https://www.ibm.com/developerworks/aix/library/au- > aix-symbol-visibility-part2/index.html > > > > if I understand it correctly, the xlc 12 default behavior should be like > > what > we'd expect from -qvisibility=hidden. > > > > Where in this article do you read this? > > ..Thomas > > > Best regards > > Christoph > > > >> -Original Message- > >> From: build-dev [mailto:build-dev-boun...@openjdk.java.net] On Behalf > Of > >> Thomas Stüfe > >> Sent: Freitag, 27. April 2018 06:55 > >> To: Volker Simonis ; Baesken, Matthias > >> > >> Cc: Simonis, Volker ; ppc-aix-port- > >> d...@openjdk.java.net; core-libs-dev@openjdk.java.net; build- > >> d...@openjdk.java.net > >> Subject: Re: RFR : 8202322: AIX: symbol visibility flags not support on xlc > 12.1 > >> > >> Hi, > >> > >> This was added by "8200178: Remove mapfiles for JDK native libraries". > >> But if the flag is not accepted, what is the default behavior? Do we > >> now export everything? > >> > >> I'd like to understand this first before removing the flag to get rid > >> of the warnings. > >> > >> Best Regards, Thomas > >> > >> On Thu, Apr 26, 2018 at 5:16 PM, Volker Simonis > >> wrote: > >> > Hi Matthias, > >> > > >> > after Bhaktavatsal Reddy's report about the problems with > >> > "-qvisibility" with xlC 13 and taking into account that we can't test > >> > this anyway because we don't currently have xlC 13 > >> > on our machines I think it would be best to completely remove this > >> > option for now on AIX. Once we get xlC 13 we can revisit the issue. > >> > > >> > Thanks, > >> > Volker > >> > > >> > > >> > On Thu, Apr 26, 2018 at 4:59 PM, Bhaktavatsal R Maram > >> > wrote: > >> >> Hi Matthias, > >> >> > >> >> At this point, I think we can remove the flag as you found that it is > >> >> not > >> supported in XLC < 13. And with XLC 13, it require more work to use this > flag. > >> >> > >> >> Thanks, > >> >> Bhaktavatsal Reddy > >> >> > >> >> > >> >> > >> >> -"Baesken, Matthias" wrote: - > >> >> To: "Langer, Christoph" , "'build- > >> d...@openjdk.java.net'" , "ppc-aix-port- > >> d...@openjdk.java.net" , "core- > libs- > >> d...@openjdk.java.net" > >> >> From: "Baesken, Matthias&
RE: RFR of JDK-8202291,java/rmi/Naming/LookupIPv6.java failed with Connection refused
Hi Hamlin, to me this looks good. Best regards Christoph > -Original Message- > From: core-libs-dev [mailto:core-libs-dev-boun...@openjdk.java.net] On > Behalf Of Hamlin Li > Sent: Montag, 7. Mai 2018 04:08 > To: core-libs-dev@openjdk.java.net > Subject: Re: RFR of JDK-8202291,java/rmi/Naming/LookupIPv6.java failed > with Connection refused > > Is someone available to review the following patch? > > Thank you > > -Hamlin > > > On 28/04/2018 11:32 AM, Hamlin Li wrote: > > would you please review the patch? > > > > bug: https://bugs.openjdk.java.net/browse/JDK-8202291 > > > > webrev: http://cr.openjdk.java.net/~mli/8202291/webrev.00/ > > > > In original code, there is a window between get registry and rebind > > which could cause connection refuse. > > > > > > Thank you > > > > -Hamlin > >
RFR (S): 8202915: [JAXP] Performance enhancements and cleanups in com.sun.org.apache.xerces.internal.impl.dtd.XMLDTDValidator
Hi, here is a little contribution that has been in SAP's JVM for several. In method XMLDTDValidator::getAttributeTypeName(XMLAttributeDecl attrDecl), a StringBuilder, preallocated to the expected length vs. a default StringBuffer could help performance for "XMLSimpleType.TYPE_ENUMERATION". I also took the chance to do some further cleanups in that class (removing/uncommenting unused variables and code lines). Please help reiviewing. Bug: https://bugs.openjdk.java.net/browse/JDK-8202915 Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8202915.0/ Thank you Christoph
RE: RFR (S): 8202915: [JAXP] Performance enhancements and cleanups in com.sun.org.apache.xerces.internal.impl.dtd.XMLDTDValidator
One addition: I think it would be more appropriate to completely remove the commented lines but I didn't dare to do so yet. What do you think? From: Langer, Christoph Sent: Donnerstag, 10. Mai 2018 17:51 To: core-libs-dev@openjdk.java.net; 'Joe Wang' Subject: RFR (S): 8202915: [JAXP] Performance enhancements and cleanups in com.sun.org.apache.xerces.internal.impl.dtd.XMLDTDValidator Hi, here is a little contribution that has been in SAP's JVM for several. In method XMLDTDValidator::getAttributeTypeName(XMLAttributeDecl attrDecl), a StringBuilder, preallocated to the expected length vs. a default StringBuffer could help performance for "XMLSimpleType.TYPE_ENUMERATION". I also took the chance to do some further cleanups in that class (removing/uncommenting unused variables and code lines). Please help reiviewing. Bug: https://bugs.openjdk.java.net/browse/JDK-8202915 Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8202915.0/ Thank you Christoph
RE: RFR (S): 8202915: [JAXP] Performance enhancements and cleanups in com.sun.org.apache.xerces.internal.impl.dtd.XMLDTDValidator
Hi Joe, thanks for reviewing. Then this would be my proposal with removing the obsolete stuff: http://cr.openjdk.java.net/~clanger/webrevs/8202915.1/ I also removed some code that was already commented out (with some BUG tags...) Are you fine with that? I'll run it through the jtreg suite before pushing, of course. Best regards Christoph From: Joe Wang [mailto:huizhe.w...@oracle.com] Sent: Donnerstag, 10. Mai 2018 20:08 To: Langer, Christoph ; core-libs-dev@openjdk.java.net Subject: Re: RFR (S): 8202915: [JAXP] Performance enhancements and cleanups in com.sun.org.apache.xerces.internal.impl.dtd.XMLDTDValidator Thanks for bringing in what's been in SAP's JVM. The change looks good to me. On whether to remove the commented lines (the unused getContentSpecType method), I would say yes since that was replaced with that of a DTDGrammar method a long long time ago :-) Cheers, Joe On 5/10/2018 9:01 AM, Langer, Christoph wrote: One addition: I think it would be more appropriate to completely remove the commented lines but I didn't dare to do so yet. What do you think? From: Langer, Christoph Sent: Donnerstag, 10. Mai 2018 17:51 To: core-libs-dev@openjdk.java.net<mailto:core-libs-dev@openjdk.java.net>; 'Joe Wang' <mailto:huizhe.w...@oracle.com> Subject: RFR (S): 8202915: [JAXP] Performance enhancements and cleanups in com.sun.org.apache.xerces.internal.impl.dtd.XMLDTDValidator Hi, here is a little contribution that has been in SAP's JVM for several. In method XMLDTDValidator::getAttributeTypeName(XMLAttributeDecl attrDecl), a StringBuilder, preallocated to the expected length vs. a default StringBuffer could help performance for "XMLSimpleType.TYPE_ENUMERATION". I also took the chance to do some further cleanups in that class (removing/uncommenting unused variables and code lines). Please help reiviewing. Bug: https://bugs.openjdk.java.net/browse/JDK-8202915 Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8202915.0/<http://cr.openjdk.java.net/%7Eclanger/webrevs/8202915.0/> Thank you Christoph
RE: [11] RFR for JDK-8202544: Hide unused exports in libzip
Hi Alexey, looks good to me. Symbols don't seem to be needed outside libzip (java.base). Best regards Christoph > -Original Message- > From: build-dev [mailto:build-dev-boun...@openjdk.java.net] On Behalf Of > Alexey Ivanov > Sent: Mittwoch, 9. Mai 2018 16:35 > To: core-libs ; hotspot-dev d...@openjdk.java.net> > Cc: build-dev > Subject: Re: [11] RFR for JDK-8202544: Hide unused exports in libzip > > Any volunteers from core-libs and/or hotspot? > > Thank you, > Alexey > > On 02/05/2018 13:02, Magnus Ihse Bursie wrote: > > Looks good to me, FWIW. > > > > /Magnus > > > >> 2 maj 2018 kl. 13:52 skrev Alexey Ivanov : > >> > >> Hi, > >> > >> Could you please review the following fix for jdk11? > >> > >> bug: https://bugs.openjdk.java.net/browse/JDK-8202544 > >> webrev: http://cr.openjdk.java.net/~aivanov/8202544/jdk11/webrev.0/ > >> > >> The following exported functions in libzip are not used: > >> ZIP_GetEntry, ZIP_FreeEntry, ZIP_Lock, ZIP_Unlock, ZIP_Read > >> > >> I removed JNIEXPORT modifiers from these functions. With the fix, > they're not exported on Windows; on Linux they're listed as Local rather than > Global. > >> > >> I ran tests, no failures. > >> > >> > >> Thank you in advance. > >> > >> Regards, > >> Alexey
RE: [11] RFR for JDK-8202544: Hide unused exports in libzip
Hi Alexey, good catch, I missed that. Best regards Christoph > -Original Message- > From: Alexey Ivanov [mailto:alexey.iva...@oracle.com] > Sent: Freitag, 11. Mai 2018 13:21 > To: Langer, Christoph ; core-libs d...@openjdk.java.net>; hotspot-dev ; > build-dev ; Magnus Ihse Bursie > > Subject: Re: [11] RFR for JDK-8202544: Hide unused exports in libzip > > Hi Christoph, > > Thank you for your review. > > I was checking the changeset before pushing and noticed I had forgotten > to remove JNIEXPORT modifier from ZIP_GetEntry in zip_util.h. > > Here's the updated webrev: > http://cr.openjdk.java.net/~aivanov/8202544/jdk11/webrev.1/ > > The only difference with previous one is in this snippet in zip_util.h: > > -JNIEXPORT jzentry * > +jzentry * > ZIP_GetEntry(jzfile *zip, char *name, jint ulen); > > > Thank you! > > Regards, > Alexey > > On 10/05/2018 22:11, Langer, Christoph wrote: > > Hi Alexey, > > > > looks good to me. Symbols don't seem to be needed outside libzip > (java.base). > > > > Best regards > > Christoph > > > >> -Original Message- > >> From: build-dev [mailto:build-dev-boun...@openjdk.java.net] On Behalf > Of > >> Alexey Ivanov > >> Sent: Mittwoch, 9. Mai 2018 16:35 > >> To: core-libs ; hotspot-dev >> d...@openjdk.java.net> > >> Cc: build-dev > >> Subject: Re: [11] RFR for JDK-8202544: Hide unused exports in libzip > >> > >> Any volunteers from core-libs and/or hotspot? > >> > >> Thank you, > >> Alexey > >> > >> On 02/05/2018 13:02, Magnus Ihse Bursie wrote: > >>> Looks good to me, FWIW. > >>> > >>> /Magnus > >>> > >>>> 2 maj 2018 kl. 13:52 skrev Alexey Ivanov : > >>>> > >>>> Hi, > >>>> > >>>> Could you please review the following fix for jdk11? > >>>> > >>>> bug: https://bugs.openjdk.java.net/browse/JDK-8202544 > >>>> webrev: > http://cr.openjdk.java.net/~aivanov/8202544/jdk11/webrev.0/ > >>>> > >>>> The following exported functions in libzip are not used: > >>>> ZIP_GetEntry, ZIP_FreeEntry, ZIP_Lock, ZIP_Unlock, ZIP_Read > >>>> > >>>> I removed JNIEXPORT modifiers from these functions. With the fix, > >> they're not exported on Windows; on Linux they're listed as Local rather > than > >> Global. > >>>> I ran tests, no failures. > >>>> > >>>> > >>>> Thank you in advance. > >>>> > >>>> Regards, > >>>> Alexey
RE: RFR (S): 8202915: [JAXP] Performance enhancements and cleanups in com.sun.org.apache.xerces.internal.impl.dtd.XMLDTDValidator
Hi Joe, jaxp_all ran well - I'll push this one next week then. Thanks Christoph From: Joe Wang [mailto:huizhe.w...@oracle.com] Sent: Freitag, 11. Mai 2018 00:03 To: Langer, Christoph ; core-libs-dev@openjdk.java.net Subject: Re: RFR (S): 8202915: [JAXP] Performance enhancements and cleanups in com.sun.org.apache.xerces.internal.impl.dtd.XMLDTDValidator Hi Christoph, Thanks for doing this! Yes, I'm fine with the additional cleanup as well. They were so obsolete, like 20 years :-), plus the old bug system was long gone. Yes, running test jaxp_all would be good. Best regards, Joe On 5/10/2018 2:08 PM, Langer, Christoph wrote: Hi Joe, thanks for reviewing. Then this would be my proposal with removing the obsolete stuff: http://cr.openjdk.java.net/~clanger/webrevs/8202915.1/<http://cr.openjdk.java.net/%7Eclanger/webrevs/8202915.1/> I also removed some code that was already commented out (with some BUG tags...) Are you fine with that? I'll run it through the jtreg suite before pushing, of course. Best regards Christoph From: Joe Wang [mailto:huizhe.w...@oracle.com] Sent: Donnerstag, 10. Mai 2018 20:08 To: Langer, Christoph <mailto:christoph.lan...@sap.com>; core-libs-dev@openjdk.java.net<mailto:core-libs-dev@openjdk.java.net> Subject: Re: RFR (S): 8202915: [JAXP] Performance enhancements and cleanups in com.sun.org.apache.xerces.internal.impl.dtd.XMLDTDValidator Thanks for bringing in what's been in SAP's JVM. The change looks good to me. On whether to remove the commented lines (the unused getContentSpecType method), I would say yes since that was replaced with that of a DTDGrammar method a long long time ago :-) Cheers, Joe On 5/10/2018 9:01 AM, Langer, Christoph wrote: One addition: I think it would be more appropriate to completely remove the commented lines but I didn't dare to do so yet. What do you think? From: Langer, Christoph Sent: Donnerstag, 10. Mai 2018 17:51 To: core-libs-dev@openjdk.java.net<mailto:core-libs-dev@openjdk.java.net>; 'Joe Wang' <mailto:huizhe.w...@oracle.com> Subject: RFR (S): 8202915: [JAXP] Performance enhancements and cleanups in com.sun.org.apache.xerces.internal.impl.dtd.XMLDTDValidator Hi, here is a little contribution that has been in SAP's JVM for several. In method XMLDTDValidator::getAttributeTypeName(XMLAttributeDecl attrDecl), a StringBuilder, preallocated to the expected length vs. a default StringBuffer could help performance for "XMLSimpleType.TYPE_ENUMERATION". I also took the chance to do some further cleanups in that class (removing/uncommenting unused variables and code lines). Please help reiviewing. Bug: https://bugs.openjdk.java.net/browse/JDK-8202915 Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8202915.0/<http://cr.openjdk.java.net/%7Eclanger/webrevs/8202915.0/> Thank you Christoph
RE: RFR : 8202322: AIX: symbol visibility flags not support on xlc 12.1
Hi Matthias, yes, reviewed. Best regards Christoph From: Baesken, Matthias Sent: Mittwoch, 16. Mai 2018 09:06 To: Langer, Christoph ; 'build-...@openjdk.java.net' ; ppc-aix-port-...@openjdk.java.net; core-libs-dev@openjdk.java.net Cc: Lindenmaier, Goetz Subject: RE: RFR : 8202322: AIX: symbol visibility flags not support on xlc 12.1 Hi Christoph can I add you as second reviewer (other reviewer was Erik Joelsson) can push the change ? Best regards, Matthias From: Langer, Christoph Sent: Donnerstag, 26. April 2018 16:38 To: Baesken, Matthias mailto:matthias.baes...@sap.com>>; 'build-...@openjdk.java.net' mailto:build-...@openjdk.java.net>>; ppc-aix-port-...@openjdk.java.net<mailto:ppc-aix-port-...@openjdk.java.net>; core-libs-dev@openjdk.java.net<mailto:core-libs-dev@openjdk.java.net> Cc: Simonis, Volker mailto:volker.simo...@sap.com>> Subject: RE: RFR : 8202322: AIX: symbol visibility flags not support on xlc 12.1 Hi Matthias, to me the change in principal looks good. I'm wondering if it is possible to do a comparison like xlc < 13 (e.g. extract major number before the first dot, then compare numerically) - but maybe it is too complicated and the current single version compare suits our needs ? Best regards Christoph From: Baesken, Matthias Sent: Donnerstag, 26. April 2018 16:14 To: 'build-...@openjdk.java.net' mailto:build-...@openjdk.java.net>>; ppc-aix-port-...@openjdk.java.net<mailto:ppc-aix-port-...@openjdk.java.net>; core-libs-dev@openjdk.java.net<mailto:core-libs-dev@openjdk.java.net> Cc: Langer, Christoph mailto:christoph.lan...@sap.com>>; Simonis, Volker mailto:volker.simo...@sap.com>> Subject: RFR : 8202322: AIX: symbol visibility flags not support on xlc 12.1 Hello , could you please review this small adjustment to the symbol visibility compilation settings on AIX ? Currently we use XLC 12.1 to compile JDK on AIX . However XLC 12.1 does not support the "-qvisibility=hidden" setting currently set on AIX. It was introduced with XLC 13.1 . Christoph found some info about it here : https://www.ibm.com/developerworks/aix/library/au-aix-symbol-visibility-part2/index.html Setting it only generates hundreds of warnings in the build log , warnings look like this : XlC12.1 bash-4.4$ xlC -qversion IBM XL C/C++ for AIX, V12.1 (5765-J02, 5725-C72) Version: 12.01..0019 bash-4.4$ xlC -qvisibility=hidden sizeof.c -o sizeof_aixxlc 1506-173 (W) Option visibility=hidden is not valid. Enter xlC for list of valid options. Compare to XLC13.1 bash-3.00$ xlC -qversion IBM XL C/C++ for AIX, V13.1 (5725-C72, 5765-J07) Version: 13.01..0008 bash-3.00$ xlC -qvisibility=default sizeof.c -o sizeof_aixxlc bash-3.00$ xlC -qvisibility=hidden sizeof.c -o sizeof_aixxlc So it is better to avoid setting these flags when using xlc12.1 . Please review : Bug : https://bugs.openjdk.java.net/browse/JDK-8202322 Change : http://cr.openjdk.java.net/~mbaesken/webrevs/8202322/ Best regards, Matthias
RE: RFR : 8202322: AIX: symbol visibility flags not support on xlc 12.1
Hi Ichiroh, we do not use the XLC 13 compiler on AIX yet here at SAP and I believe nobody of my colleagues has played with it yet. So you are on a new playground here 😊 However, I believe the idea in OpenJDK with the abolition of map files is that symbols should be invisible externally unless they are declared exported, e.g. JNIEXPORT. So I would think "-qvisibility=hidden" should be the correct default and whatever JNIEXPORT expands to should contain the right attributes to get that symbol visible. Can you check if either my assumption is completely wrong, JNIEXPORT does not expand to the right thing, XLC 13 has a bug or maybe just sume specific required symbols are not declared correctly? Best regards Christoph > -Original Message- > From: Ichiroh Takiguchi [mailto:taki...@linux.vnet.ibm.com] > Sent: Donnerstag, 31. Mai 2018 09:55 > To: Langer, Christoph > Cc: Baesken, Matthias ; 'build- > d...@openjdk.java.net' ; ppc-aix-port- > d...@openjdk.java.net; core-libs-dev@openjdk.java.net; Lindenmaier, > Goetz > Subject: RE: RFR : 8202322: AIX: symbol visibility flags not support on xlc > 12.1 > > Hello. > 8202322 was integrated into jdk-11+15. > I'm using XLC 13.1.3 on AIX 7.1.4. > Build was failed because of "-qvisibility=hidden" on > make/lib/LibCommon.gmk. > According to "XL C/C++ for AIX 13.1.3" documentation [1], > "-qvisibility=hidden" cannot create shared libraries entry points. > For example, libverify.so was there, but entry points were not resolved > by "-lverify" option. > I think it should be "-qvisibility=default" (I tried, it worked) > or "-qvisibility=protected" (I had not tried) ? > I'm not familiar with -qvisibility option, but I'd like to find out > right way. > > [1] > https://www.ibm.com/support/knowledgecenter/SSGH3R_13.1.3/com.ibm. > xlcpp1313.aix.doc/compiler_ref/opt_visibility.html > > On 2018-05-16 16:08, Langer, Christoph wrote: > > Hi Matthias, > > > > yes, reviewed. > > > > Best regards > > Christoph > > > > From: Baesken, Matthias > > Sent: Mittwoch, 16. Mai 2018 09:06 > > To: Langer, Christoph ; > > 'build-...@openjdk.java.net' ; > > ppc-aix-port-...@openjdk.java.net; core-libs-dev@openjdk.java.net > > Cc: Lindenmaier, Goetz > > Subject: RE: RFR : 8202322: AIX: symbol visibility flags not support on > > xlc 12.1 > > > > Hi Christoph can I add you as second reviewer (other reviewer was > > Erik Joelsson) can push the change ? > > > > Best regards, Matthias > > > > > > > > From: Langer, Christoph > > Sent: Donnerstag, 26. April 2018 16:38 > > To: Baesken, Matthias > > mailto:matthias.baes...@sap.com>>; > > 'build-...@openjdk.java.net' > > mailto:build-...@openjdk.java.net>>; > > ppc-aix-port-...@openjdk.java.net<mailto:ppc-aix-port- > d...@openjdk.java.net>; > > core-libs-dev@openjdk.java.net<mailto:core-libs-dev@openjdk.java.net> > > Cc: Simonis, Volker > > mailto:volker.simo...@sap.com>> > > Subject: RE: RFR : 8202322: AIX: symbol visibility flags not support on > > xlc 12.1 > > > > Hi Matthias, > > > > to me the change in principal looks good. > > > > I'm wondering if it is possible to do a comparison like xlc < 13 (e.g. > > extract major number before the first dot, then compare numerically) - > > but maybe it is too complicated and the current single version compare > > suits our needs ? > > > > Best regards > > Christoph > > > > From: Baesken, Matthias > > Sent: Donnerstag, 26. April 2018 16:14 > > To: 'build-...@openjdk.java.net' > > mailto:build-...@openjdk.java.net>>; > > ppc-aix-port-...@openjdk.java.net<mailto:ppc-aix-port- > d...@openjdk.java.net>; > > core-libs-dev@openjdk.java.net<mailto:core-libs-dev@openjdk.java.net> > > Cc: Langer, Christoph > > mailto:christoph.lan...@sap.com>>; Simonis, > > Volker mailto:volker.simo...@sap.com>> > > Subject: RFR : 8202322: AIX: symbol visibility flags not support on xlc > > 12.1 > > > > Hello , could you please review this small adjustment to the symbol > > visibility compilation settings on AIX ? > > Currently we use XLC 12.1 to compile JDK on AIX . > > > > However XLC 12.1 does not support the "-qvisibility=hidden" > > setting currently set on AIX. > > It was introduced with XLC 13.1 . Christoph found some info about it > > here : > > > > https://www.ibm.com/developerw
RE: RFR : 8202322: AIX: symbol visibility flags not support on xlc 12.1
Hi Ichiroh,
your proposal seems to make sense. I have created a bug for this:
https://bugs.openjdk.java.net/browse/JDK-8204541
Can you please generate a webrev (referencing this bug, -c option of
webrev.ksh) and mail it over to me. Then I'll upload it and you can post an
official RFR mail.
Best regards
Christoph
> -Original Message-
> From: Ichiroh Takiguchi [mailto:taki...@linux.vnet.ibm.com]
> Sent: Dienstag, 5. Juni 2018 08:59
> To: Baesken, Matthias
> Cc: Langer, Christoph ; 'build-
> d...@openjdk.java.net' ; ppc-aix-port-
> d...@openjdk.java.net; core-libs-dev@openjdk.java.net; Lindenmaier,
> Goetz
> Subject: RE: RFR : 8202322: AIX: symbol visibility flags not support on xlc
> 12.1
>
> Hello Matthias and Christoph.
> Thank you for your explanations.
>
> I did not have enough knowledge about "visibility".
>
> I created following patches.
>
>
> diff -r 02934b0d661b
> src/java.base/share/native/libjimage/NativeImageBuffer.cpp
> --- a/src/java.base/share/native/libjimage/NativeImageBuffer.cpp Wed
> May
> 30 14:46:28 2018 +0200
> +++ b/src/java.base/share/native/libjimage/NativeImageBuffer.cpp Tue Jun
> 05 12:10:41 2018 +0900
> @@ -39,7 +39,9 @@
> #include "imageFile.hpp"
> #include "inttypes.hpp"
> #include "jimage.hpp"
> +#if !defined(_AIX)
> #include "osSupport.hpp"
> +#endif
>
> #include "jdk_internal_jimage_NativeImageBuffer.h"
>
> diff -r 02934b0d661b src/java.base/unix/native/include/jni_md.h
> --- a/src/java.base/unix/native/include/jni_md.h Wed May 30 14:46:28
> 2018 +0200
> +++ b/src/java.base/unix/native/include/jni_md.h Tue Jun 05 12:10:41
> 2018 +0900
> @@ -29,7 +29,8 @@
> #ifndef __has_attribute
> #define __has_attribute(x) 0
> #endif
> -#if (defined(__GNUC__) && ((__GNUC__ > 4) || (__GNUC__ == 4) &&
> (__GNUC_MINOR__ > 2))) || __has_attribute(visibility)
> +#if (defined(__GNUC__) && ((__GNUC__ > 4) || (__GNUC__ == 4) &&
> (__GNUC_MINOR__ > 2))) || __has_attribute(visibility) \
> +|| (defined(_AIX) && (defined(__xlC__) && (__xlC__ >= 0xD01)))
> #ifdef ARM
> #define JNIEXPORT
> __attribute__((externally_visible,visibility("default")))
> #define JNIIMPORT
> __attribute__((externally_visible,visibility("default")))
>
>
> If "osSupport.hpp" was included, XLC++ compiler complained.
> I could not understand, which part was invalid...
> I'm not sure, "osSupport.hpp" is really required on
> NativeImageBuffer.cpp or not...
>
> I added __xlC__ macro to determine XLC/C++'s version# into jni_md.h. [1]
> 0xD01 means 13.1 by hexadecimal.
>
> I checked symbol table by "dump -Tv -X64". [2]
> It seemed it was fine, some symbols were hidden.
>
> Does someone review my code?
>
> [1]
> https://www.ibm.com/support/knowledgecenter/en/SSGH2K_13.1.3/com.i
> bm.xlc1313.aix.doc/compiler_ref/xlmacros.html
> [2]
> https://www.ibm.com/developerworks/aix/library/au-aix-symbol-
> visibility/index.html
>
> On 2018-06-01 21:12, Baesken, Matthias wrote:
> > Hi , my change 8202322 just handled the fact that the
> > visibility - flags are not supported with xlc 12.1 , so setting
> > them generated a TON of compile - time warnings .
> >
> > The introduction of the "-qvisibility=hidden"came with the
> > mapfile removal changes :
> >
> > 8200358: Remove mapfiles for JDK executables
> > http://hg.openjdk.java.net/jdk/jdk/rev/210cf224b690
> >
> > 8200178: Remove mapfiles for JDK native libraries
> > http://hg.openjdk.java.net/jdk/jdk/rev/396ea30afbd5
> >
> > I guess it might need further testing+adjustments to make the
> > "visibility hiding" work nicely with XLC13 , but currently we
> > build only with XLC12 .
> >
> > As a workaround you might want to remove the "-qvisibility=hidden"
> > setting for XLC 13 as well , like I did for XLC12 with the change
> > 8202322 .
> >
> >
> > Best regards, Matthias
> >
> >
> >
> >
> >> -Original Message-
> >> From: Langer, Christoph
> >> Sent: Freitag, 1. Juni 2018 10:57
> >> To: Ichiroh Takiguchi
> >> Cc: Baesken, Matthias ; 'build-
> >> d...@openjdk.java.net' ; ppc-aix-port-
> >> d...@openjdk.java.net; core-libs-dev@openjdk.java.net; Lindenmaier,
> >> Goetz
> >> Subject: RE: RFR :
RE: [RFR] 8204539: improve error messages in matchJavaTZ [windows]
Hi Matthias, in line 527, where the actual output is done, I think you would need to replace variable 'message' with 'outputMessage', otherwise I guess it won't compile?? Also, mapFileName can't be used at this place, because it has already been free'ed there. But in general the additions make sense and will make it easier to find issues in the tzmappings file. Best regards Christoph From: Baesken, Matthias Sent: Donnerstag, 7. Juni 2018 13:35 To: core-libs-dev@openjdk.java.net Cc: Langer, Christoph ; Lindenmaier, Goetz Subject: [RFR] 8204539: improve error messages in matchJavaTZ [windows] Hi, could you please review this small change that improves the error messages in matchJavaTZ . A reason of the failure is added to the message , and also the offset where the error happened . Bug : https://bugs.openjdk.java.net/browse/JDK-8204539 Webrev : http://cr.openjdk.java.net/~mbaesken/webrevs/8204539/ Thanks, Matthias
RE: RFR : 8204541 Correctly support AIX xlC 13.1 symbol visibility flags
Hi Ichiroh,
what's the exact error message if you #include "osSupport.hpp"? (I have no xlC
13 at hand to try myself...)
Best regards
Christoph
> -Original Message-
> From: Ichiroh Takiguchi [mailto:taki...@linux.vnet.ibm.com]
> Sent: Donnerstag, 7. Juni 2018 14:53
> To: build-...@openjdk.java.net; ppc-aix-port-...@openjdk.java.net; core-
> libs-...@openjdk.java.net
> Cc: Lindenmaier, Goetz ; Baesken, Matthias
> ; Langer, Christoph
>
> Subject: RFR : 8204541 Correctly support AIX xlC 13.1 symbol visibility flags
>
> Hello.
>
> Could you review it ?
> Bug:https://bugs.openjdk.java.net/browse/JDK-8204541
> Change: http://cr.openjdk.java.net/~clanger/webrevs/8204541.0/
>
> Thanks,
> Ichiroh Takiguchi
> IBM Japan, Ltd.
>
> Original Message
> Subject: RE: RFR : 8202322: AIX: symbol visibility flags not support on
> xlc 12.1
> Date: 2018-06-07 20:43
> From: "Langer, Christoph"
> To: Ichiroh Takiguchi
> Cc: "'build-...@openjdk.java.net'" ,
> "ppc-aix-port-...@openjdk.java.net" d...@openjdk.java.net>,
> "core-libs-dev@openjdk.java.net" ,
> "Lindenmaier, Goetz" , "Baesken, Matthias"
>
>
> Hi Ichiroh,
>
> your proposal seems to make sense. I have created a bug for this:
> https://bugs.openjdk.java.net/browse/JDK-8204541
>
> Can you please generate a webrev (referencing this bug, -c option of
> webrev.ksh) and mail it over to me. Then I'll upload it and you can post
> an official RFR mail.
>
> Best regards
> Christoph
>
> > -Original Message-
> > From: Ichiroh Takiguchi [mailto:taki...@linux.vnet.ibm.com]
> > Sent: Dienstag, 5. Juni 2018 08:59
> > To: Baesken, Matthias
> > Cc: Langer, Christoph ; 'build-
> > d...@openjdk.java.net' ; ppc-aix-port-
> > d...@openjdk.java.net; core-libs-dev@openjdk.java.net; Lindenmaier,
> > Goetz
> > Subject: RE: RFR : 8202322: AIX: symbol visibility flags not support on
> > xlc 12.1
> >
> > Hello Matthias and Christoph.
> > Thank you for your explanations.
> >
> > I did not have enough knowledge about "visibility".
> >
> > I created following patches.
> >
> >
> > diff -r 02934b0d661b
> > src/java.base/share/native/libjimage/NativeImageBuffer.cpp
> > --- a/src/java.base/share/native/libjimage/NativeImageBuffer.cppWed
> > May
> > 30 14:46:28 2018 +0200
> > +++ b/src/java.base/share/native/libjimage/NativeImageBuffer.cpp
> Tue
> > Jun
> > 05 12:10:41 2018 +0900
> > @@ -39,7 +39,9 @@
> > #include "imageFile.hpp"
> > #include "inttypes.hpp"
> > #include "jimage.hpp"
> > +#if !defined(_AIX)
> > #include "osSupport.hpp"
> > +#endif
> >
> > #include "jdk_internal_jimage_NativeImageBuffer.h"
> >
> > diff -r 02934b0d661b src/java.base/unix/native/include/jni_md.h
> > --- a/src/java.base/unix/native/include/jni_md.hWed May 30 14:46:28
> > 2018 +0200
> > +++ b/src/java.base/unix/native/include/jni_md.hTue Jun 05 12:10:41
> > 2018 +0900
> > @@ -29,7 +29,8 @@
> > #ifndef __has_attribute
> > #define __has_attribute(x) 0
> > #endif
> > -#if (defined(__GNUC__) && ((__GNUC__ > 4) || (__GNUC__ == 4) &&
> > (__GNUC_MINOR__ > 2))) || __has_attribute(visibility)
> > +#if (defined(__GNUC__) && ((__GNUC__ > 4) || (__GNUC__ == 4) &&
> > (__GNUC_MINOR__ > 2))) || __has_attribute(visibility) \
> > +|| (defined(_AIX) && (defined(__xlC__) && (__xlC__ >= 0xD01)))
> > #ifdef ARM
> > #define JNIEXPORT
> > __attribute__((externally_visible,visibility("default")))
> > #define JNIIMPORT
> > __attribute__((externally_visible,visibility("default")))
> >
> >
> > If "osSupport.hpp" was included, XLC++ compiler complained.
> > I could not understand, which part was invalid...
> > I'm not sure, "osSupport.hpp" is really required on
> > NativeImageBuffer.cpp or not...
> >
> > I added __xlC__ macro to determine XLC/C++'s version# into jni_md.h.
> > [1]
> > 0xD01 means 13.1 by hexadecimal.
> >
> > I checked symbol table by "dump -Tv -X64". [2]
> > It seemed it was fine, some symbols were hidden.
> >
> > Does someone review my code?
> >
> > [1]
> >
> https://www.ibm.com/support/knowledgecenter/en/
RE: [RFR] 8204539: improve error messages in matchJavaTZ [windows]
Hi Matthias, this looks good. Reviewed from my end. Best regards Christoph From: Baesken, Matthias Sent: Freitag, 8. Juni 2018 08:51 To: Langer, Christoph ; core-libs-dev@openjdk.java.net; 'sean.cof...@oracle.com' Cc: Lindenmaier, Goetz Subject: RE: [RFR] 8204539: improve error messages in matchJavaTZ [windows] Hi, looks like I uploaded a wrong version of the patch. Updated webrev is here : http://cr.openjdk.java.net/~mbaesken/webrevs/8204539.1/ I followed the advice of Sean and removed the mapFileName from the output because it is usually a static name . Updated webrev was going through our internal build/tests . Best regards, Matthias From: Langer, Christoph Sent: Donnerstag, 7. Juni 2018 13:52 To: Baesken, Matthias mailto:matthias.baes...@sap.com>>; core-libs-dev@openjdk.java.net<mailto:core-libs-dev@openjdk.java.net> Cc: Lindenmaier, Goetz mailto:goetz.lindenma...@sap.com>> Subject: RE: [RFR] 8204539: improve error messages in matchJavaTZ [windows] Hi Matthias, in line 527, where the actual output is done, I think you would need to replace variable 'message' with 'outputMessage', otherwise I guess it won't compile?? Also, mapFileName can't be used at this place, because it has already been free'ed there. But in general the additions make sense and will make it easier to find issues in the tzmappings file. Best regards Christoph From: Baesken, Matthias Sent: Donnerstag, 7. Juni 2018 13:35 To: core-libs-dev@openjdk.java.net<mailto:core-libs-dev@openjdk.java.net> Cc: Langer, Christoph mailto:christoph.lan...@sap.com>>; Lindenmaier, Goetz mailto:goetz.lindenma...@sap.com>> Subject: [RFR] 8204539: improve error messages in matchJavaTZ [windows] Hi, could you please review this small change that improves the error messages in matchJavaTZ . A reason of the failure is added to the message , and also the offset where the error happened . Bug : https://bugs.openjdk.java.net/browse/JDK-8204539 Webrev : http://cr.openjdk.java.net/~mbaesken/webrevs/8204539/ Thanks, Matthias
RE: RFR : 8204541 Correctly support AIX xlC 13.1 symbol visibility flags
Hi Ichiroh, Ok, so as per the output, via the include of osSupport.hpp, something must happen which undeclares "visibility" or makes it ambiguous. Looking at osSupport.hpp, I can't see anything special. It would just include pthread.h and declare some c++ classes. You could try to get and analyze the preprocessed output of xlC by specifying the option -P or -E to the compile call. You will get the original xlC command line by calling 'cat support/native/java.base/libjimage/NativeImageBuffer.o.cmdline' in your build directory. I think we should really understand what's happening there and fix it correctly instead of just excluding osSupport.hpp. Best regards Christoph > -Original Message- > From: Ichiroh Takiguchi [mailto:taki...@linux.vnet.ibm.com] > Sent: Donnerstag, 7. Juni 2018 18:29 > To: Langer, Christoph > Cc: build-...@openjdk.java.net; ppc-aix-port-...@openjdk.java.net; core- > libs-...@openjdk.java.net; Lindenmaier, Goetz > ; Baesken, Matthias > > Subject: RE: RFR : 8204541 Correctly support AIX xlC 13.1 symbol visibility > flags > > Hello Christoph > > According to build log, if <#include "osSupport.hpp"> was there: > "/home/jdktest/sandbox/jdk/build/aix-ppc64-normal-server- > release/support/headers/java.base/jdk_internal_jimage_NativeImageBuffe > r.h", > line 15.27: 1540-0040 (S) The text > "Java_jdk_internal_jimage_NativeImageBuffer_getNativeMap" is > unexpected. "visibility" may be undeclared or ambiguous. > make[3]: *** > [/home/jdktest/sandbox/jdk/build/aix-ppc64-normal-server- > release/support/native/java.base/libjimage/NativeImageBuffer.o] > Error 1 > make[3]: Leaving directory `/home/jdktest/sandbox/jdk/make' > make[2]: *** [java.base-libs] Error 2 > make[2]: *** Waiting for unfinished jobs > > On 2018-06-07 22:06, Langer, Christoph wrote: > > Hi Ichiroh, > > > > what's the exact error message if you #include "osSupport.hpp"? (I > > have no xlC 13 at hand to try myself...) > > > > Best regards > > Christoph > > > >> -Original Message- > >> From: Ichiroh Takiguchi [mailto:taki...@linux.vnet.ibm.com] > >> Sent: Donnerstag, 7. Juni 2018 14:53 > >> To: build-...@openjdk.java.net; ppc-aix-port-...@openjdk.java.net; > >> core- > >> libs-...@openjdk.java.net > >> Cc: Lindenmaier, Goetz ; Baesken, > Matthias > >> ; Langer, Christoph > >> > >> Subject: RFR : 8204541 Correctly support AIX xlC 13.1 symbol > >> visibility flags > >> > >> Hello. > >> > >> Could you review it ? > >> Bug:https://bugs.openjdk.java.net/browse/JDK-8204541 > >> Change: http://cr.openjdk.java.net/~clanger/webrevs/8204541.0/ > >> > >> Thanks, > >> Ichiroh Takiguchi > >> IBM Japan, Ltd. > >> > >> Original Message > >> Subject: RE: RFR : 8202322: AIX: symbol visibility flags not support > >> on > >> xlc 12.1 > >> Date: 2018-06-07 20:43 > >> From: "Langer, Christoph" > >> To: Ichiroh Takiguchi > >> Cc: "'build-...@openjdk.java.net'" , > >> "ppc-aix-port-...@openjdk.java.net" >> d...@openjdk.java.net>, > >> "core-libs-dev@openjdk.java.net" , > >> "Lindenmaier, Goetz" , "Baesken, > Matthias" > >> > >> > >> Hi Ichiroh, > >> > >> your proposal seems to make sense. I have created a bug for this: > >> https://bugs.openjdk.java.net/browse/JDK-8204541 > >> > >> Can you please generate a webrev (referencing this bug, -c option of > >> webrev.ksh) and mail it over to me. Then I'll upload it and you can > >> post > >> an official RFR mail. > >> > >> Best regards > >> Christoph > >> > >> > -Original Message- > >> > From: Ichiroh Takiguchi [mailto:taki...@linux.vnet.ibm.com] > >> > Sent: Dienstag, 5. Juni 2018 08:59 > >> > To: Baesken, Matthias > >> > Cc: Langer, Christoph ; 'build- > >> > d...@openjdk.java.net' ; ppc-aix-port- > >> > d...@openjdk.java.net; core-libs-dev@openjdk.java.net; Lindenmaier, > >> > Goetz > >> > Subject: RE: RFR : 8202322: AIX: symbol visibility flags not support on > >> > xlc 12.1 > >> > > >> > Hello Matthias and Christoph. > >> > Thank you for your explanations. > >> > > >> > I did not have enough knowled
RE: RFR(S): 8207766: [testbug] Adapt tests for Aix.
Hi Goetz, Are you sure the quotation change in EvalArraysAsList.sh works well on other platforms? Other than that the changes look good. Reviewed. Best regards Christoph > -Original Message- > From: core-libs-dev On Behalf > Of Lindenmaier, Goetz > Sent: Mittwoch, 18. Juli 2018 09:28 > To: core-libs-dev@openjdk.java.net > Subject: [CAUTION] RFR(S): 8207766: [testbug] Adapt tests for Aix. > > Hi, > > I would like to fix these tests to run on aix: > http://cr.openjdk.java.net/~goetz/wr18/8207766-aixTestFixes/01/ > > > MulticastSocket tests > Opened bug and asked IBM to fix. Put on ProblemList. See > https://bugs.openjdk.java.net/browse/JDK-8207404 > > EvalArraysAsList.sh > Quotes need to be escaped on AIX. > > WrappedToolkitTest.sh > Check for AIX missing. > > PKCS11Test.java > getNSSLibDir can return null. If so, static initialization of badNSSVersion > fails: > java.io.FileNotFoundException: nulllibsoftokn3.so (No such file or directory) > > TestNssDbSqlite.java > Don't run this on AIX. There is no NSS on basic installations. > > launcher/SourceMode.java > AIX does not support arguments to the shebang. > > Best regards, > Goetz.
RFR: 8209937: Enhance java.io.Console - provide methods to query console width and height
Hi, please review and give feedback for this augmentation of java.io.Console. We have users that have the need to query for Width and Height of the console device in Java. In our own JVM implementation we had given them this hook as a custom extension already quite some time ago. We think, however, that it is a worthwhile augmentation of java.io.Console. Bug: https://bugs.openjdk.java.net/browse/JDK-8209937 Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8209937.0/ I'll also open a CSR for this, but I'll wait for some feedback on our proposal first. Thanks & Best regards Christoph
RE: RFR: 8209937: Enhance java.io.Console - provide methods to query console width and height
Hi Rémi, Hi Peter, thanks for your quick answers. What you've suggested, Rémi, is perfectly right. I've updated my webrev. The methods were copied from our old implementation (of a different class) where they were provided as static. I will also think of using an optional. I'm furthermore wondering if we should provide a method "dimensions()" returning an (optional) java.io.Console.Dimension object that contains both height and width... Here is a new webrev: http://cr.openjdk.java.net/~clanger/webrevs/8209937.1/ Best regards Christoph > -Original Message- > From: Peter Levart > Sent: Freitag, 24. August 2018 13:06 > To: Remi Forax ; Langer, Christoph > > Cc: core-libs-dev ; Baesken, Matthias > > Subject: Re: RFR: 8209937: Enhance java.io.Console - provide methods to > query console width and height > > > > On 08/24/2018 12:09 PM, Remi Forax wrote: > > Hi Christoph, > > getWidth() and getHeight() should be instance methods and not static > methods, providing the weight or the height if there is no console seems > weird. > > Also, they should be named width() and height() given the rest of the > methods of java.io.Console doesn't use the "get" convention. > > > > regards, > > Rémi > > They could also return OptionalInt to force user to think about > "not-available" situations. > > Regards, Peter > > > > > - Mail original - > >> De: "Christoph Langer" > >> À: "core-libs-dev" > >> Cc: "Baesken, Matthias" > >> Envoyé: Vendredi 24 Août 2018 11:33:05 > >> Objet: RFR: 8209937: Enhance java.io.Console - provide methods to query > console width and height > >> Hi, > >> > >> please review and give feedback for this augmentation of java.io.Console. > >> > >> We have users that have the need to query for Width and Height of the > console > >> device in Java. In our own JVM implementation we had given them this > hook as a > >> custom extension already quite some time ago. We think, however, that > it is a > >> worthwhile augmentation of java.io.Console. > >> > >> Bug: https://bugs.openjdk.java.net/browse/JDK-8209937 > >> Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8209937.0/ > >> > >> I'll also open a CSR for this, but I'll wait for some feedback on our > >> proposal > >> first. > >> > >> Thanks & Best regards > >> Christoph
RE: RFR: 8209937: Enhance java.io.Console - provide methods to query console width and height
Hi Alan, thanks for your feedback (and thanks of course to all the others who commented on this proposal). As I've written in my first mail, this request came up because we had implemented getWidth/getHeigth APIs for console windows in our licensed VM to be used in a certain usage scenario. Now we are in the process of moving this user to an OpenJDK based VM and thought these APIs might be candidates for contribution as general purpose API. Looking at java.io.Console, it seemed the APIs would fit in there. We also thought that there could be further additions to this class. However, we had not been aware of the history of java.io.Console and we also did not know jline project nor that it was adopted by the OpenJDK into jdk.internal.le for usage by jshell and nashorn. Thanks for pointing this out. After exploring this a bit, I'm at a point where I would go to our user and ask them to try jline. I'm quite sure they'll be able to adopt it. Regardless of that, isn't it still worth to gradually enhance java.io.Console over time? I don't know whether we'd eventually get to a point where jline could be replaced for jshell but maybe that could be a long term goal. If we can agree to evolve java.io.Console, I'll continue with my work for width/height APIs. But if it's consent to keep java.io.Console in its limited current state, I would also be willing to withdraw this request. What's the way to go? Best regards Christoph > -Original Message- > From: Alan Bateman > Sent: Montag, 27. August 2018 13:30 > To: Langer, Christoph > Cc: core-libs-dev ; Baesken, Matthias > > Subject: Re: RFR: 8209937: Enhance java.io.Console - provide methods to > query console width and height > > On 24/08/2018 14:18, Langer, Christoph wrote: > > Hi Rémi, Hi Peter, > > > > thanks for your quick answers. > > > > What you've suggested, Rémi, is perfectly right. I've updated my webrev. > The methods were copied from our old implementation (of a different class) > where they were provided as static. > > > > I will also think of using an optional. I'm furthermore wondering if we > should provide a method "dimensions()" returning an (optional) > java.io.Console.Dimension object that contains both height and width... > > > > Here is a new webrev: > http://cr.openjdk.java.net/~clanger/webrevs/8209937.1/ > I'm in two minds as to whether this API is the right thing to do. Can > you expand a bit on how you are using them? There have been several > requests for additions to the Console API over the years, mostly to > support formatting or exposing the console encoding, but I don't recall > anyone looking for the dimensions. Roger is right that it's a bit of a > slippery slope. As I recall, java.io.Console was deliberately minimized > for Java SE 6 to focus on password input and avoid introducing an > extensive API. In the mean-time, the additional of jshell has exposed > some of the shorting comings and maybe we should look at exposing just > enough to support that type of use-case (editing and line history mostly). > > As regards the proposal then I think the API docs will need fleshing out > to define what height and width as the current javadoc isn't clear if it > means bytes, characters, pixels in whatever font is used for the > console, or something else. In addition, the javadoc would need to be > clear that it's just a snaphot of the dimension as it can change at any > time. > > -Alan
RE: [RFR] 8205525 : Improve exception messages during manifest parsing of jar archives
Hi Matthias, I have reviewed your change, +1 I also reviewed the CSR. Best regards Christoph > -Original Message- > From: Baesken, Matthias > Sent: Mittwoch, 5. September 2018 10:07 > To: security-...@openjdk.java.net; Weijun Wang > ; core-libs-dev@openjdk.java.net > Cc: Langer, Christoph > Subject: Re: [RFR] 8205525 : Improve exception messages during manifest > parsing of jar archives > > Hi Max, thanks for adding yourself as a reviewer. > > I set the CSR ( https://bugs.openjdk.java.net/browse/JDK-8207768 ) to > proposed. > > Best regards, Matthias > > > > > Message: 2 > > Date: Tue, 4 Sep 2018 21:31:58 +0800 > > From: Weijun Wang > > To: "Baesken, Matthias" > > Cc: "security-...@openjdk.java.net" , > > "core-libs-dev@openjdk.java.net" > d...@openjdk.java.net> > > Subject: Re: [RFR] 8205525 : Improve exception messages during > > manifest parsing of jar archives > > Message-ID: <058bd7b5-4d3a-4b56-acb0-0dedddea2...@oracle.com> > > Content-Type: text/plain; charset=us-ascii > > > > I've added myself as a reviewer. You might want to set scope to "JDK". > > > > A CSR is approved by the CSR group after you finalize it. First you should > > propose it. If you think it's urgent or trivial, you can also fast-track it. > > > > Read https://wiki.openjdk.java.net/display/csr/Main for more details. > > > > Thanks > > Max > > > > > On Sep 4, 2018, at 7:33 PM, Baesken, Matthias > > wrote: > > > > > >> The change looks fine. We can enhance the name if we want to support > > .SF > > >> parsing later. > > >> > > >> Please revise your CSR and get it approved first. > > > > > > Hi Max, Thanks ! > > > > > > I adjusted the CSR , please approve : > > > > > > https://bugs.openjdk.java.net/browse/JDK-8207768 > > > > > > Best regards, Matthias
RE: [RFR] 8205525 : Improve exception messages during manifest parsing of jar archives
Hi Matthias,
I think it would be enough to drop the privileged section and just return
"filename" as is. (without conveting to a file object).
I also agree with Sean that the common parts of the new
src/java.base/share/classes/sun/security/util/SecurityProperties.java should be
aligned with src/java.base/share/classes/sun/net/util/SocketExceptions.java
(That is, SocketExceptions calling SecurityProperties)
Best regards
Christoph
> -Original Message-
> From: core-libs-dev On Behalf
> Of Baesken, Matthias
> Sent: Montag, 10. September 2018 09:53
> To: Wang Weijun ; Sean Mullan
>
> Cc: security-...@openjdk.java.net; core-libs-dev@openjdk.java.net
> Subject: [CAUTION] RE: [RFR] 8205525 : Improve exception messages during
> manifest parsing of jar archives
>
> Hello are you fine with changing from file.getAbsolutePath()to
> file.getName() ?
>
>
> http://cr.openjdk.java.net/~mbaesken/webrevs/8205525.8/src/java.base/s
> hare/classes/java/util/jar/Manifest.java.frames.html
>
>
> 206 static String getErrorPosition(String filename, final int lineNumber)
> {
> 207 if (filename == null || !jarPathInExceptionText) {
> 208 return "line " + lineNumber;
> 209 }
> 210
> 211 final File file = new File(filename);
> 212 return AccessController.doPrivileged(new
> PrivilegedAction()
> {
> 213 public String run() {
> 214 return "manifest of " + file.getName() + ":" +
> lineNumber;
> 215 }
>
>
> Best regards, Matthias
>
>
> > -Original Message-
> > From: Wang Weijun
> > Sent: Samstag, 8. September 2018 17:43
> > To: Sean Mullan
> > Cc: Baesken, Matthias ; Alan Bateman
> > ; Chris Hegarty ;
> > security-...@openjdk.java.net; core-libs-dev@openjdk.java.net
> > Subject: Re: [RFR] 8205525 : Improve exception messages during manifest
> > parsing of jar archives
> >
> > Thinking about this again. Looks like the absolute path is not necessary.
> Even
> > if there are multiple files using the same name, they will be in different
> > directories, no matter absolute or relative. Suppose the jarPath info is
> > used
> > for debugging purpose mostly like the developer can find out what the
> > current working directory is and can find the files. *Matthias*: Is the
> absolute
> > path really necessary? Are you working on actual case?
> >
> > As for the possible global effect of a security property, maybe we can
> > emphasis that this is both a security property and system property, and if
> it’s
> > just for one time use, it’s better to use a system property.
> >
> > BTW, does the existing value “hostInfo” of the property have a similar
> > problem?
> >
> > Thanks
> > Max
> >
> > >> 在 2018年9月8日，21:42，Sean Mullan 写
> > 道：
> > >>
> > >> On 9/7/18 7:58 PM, Weijun Wang wrote:
> > >> In my understanding, the author deliberately wants to show the
> absolute
> > paths when there are multiple jar files with the same name (Ex: a jar hell).
> > >
> > > If you are very familiar with a particular application and understand the
> risks
> > associated with running it, then I agree that is ok. But security properties
> > apply to all applications using that JDK, and so I don't always think it is
> > practical for an admin to understand every type of application that may be
> > using that JDK and whether or not enabling this property presents a risk.
> > >
> > >> Maybe we can add some more detail in the java.security so an admin
> > knows what exact impact it has.
> > >
> > > It would be a slippery slope in my opinion. You would have to say
> > something like: "enabling this property may allow untrusted code running
> > under a SecurityManager to access sensitive information such as the
> > user.home system property even if it has not been granted permission to
> do
> > so."
> > >
> > > I think we should consider not allowing this property to take effect if a
> > SecurityManager is running.
> > >
> > > --Sean
> > >
> > >> --Max
> > >>> On Sep 8, 2018, at 3:41 AM, Sean Mullan
> > wrote:
> > >>>
> > >>> On 8/29/18 10:01 AM, Baesken, Matthias wrote:
> > Hi Max, thanks for your input .
> > I created another webrev , this contains now the suggested
> > SecurityProperties class :
> > http://cr.openjdk.java.net/~mbaesken/webrevs/8205525.6/
> > >>>
> > >>> java/util/jar/Attributes.java
> > >>>
> > >>> 469 return AccessController.doPrivileged(new
> > PrivilegedAction() {
> > >>> 470 public String run() {
> > >>> 471 return file.getAbsolutePath() + ":" + lineNumber;
> > >>> 472 }
> > >>> 473 });
> > >>>
> > >>> I have a serious concern with the code above.
> > >>>
> > >>> With this change, untrusted code running under a SecurityManager
> > could potentially create a JarFile on a non-absolute path ex: "foo.jar", and
> > (somehow) cause an IOException to be thrown which would then reveal
> the
> > absolute path of where the jar was located, and thus co
RE: [RFR] 8205525 : Improve exception messages during manifest parsing of jar archives
Hi Sean, Max,
do you think we need property jdk.includeInExceptions=jar at all, if
we don't resolve the absolute path?
@Matthias:
small remark to the code:
src/java.base/share/classes/sun/security/util/SecurityProperties.java
36 public static String privilegeGetOverridable(String propName) {
Should that method really be public? At the moment it doesn't seem to be used
outside of SecurityProperties.
Best regards
Christoph
> -Original Message-
> From: Weijun Wang
> Sent: Montag, 10. September 2018 16:43
> To: Sean Mullan
> Cc: Baesken, Matthias ; Langer, Christoph
> ; security-...@openjdk.java.net; core-libs-
> d...@openjdk.java.net
> Subject: Re: [RFR] 8205525 : Improve exception messages during manifest
> parsing of jar archives
>
>
>
> > On Sep 10, 2018, at 10:35 PM, Sean Mullan
> wrote:
> >
> >> After the changes I wonder - should the jarPath category be renamed
> to jarFile (or something else) ?
> >
> > Yes, renaming it to "jarFile" makes more sense. You will need to update the
> CSR with this change too.
>
> Well, maybe either is OK.
>
> I am still thinking that one day we could make this more precise, for example:
>
>/path/to/lib.jar!META-INF/MANIFEST.MF:23
>/path/to/lib.jar!META-INF/SIGNER.SF:34
>
> The name should (at least vaguely) cover these info.
>
> --Max
RE: [RFR] 8205525 : Improve exception messages during manifest parsing of jar archives
Hi,
> > do you think we need property jdk.includeInExceptions=jar at
> all, if we don't resolve the absolute path?
>
> I think so. File path is still sensitive.
>
> In fact, I tend to believe people usually use absolute paths for JAR files (or
> maybe made absolute by using a file:// URL somewhere inside JDK). Do you
> really see relative jar paths while testing this code change?
I agree.
> > small remark to the code:
> > src/java.base/share/classes/sun/security/util/SecurityProperties.java
> > 36 public static String privilegeGetOverridable(String propName) {
> >
> > Should that method really be public? At the moment it doesn't seem to be
> used outside of SecurityProperties.
>
> I like it to be public. There are quite some other such system/security
> properties (Ex: jdk.serialFilter) that can make use of this method.
Ok, maybe it should be named "priviledgedGetOverridable" then.
@Matthias:
Further small cleanups, as you touch the files:
src/java.base/share/classes/java/util/jar/Manifest.java: line 35: you can
remove "import java.util.Iterator;"
src/java.base/share/classes/sun/net/util/SocketExceptions.java:
line 41: indentation is ridiculously long, I think it should be 8 chars
src/java.base/share/classes/sun/security/util/SecurityProperties.java:
Indentation of lines 38-45 is too deep, should be 12.
The 2 methods could use a brief Javadoc.
Best regards
Christoph
RE: [RFR] 8205525 : Improve exception messages during manifest parsing of jar archives
Hi,
first of all, I suggest to use "jarDetails" instead of "jarPath" as category
name. Because with this contribution we add the notion of jar file plus line of
manifest to Exceptions occurring when parsing jar manifests. And if there were
further Exception details to be added in the area of jar files, they could go
under a category "jarDetails" as well. Would you agree? Then, in Manifest.java,
the field "jarPathInExceptionText" could be renamed to "detailedExceptions".
As for the code, I have the following remarks:
src/java.base/share/classes/java/util/jar/Manifest.java:
You could further clean up the ordering of includes by sorting them
alphabetically and add a blank line between lines 34/35.
Line 52: I suggest an indentation of 8 chars
Please use jarFilename as final. You'll have to initialize jarFilename in each
constructor then or initialize it to null in the default constructor and call
this constructor from all the other ctors except for the one taking the
jarFilename as param.
src/java.base/share/classes/sun/net/util/SocketExceptions.java
please add an empty line between 32 and 33
Line 39: I suggest an indentation of 8 chars
src/java.base/share/classes/sun/security/util/SecurityProperties.java
Line 35: change comment opener to /** (from /*), then it's a real Javadoc
Furthermore I suggest to change the wording to "Returns the value of the
security property propName, which can be overridden by a system property of the
same name"
Best regards
Christoph
> -Original Message-
> From: Baesken, Matthias
> Sent: Dienstag, 11. September 2018 13:29
> To: Weijun Wang
> Cc: Langer, Christoph ; Sean Mullan
> ; security-...@openjdk.java.net; core-libs-
> d...@openjdk.java.net
> Subject: RE: [RFR] 8205525 : Improve exception messages during manifest
> parsing of jar archives
>
> > I don't have a strong opinion on making Manifest.jarFilename final
>
> Hi, just making it final leads to compile errors anyway.
>
> Best regards, Matthias
>
>
> > -----Original Message-
> > From: Weijun Wang
> > Sent: Dienstag, 11. September 2018 13:04
> > To: Baesken, Matthias
> > Cc: Langer, Christoph ; Sean Mullan
> > ; security-...@openjdk.java.net; core-libs-
> > d...@openjdk.java.net
> > Subject: Re: [RFR] 8205525 : Improve exception messages during manifest
> > parsing of jar archives
> >
> > Attributes.java:
> >
> > - Line 377: Too long, add a break.
> >
> > Otherwise fine.
> >
> > I don't have a strong opinion on making Manifest.jarFilename final or a
> > different property name.
> >
> > Thanks
> > Max
> >
> > > On Sep 11, 2018, at 5:07 PM, Baesken, Matthias
> > wrote:
> > >
> > > Hello, please check the new webrev :
> > >
> > > http://cr.openjdk.java.net/~mbaesken/webrevs/8205525.10/
> > >
> > > I kept the jarPath category name .
> > >
> > > Best regards, Matthias
> > >
> > >
> > >> -Original Message-
> > >> From: Langer, Christoph
> > >> Sent: Montag, 10. September 2018 21:30
> > >> To: Weijun Wang ; Baesken, Matthias
> > >>
> > >> Cc: Sean Mullan ; security-
> > >> d...@openjdk.java.net; core-libs-dev@openjdk.java.net
> > >> Subject: RE: [RFR] 8205525 : Improve exception messages during
> manifest
> > >> parsing of jar archives
> > >>
> > >> Hi,
> > >>
> > >>>> do you think we need property
> jdk.includeInExceptions=jar
> > at
> > >>> all, if we don't resolve the absolute path?
> > >>>
> > >>> I think so. File path is still sensitive.
> > >>>
> > >>> In fact, I tend to believe people usually use absolute paths for JAR
> > >>> files
> > (or
> > >>> maybe made absolute by using a file:// URL somewhere inside JDK).
> Do
> > >> you
> > >>> really see relative jar paths while testing this code change?
> > >>
> > >> I agree.
> > >>
> > >>>> small remark to the code:
> > >>>> src/java.base/share/classes/sun/security/util/SecurityProperties.java
> > >>>> 36 public static String privilegeGetOverridable(String propName) {
> > >>>>
> > >>>> Should that method really be public? At the moment it doesn't seem
> to
> > >> be
> > >>> used outside of SecurityProperties.
> > >>>
> > >>> I like it to be public. There are quite some other such system/security
> > >>> properties (Ex: jdk.serialFilter) that can make use of this method.
> > >>
> > >> Ok, maybe it should be named "priviledgedGetOverridable" then.
> > >>
> > >> @Matthias:
> > >> Further small cleanups, as you touch the files:
> > >> src/java.base/share/classes/java/util/jar/Manifest.java: line 35: you can
> > >> remove "import java.util.Iterator;"
> > >>
> > >> src/java.base/share/classes/sun/net/util/SocketExceptions.java:
> > >> line 41: indentation is ridiculously long, I think it should be 8 chars
> > >>
> > >> src/java.base/share/classes/sun/security/util/SecurityProperties.java:
> > >> Indentation of lines 38-45 is too deep, should be 12.
> > >> The 2 methods could use a brief Javadoc.
> > >>
> > >> Best regards
> > >> Christoph
> > >
>
RE: [RFR] 8205525 : Improve exception messages during manifest parsing of jar archives
Looks good to me. > -Original Message- > From: Baesken, Matthias > Sent: Mittwoch, 12. September 2018 11:27 > To: Sean Mullan ; Langer, Christoph > ; Weijun Wang > Cc: security-...@openjdk.java.net; core-libs-dev@openjdk.java.net > Subject: RE: [RFR] 8205525 : Improve exception messages during manifest > parsing of jar archives > > Hello I changed it to jar , also added some minor adjustments suggested by > Christoph . > > http://cr.openjdk.java.net/~mbaesken/webrevs/8205525.12/ > > > Regards, Matthias > > > > -Original Message- > > From: Sean Mullan > > Sent: Dienstag, 11. September 2018 20:44 > > To: Langer, Christoph ; Baesken, Matthias > > ; Weijun Wang > > Cc: security-...@openjdk.java.net; core-libs-dev@openjdk.java.net > > Subject: Re: [RFR] 8205525 : Improve exception messages during manifest > > parsing of jar archives > > > > On 9/11/18 8:14 AM, Langer, Christoph wrote: > > > Hi, > > > > > > first of all, I suggest to use "jarDetails" instead of "jarPath" as > > > category > > name. Because with this contribution we add the notion of jar file plus line > of > > manifest to Exceptions occurring when parsing jar manifests. And if there > > were further Exception details to be added in the area of jar files, they > could > > go under a category "jarDetails" as well. Would you agree? Then, in > > Manifest.java, the field "jarPathInExceptionText" could be renamed to > > "detailedExceptions". > > > > Or just "jar" would be my preference. I don't like "jarPath" as that > > sounds too much like a file pathname to me, which we have now removed. > > > > --Sean
RE: RFR(s): 8210549: Runtime.exec: in closeDescriptors(), use FD_CLOEXEC instead of close()
Hi Thomas,
I like the full patch 😊 +1
Best regards
Christoph
> -Original Message-
> From: core-libs-dev On Behalf
> Of Thomas Stüfe
> Sent: Dienstag, 11. September 2018 19:27
> To: Java Core Libs
> Subject: RFR(s): 8210549: Runtime.exec: in closeDescriptors(), use
> FD_CLOEXEC instead of close()
>
> Hi all,
>
> May I please have reviews for this small fix:
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8210549
>
> patch (full): http://cr.openjdk.java.net/~stuefe/webrevs/8210549-Use-
> FD_CLOEXEC-instead-of-close/webrev.00/webrev/
> patch (minimal alternative):
> http://cr.openjdk.java.net/~stuefe/webrevs/8210549-Use-FD_CLOEXEC-
> instead-of-close/webrev.minimal/webrev/
>
> See also prior discussion:
> http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-
> September/055173.html
>
> Submit tests ran with full patch successfully through.
>
> ---
>
> Background:
>
> After fork()/vfork() and before exec(), the child process needs to
> close all inherited file descriptors apart from the stdin/out/err pipe
> ends. We do this by iterating thru all file descriptors in
> /proc//fd or whatever the equivalent is on that platform. This is
> done using opendir(), readdir().
>
> We then close all these file descriptors using close().
>
> The problem with that technique is that we may accidentally close the
> file descriptor opendir() is using internally to read the directory
> content of /proc//fd, thereby sawing off the branch we are
> sitting on. The coding does some magic to attempt to prevent this:
>
>
> 86/* We're trying to close all file descriptors, but opendir() might
> 87 * itself be implemented using a file descriptor, and we certainly
> 88 * don't want to close that while it's in use. We assume that if
> 89 * opendir() is implemented using a file descriptor, then it uses
> 90 * the lowest numbered file descriptor, just like open(). So we
> 91 * close a couple explicitly. */
> 92
> 93close(from_fd); /* for possible use by opendir() */
> 94close(from_fd + 1); /* another one for good luck */
>
> ...
> 108while ((dirp = readdir64(dp)) != NULL) {
> 109int fd;
> 110if (isAsciiDigit(dirp->d_name[0]) &&
> 111(fd = strtol(dirp->d_name, NULL, 10)) >= from_fd +
> 2)Improve "close all filedescriptors" coding.
> 112close(fd);
> 113}
>
>
>
> This workaround can be removed if, instead of outright closing the
> file descriptor in the loop, we were to set the file descriptor to
> FD_CLOEXEC. Closing all descriptors would be defered to the actual
> exec() call some milliseconds later.
>
>
>
> The patch changes the close() call to fcntl(FD_CLOEXEC). This removes
> the need for the workaround as described.
>
> In addition to that, the full version of the patch also adds error
> handling to the readdir() loop.
>
> But note that this exposes us to a behavioral change should we run
> into a readdir() error:
>
> - currently, we would leave the remaining file descriptors quietly
> unclosed. This usually would have no effect, but in rare cases may
> lead to difficult-to-analyze errors when child processes accidentally
> disturb parent process IO.
>
> - With this patch, we will fail if readdir fails. However, we do not
> just yet fail the Runtime.exec() call, but enter a fallback code
> section here:
>
> 364 /* close everything */
> 365 if (closeDescriptors() == 0) { /* failed, close the old way */
> 366 int max_fd = (int)sysconf(_SC_OPEN_MAX);
> 367 int fd;
> 368 for (fd = FAIL_FILENO + 1; fd < max_fd; fd++)
> 369 if (close(fd) == -1 && errno != EBADF)
> 370 goto WhyCantJohnnyExec;
> 371 }
>
> I am not convinced this is a good fallback. _SC_OPEN_MAX depends on
> ulimit -Hn and may be high. On my Ubuntu 16.4 box, default value after
> installation is 1048576. When we hit this fallback code, it takes
> almost a full second to spawn a single child process. In fork
> intensive scenarios this can get pretty bad.
>
> What do you think?
>
> Thanks & Best Regards,
>
> Thomas
Re-iterate JDK-6194856: Zip Files lose ALL ownership and permissions of the files
Hi all, I'm currently revisiting a long standing shortcoming of the java.util.zip (and java.util.jar) implementation. The lack is that in the current implementation, Unix mode bits (e.g. rwx) are not stored/read from zip or jar files and the jar tool has no capabilities to store/preserve the mode information. There have been several bugs opened but they were closed with "Won't Fix". One of them is JDK-6194856, mentioned in this mail's subject [1]. Unfortunately there is no detailed reasoning given why that can't be done. Maybe these are compatibility or security issues? Maybe someone can enlighten me about previous discussions concerning this matter... I personally can imagine contributing the following: First step: Enrich java.util.zip.ZipEntry with some method(s) like getUnixPermissions() and setUnixPermissions(). For writing zip files it would mean depending on the availability of Unix permission information in a ZipEntry object, the entry version would be set to "3 - Unix" according to section "D. Explanation of fields", "version made by (2 bytes)" of the zip specification that is referenced in the current Javadoc for the java.util.zip package [2]. Reading Zip files would take into account the file attribute mapping of the version field and in case of "3 - Unix", it would read the permissions settings from the external file attributes section of the entry header. This seems to be the de-facto standard of the GNU zip/unzip implementation. With that addition the the java.util.zip API, developers using it would get the ability to cope with Unix permissions. As a second step, I can imagine spending a flag for the jar utility to have it handle Unix permissions when packing/extracting. Isn't that something that should be done? Or, if not, why? Thanks and best regards in advance Christoph [1] https://bugs.openjdk.java.net/browse/JDK-6194856 [2] http://www.info-zip.org/doc/appnote-19970311-iz.zip
RE: RFR JDK-8034802: (zipfs) newFileSystem throws UOE when the zip file is located in a custom file system
Hi Sherman,
as I'm currently looking into zip stuff as well, it's a good exercise to review
your changeset. Overall it looks good. I found a few nits mostly in the area of
spelling and whitespace 😊
src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipFileSystem.java:
88 ZipFileSystem(ZipFileSystemProvider provider,
I think, the constructor should initialize these items as last statements:
92 this.provider = provider;
93 this.zfpath = zfpath;
As per section 7-3 of the security guide which should probably apply here:
https://www.oracle.com/technetwork/java/seccodeguide-139067.html#7
132 // returns ture if there is a name=true/"true" setting in env
-> spelling: // returns true if there is a name=true/"true" setting in env
282 // and sync dose not close the ch
-> spelling: // and sync did not close the ch
587 // (1) writing the contents of a new entry, if the entry doesn't exits,
or
-> spelling: // (1) writing the contents of a new entry, if the entry doesn't
exist, or
1471 }
1472 else { // untouced CEN or COPY
-> put brace and else on the same line to match style above and spelling of
"untouched": } else { // untouched CEN or COPY
remove these 2 lines that have been commented out:
1881 // Entry(byte[] name, boolean isdir) {
1889 //this.method = METHOD_DEFLTED;
2402 fm.format("name: %s%n", new String(name));
2403 fm.format("creationTime: %tc%n",
creationTime().toMillis());
-> I would align name with the start of creationTime (and the other lines below)
test/jdk/jdk/nio/zipfs/ZipFSTester.java:
134 try ( OutputStream os = Files.newOutputStream(src)) {
-> the blank between ( and OutputStream could be removed
436 private static void checkRead(Path path, byte[] expected) throws
IOException
-> should take brace from line 437 on the same line, line length is not too
long yet
438 //streams
-> insert a blank between // and streams
447try (SeekableByteChannel sbc = Files.newByteChannel(path);
insert one more space before try to have correct indentation
src/jdk.zipfs/share/classes/jdk/nio/zipfs/ByteArrayChannel.java
43 * The currently position of this channel.
-> The current position of this channel.
97 throw new IllegalArgumentException("negative position " +
pos);
-> shouldn't it rather be "illegal position " as it must not necessarily be a
negative position?
Assuming that the updated test case runs correctly, you can consider the
changes reviewed from my end.
I'll post a change for adding executable bit support soon, based on your
changes.
Best regards
Christoph
> -Original Message-
> From: nio-dev On Behalf Of Xueming
> Shen
> Sent: Sonntag, 16. September 2018 09:06
> To: core-libs-dev@openjdk.java.net; nio-dev
> Subject: RFR JDK-8034802: (zipfs) newFileSystem throws UOE when the zip
> file is located in a custom file system
>
> Hi
>
> Please help review the change for JDK-8034802.
>
> issue: https://bugs.openjdk.java.net/browse/JDK-8034802
> webrev: http://cr.openjdk.java.net/~sherman/8034802/webrev
>
> One of the reasons that the implementation works this way is that I
> didn't have a
> complete SeekableByteChannel support (mainly the position(...)) from
> newByteChannel()
> back then (so you really can't get a ZipFileSystem from a jar/zip file
> inside a zfs :-))
>
> So this changeset also includes a ByteArrayChannel which implements SBC
> and has
> better position()/position(int) support, and now we can have a zipfs
> from a zip file
> inside a zipfs.
>
> Thanks,
> Sherman
>
RE: Issues running JAXP jtreg tests ("java.lang.RuntimePermission" "accessDeclaredMembers")
Hi,
we are running jtreg with something like -jdk:/images/jdk. So
would that be the exploded version or not?
FWIW: I think all test that fail don't have void test methods but the test
methods expect input parameters and hence a tag @Test(dataProvider = "...")
exists. Can it be that we are using a testng framework that is "too new" and
maybe contains something which makes it not work in the jaxp scenario?
Best,
Christoph
> -Original Message-
> From: Daniel Fuchs [mailto:daniel.fu...@oracle.com]
> Sent: Dienstag, 22. November 2016 13:42
> To: Volker Simonis ; Langer, Christoph
> ; Langer, Christoph
> Cc: Chris Hegarty ; code-tools-
> d...@openjdk.java.net; core-libs-dev@openjdk.java.net; jtreg-
> u...@openjdk.java.net
> Subject: Re: Issues running JAXP jtreg tests ("java.lang.RuntimePermission"
> "accessDeclaredMembers")
>
> Hi guys,
>
> Are you running the tests with the exploded jdk or with the image?
>
> I'm seeing failures with the exploded jdk.
>
> That could explain the difference with permission checks.
>
> best regards,
>
> -- daniel
>
>
> On 22/11/16 12:32, Daniel Fuchs wrote:
> > Hi Volker,
> >
> > On 22/11/16 12:25, Chris Hegarty wrote:
> >> Volker,
> >>
> >> Just to add, jtreg has support in its tags to start the test VM with a
> >> security manager and a specified policy. In the case of the test failure
> >> you are seeing, the built-in jtreg support is not being used. Instead,
> >> the test is executing with a test-specific system property that is being
> >> used to trigger the programatic setting of a security manager with a
> >> generated policy. I think this explains why you are not seeing any
> >> policy file in the JTwork directory.
> >>
> >> What is odd is that the stack trace you are seeing appears to be
> >> before the test’s main entry point, so I would not expect to the
> >> security manager to be active at this point ( since no test code
> >> has run ). My previous comment regarding 7901792 is not relevant
> >> since it relates to tests executing with a security manager set
> >> through jtreg tags.
> >
> > I agree with Chris - I believe CODETOOLS-7901792 was a red herring.
> > I'm going to try the jtreg you pointed at and see if there's any
> > difference (I'm using jtreg 4.2 fcs b03).
> >
> >> Is there anything in the environment that could trigger the VM
> >> to start up with a security manager enabled?
> >
> > This is a good question.
> >
> > best regards,
> >
> > -- daniel
> >
> >>
> >> -Chris.
> >>
> >>> On 22 Nov 2016, at 12:13, Volker Simonis
> >>> wrote:
> >>>
> >>> Hi Daniel,
> >>>
> >>> thanks for your support - this problem really drives us crazy!
> >>>
> >>> What version of jtreg are you using?
> >>> If you are using a central one which was configured and build by
> >>> Oracle, could you please also try with the one from
> >>> https://adopt-
> openjdk.ci.cloudbees.com/job/jtreg/lastSuccessfulBuild/artifact/
> >>>
> >>> ?
> >>>
> >>> Where can we find the generated policy files? It seems they get
> >>> deleted during test post-run cleanup phase (at least I could not find
> >>> any of them under JTwork). Is there a way to get a more detailed trace
> >>> on how JTreg executes testng and to leave all the generated files in
> >>> place after the test?
> >>>
> >>> I'm currently running the following JAXP test and get the same error
> >>> as described by Christoph:
> >>>
> >>> /tmp/jtreg/bin/jtreg -verbose:summary -jdk
> >>> /output-jdk9-hs-dbg/images/jdk/
> >>> /OpenJDK/jdk9-
> hs/jaxp/test/javax/xml/jaxp/functional/catalog/CatalogReferCircularityTest.jav
> a
> >>>
> >>>
> >>> What I don't really understand is how this is supposed to work at all,
> >>> because every JAXP test which runs with "-DrunSecMngr=true" will
> >>> execute the following code from JAXPPolicyManager:
> >>>
> >>>/*
> >>> * Install a SecurityManager along with a default Policy to allow
> >>> testNG to
> >>> * run when there is a security manager.
> >>> */
> >>> private JAXPPolicyManager() {
> >>>// Backing up policy and security manager for restore
>
RE: Issues running JAXP jtreg tests ("java.lang.RuntimePermission" "accessDeclaredMembers")
Hi Daniel,
yes - that looks like what we observe, see Volker's mail. It seems to work with
testing 6.9.5 but not with 6.9.10 or 6.9.11.
So for my jaxp testing I can use 6.9.5, I'm so happy :)
But, as for fixing with the new testng, will you look into this? Shall I open a
bug?
Thanks
Christoph
> -Original Message-
> From: Daniel Fuchs [mailto:daniel.fu...@oracle.com]
> Sent: Dienstag, 22. November 2016 15:31
> To: Langer, Christoph ; Volker Simonis
>
> Cc: code-tools-...@openjdk.java.net; core-libs-dev@openjdk.java.net; jtreg-
> u...@openjdk.java.net
> Subject: Re: Issues running JAXP jtreg tests ("java.lang.RuntimePermission"
> "accessDeclaredMembers")
>
> Hi,
>
> The issue seems to be caused by changes in testng - but maybe that
> needs to be fixed in the JAXP test base.
>
> I could reproduce the issue by running the jaxp tests, after
> unsetting JT_HOME and running with
> https://adopt-
> openjdk.ci.cloudbees.com/job/jtreg/lastSuccessfulBuild/artifact/jtreg-4.2-
> b03.tar.gz
> which has testng 6.9.11.
>
> When I dowloaded
> http://mvnrepository.com/artifact/org.testng/testng/6.9.5
> and installed it in the jtreg install dir, the issue disappeared.
>
> best regards,
>
> -- daniel
>
RE: Issues running JAXP jtreg tests ("java.lang.RuntimePermission" "accessDeclaredMembers")
In that case, if we can't change testng, maybe the jaxp SecurityManager can
allow testng to access the declared members without granting this to the testee
code?
> -Original Message-
> From: Daniel Fuchs [mailto:daniel.fu...@oracle.com]
> Sent: Dienstag, 22. November 2016 15:47
> To: Langer, Christoph
> Cc: code-tools-...@openjdk.java.net; core-libs-dev@openjdk.java.net; jtreg-
> u...@openjdk.java.net; Volker Simonis
> Subject: Re: Issues running JAXP jtreg tests ("java.lang.RuntimePermission"
> "accessDeclaredMembers")
>
> On 22/11/16 14:43, Langer, Christoph wrote:
> > But, as for fixing with the new testng, will you look into this? Shall I
> > open a
> bug?
>
> I am unsure on how to fix this actually.
>
> The new testng must attempt to load the data provider
> after having invoked the listener, while the old testng
> probably did the opposite.
>
> Let me try some experiments...
>
> best regards,
>
> -- daniel
RE: Issues running JAXP jtreg tests ("java.lang.RuntimePermission" "accessDeclaredMembers")
Thanks a lot Volker and Daniel for the big support to analyze and fix this.
It seems to me that the proposed fix
(http://cr.openjdk.java.net/~dfuchs/webrev_8170192/webrev.00/ ) looks like the
best that can be done at the moment. I agree that it would be nicer if jtreg
would leave the jtreg lib path as java property to be able to elevate all of
its contents. But the current proposal with a set of TEST_JARS of jtreg,
javatest and testng is probably sufficient for jaxp testing.
The best thing to find out about other issues with the new version of testng
would certainly be if Oracle's internal version of jtreg be updated to use the
latest testng :-)
Best regards
Christoph
> -Original Message-
> From: Volker Simonis [mailto:volker.simo...@gmail.com]
> Sent: Dienstag, 22. November 2016 20:25
> To: Daniel Fuchs
> Cc: Langer, Christoph ; code-tools-
> d...@openjdk.java.net; core-libs-dev@openjdk.java.net; jtreg-
> u...@openjdk.java.net
> Subject: Re: Issues running JAXP jtreg tests ("java.lang.RuntimePermission"
> "accessDeclaredMembers")
>
> Hi Daniel,
>
> thanks for your patch!
>
> I've meanwhile tried to better understand the root cause of the problem.
>
> I don't think that the invocation order of the data provider the
> listener have changed. If you look at the the two version 6.9.5 and
> 6.9.13 of testng, the org.testng.TestRunner.run() methods look exactly
> the same in both 6.9.5 [1] and 6.9.13 [2]:
>
> public void run() {
> beforeRun();
>
> try {
> XmlTest test= getTest();
> if(test.isJUnit()) {
> privateRunJUnit(test);
> }
> else {
> privateRun(test);
> }
> }
> finally {
> afterRun();
> }
>
> I think the problem is in
> org.testng.internal.ClassHelper.getAvailableMethods() where we testng
> only collected the methods until (i.e. excluding) java.lang.Object in
> 6.9.5 [3] but including java.lang.Object in 6.9.13 [4]:
>
> 6.9.5
> =
> public static Set getAvailableMethods(Class clazz) {
> Set methods = Sets.newHashSet();
> methods.addAll(Arrays.asList(clazz.getDeclaredMethods()));
>
> Class parent = clazz.getSuperclass();
> while (Object.class != parent) {
> methods.addAll(extractMethods(clazz, parent, methods));
> parent = parent.getSuperclass();
> }
>
> 6.9.13
> =
> public static Set getAvailableMethods(Class clazz) {
> Set methods = Sets.newHashSet();
> methods.addAll(Arrays.asList(clazz.getDeclaredMethods()));
>
> Class parent = clazz.getSuperclass();
> while (null != parent) {
> methods.addAll(extractMethods(clazz, parent, methods));
> parent = parent.getSuperclass();
> }
>
> But java.lang.Object has a different class loader (null) compared to
> the test class (which is loaded by the application class loader),
> which leads to the AccessControlException with 6.9.13.
>
> As far as I can see, this was changed in testng 6.9.10 [5] to fix
> https://github.com/cbeust/testng/issues/876
>
> This behavior may potentially also affect other tests which are
> running with a security manger so I'm not sure you fix will help for
> all of them. And I also wonder why this hasn't been detected by other
> who run testng with a security manager (but maybe nobody is doing that
> :)
>
> Regards,
> Volker
>
> [1] https://github.com/cbeust/testng/blob/testng-
> 6.9.5/src/main/java/org/testng/TestRunner.java
> [2]
> https://github.com/cbeust/testng/blob/6.9.13/src/main/java/org/testng/TestRu
> nner.java
> [3] https://github.com/cbeust/testng/blob/testng-
> 6.9.5/src/main/java/org/testng/internal/ClassHelper.java
> [4]
> https://github.com/cbeust/testng/blob/6.9.13/src/main/java/org/testng/interna
> l/ClassHelper.java
> [5]
> https://github.com/cbeust/testng/pull/886/commits/fefedec34706e40ff2bf780b
> ff7716fca29daaab
>
>
> On Tue, Nov 22, 2016 at 5:24 PM, Daniel Fuchs
> wrote:
> > Hi Christoph,
> >
> > I have logged https://bugs.openjdk.java.net/browse/JDK-8170192
> >
> > best regards,
> >
> > -- daniel
> >
> >
> > On 22/11/16 14:47, Daniel Fuchs wrote:
> >>
> >> On 22/11/16 14:43, Langer, Christoph wrote:
> >>>
> >>> But, as for fixing with the new testng, will you look into this? Shall
> >>> I open a bug?
> >
> >
RE: RFR: 8169772: [JAXP] XALAN: Transformation of DOM with null valued text node causes NPE
Hi Joe,
as my jtreg issues are solved now, I'm finalizing the patches.
For this one I've updated the test method:
http://cr.openjdk.java.net/~clanger/webrevs/8169772.2/
Could you please quickly check if my new test method "testBug8169772()" looks
as you want it? I've added some more detailed comments for the method. The
point is that without the fix you'd encounter an NPE. Or should there be some
special assertion around it?
If it's ok for you, I'd push tomorrow.
Thanks & Best regards
Christoph
From: Joe Wang [mailto:huizhe.w...@oracle.com]
Sent: Samstag, 19. November 2016 00:23
To: Langer, Christoph
Cc: core-libs-dev@openjdk.java.net
Subject: Re: RFR: 8169772: [JAXP] XALAN: Transformation of DOM with null valued
text node causes NPE
On 11/18/16, 12:17 PM, Langer, Christoph wrote:
Hi Joe,
Yep, I'll conduct the testing before pushing.
With the "Expected Result", you mean some comment for the test method, right?
Yes, some comment for the test method would do. But I see that you've added
assertions? That would be even better. Comments help in this case since without
the patch, the process would have thrown an NPE.
Once a bug is fixed, SQE will take the new test and verify the fix. Knowing the
success/fail (before and after fix) conditions is helpful for them.
Thanks,
Joe
Thanks
Christoph
From: Joe Wang [mailto:huizhe.w...@oracle.com]
Sent: Freitag, 18. November 2016 20:00
To: Langer, Christoph
<mailto:christoph.lan...@sap.com>
Cc: core-libs-dev@openjdk.java.net<mailto:core-libs-dev@openjdk.java.net>
Subject: Re: RFR: 8169772: [JAXP] XALAN: Transformation of DOM with null valued
text node causes NPE
Looks good, Christoph.
I assume you'll do an all-test run (all in jaxp/test) before pushing. No need
for further review, but it'd be nice to add an "Expected result" for the new
test testBug8169772, with/without the fix (e.g. NPE).
Best regards,
Joe
On 11/18/16, 4:38 AM, Langer, Christoph wrote:
Hi Joe,
thanks for the feedback.
I've created a new version of the webrev working in your suggestions, adding
some further formatting cleanups in the files and I also moved a small
refactoring in TransformerTest.java to this changeset.
http://cr.openjdk.java.net/~clanger/webrevs/8169772.1/<http://cr.openjdk.java.net/%7Eclanger/webrevs/8169772.1/>
>From my end this one is ready for pushing - waiting for your final go.
Best regards
Christoph
From: Joe Wang [mailto:huizhe.w...@oracle.com]
Sent: Freitag, 18. November 2016 07:36
To: Langer, Christoph
<mailto:christoph.lan...@sap.com>
Cc: core-libs-dev@openjdk.java.net<mailto:core-libs-dev@openjdk.java.net>
Subject: Re: RFR: 8169772: [JAXP] XALAN: Transformation of DOM with null valued
text node causes NPE
Looks fine.
License header: in general, don't change / add Year if there's no material
change, removing the legacy $Id field in EmptySerializer.java for example, does
not constitute a change to the code, so just keep the original year (see below).
The initial years for the classes:
EmptySerializer.java 2012
SerializerBase.java 2012
ToSAXHandler.java none (meaning if you make changes to this class,
just add 2016)
ToStream.java 2006
ToUnknownStream.java 2007
XSLOutputAttributes.java none (so keep the original "DO NOT REMOVE
OR ALTER!" block)
Thanks,
Joe
On 11/16/16, 6:22 AM, Langer, Christoph wrote:
Hi,
please review another XALAN fix.
The Serializer should be able to handle text nodes with null input. There has
already been some discussion here:
http://mail.openjdk.java.net/pipermail/core-libs-dev/2016-November/044567.html
Bug: https://bugs.openjdk.java.net/browse/JDK-8169772
Webrev:
http://cr.openjdk.java.net/~clanger/webrevs/8169772.0/<http://cr.openjdk.java.net/%7Eclanger/webrevs/8169772.0/>
The actual fix is in ToUnknownStream.java, method "public void
characters(String chars) throws SAXException". I don't know if one should even
directly return after chars being null or of size() 0. The rest of this change
is cleanups and a test case.
Thanks for reviewing.
Best regards
Christoph
RE: Issues running JAXP jtreg tests ("java.lang.RuntimePermission" "accessDeclaredMembers")
Thanks, Frank.
we run scheduled jtreg tests for jdk every night so we should have encountered
issues if there were some. But langtools could be interesting, I don't think
those run automatically for OpenJDK in our environment.
Best regards
Christoph
> -Original Message-
> From: Frank Yuan [mailto:frank.y...@oracle.com]
> Sent: Mittwoch, 23. November 2016 06:26
> To: Langer, Christoph ; 'Volker Simonis'
> ; 'Daniel Fuchs'
> Cc: code-tools-...@openjdk.java.net; core-libs-dev@openjdk.java.net; jtreg-
> u...@openjdk.java.net
> Subject: RE: Issues running JAXP jtreg tests ("java.lang.RuntimePermission"
> "accessDeclaredMembers")
>
> Hi Christoph and Volker
>
> I have been launching jdk and langtools tests with the new jtreg, will update
> to
> you once I get the result.
> Hope jaxp test is special because most of tests should control the Security
> Manager setting inside the test methods.
>
> Thanks
> Frank
>
>
>
> > -----Original Message-
> > From: core-libs-dev [mailto:core-libs-dev-boun...@openjdk.java.net] On
> Behalf Of Langer, Christoph
> > Sent: Wednesday, November 23, 2016 3:51 AM
> > Subject: RE: Issues running JAXP jtreg tests ("java.lang.RuntimePermission"
> "accessDeclaredMembers")
> >
> > Thanks a lot Volker and Daniel for the big support to analyze and fix this.
> >
> > It seems to me that the proposed fix
> > (http://cr.openjdk.java.net/~dfuchs/webrev_8170192/webrev.00/ ) looks like
> > the best that can be done at the moment. I agree that it would be nicer if
> > jtreg would leave the jtreg lib path as java property to be able to elevate
> > all of its contents. But the current proposal with a set of TEST_JARS of
> > jtreg, javatest and testng is probably sufficient for jaxp testing.
> >
> > The best thing to find out about other issues with the new version of testng
> > would certainly be if Oracle's internal version of jtreg be updated to use
> > the latest testng :-)
> >
> > Best regards
> > Christoph
> >
> >
> >
> > > -Original Message-
> > > From: Volker Simonis [mailto:volker.simo...@gmail.com]
> > > Sent: Dienstag, 22. November 2016 20:25
> > > To: Daniel Fuchs
> > > Cc: Langer, Christoph ; code-tools-
> > > d...@openjdk.java.net; core-libs-dev@openjdk.java.net; jtreg-
> > > u...@openjdk.java.net
> > > Subject: Re: Issues running JAXP jtreg tests
> > > ("java.lang.RuntimePermission"
> > > "accessDeclaredMembers")
> > >
> > > Hi Daniel,
> > >
> > > thanks for your patch!
> > >
> > > I've meanwhile tried to better understand the root cause of the problem.
> > >
> > > I don't think that the invocation order of the data provider the
> > > listener have changed. If you look at the the two version 6.9.5 and
> > > 6.9.13 of testng, the org.testng.TestRunner.run() methods look exactly
> > > the same in both 6.9.5 [1] and 6.9.13 [2]:
> > >
> > > public void run() {
> > > beforeRun();
> > >
> > > try {
> > > XmlTest test= getTest();
> > > if(test.isJUnit()) {
> > > privateRunJUnit(test);
> > > }
> > > else {
> > > privateRun(test);
> > > }
> > > }
> > > finally {
> > > afterRun();
> > > }
> > >
> > > I think the problem is in
> > > org.testng.internal.ClassHelper.getAvailableMethods() where we testng
> > > only collected the methods until (i.e. excluding) java.lang.Object in
> > > 6.9.5 [3] but including java.lang.Object in 6.9.13 [4]:
> > >
> > > 6.9.5
> > > =
> > > public static Set getAvailableMethods(Class clazz) {
> > > Set methods = Sets.newHashSet();
> > > methods.addAll(Arrays.asList(clazz.getDeclaredMethods()));
> > >
> > > Class parent = clazz.getSuperclass();
> > > while (Object.class != parent) {
> > > methods.addAll(extractMethods(clazz, parent, methods));
> > > parent = parent.getSuperclass();
> > > }
> > >
> > > 6.9.13
> > > =
> > > public static Set getAvailableMethods(Class clazz) {
> > > Set methods = Sets.newHashSet();
> > > methods.addAll(Arrays.asList(clazz.getDeclaredMethods()));
> > >
> > > Class parent = clazz.getSuperclass();
> > > while (null
RE: RFR JDK-8170192 [JAXP] [TESTBUG] test/javax/xml/jaxp/libs/jaxp/library/JAXPPolicyManager.java should grant permissions to jtreg, javatest, and testng jars
Hi Frank, to me this looks fine. Maybe a small cosmetical thing: Around lines 212 and 240 you could remove the additional empty lines to have one blank line between methods as in the rest of the file. Note that I'm not a reviewer. Best regards Christoph From: Frank Yuan [mailto:frank.y...@oracle.com] Sent: Mittwoch, 23. November 2016 05:41 To: core-libs-dev@openjdk.java.net; 'Daniel Fuchs' ; Langer, Christoph ; 'Volker Simonis' ; 'Joe Wang' Cc: code-tools-...@openjdk.java.net; jtreg-...@openjdk.java.net Subject: RFR JDK-8170192 [JAXP] [TESTBUG] test/javax/xml/jaxp/libs/jaxp/library/JAXPPolicyManager.java should grant permissions to jtreg, javatest, and testng jars Hi All Would you like to review http://cr.openjdk.java.net/~fyuan/8170192/webrev.00/? Bug: https://bugs.openjdk.java.net/browse/JDK-8170192 This patch is fully same as Daniel provided except a few lines of additional cleaning, thanks to Daniel for providing the patch! Thanks Frank
RE: RFR: 8169772: [JAXP] XALAN: Transformation of DOM with null valued text node causes NPE
Hi Joe,
tests went well and I pushed:
http://hg.openjdk.java.net/jdk9/dev/jaxp/rev/31ac7aab52da
Thanks
Christoph
From: Joe Wang [mailto:huizhe.w...@oracle.com]
Sent: Dienstag, 22. November 2016 23:02
To: Langer, Christoph
Cc: core-libs-dev@openjdk.java.net
Subject: Re: RFR: 8169772: [JAXP] XALAN: Transformation of DOM with null valued
text node causes NPE
Hi Christoph,
That looks good to me.
Best,
Joe
On 11/22/16, 1:26 PM, Langer, Christoph wrote:
Hi Joe,
as my jtreg issues are solved now, I'm finalizing the patches.
For this one I've updated the test method:
http://cr.openjdk.java.net/~clanger/webrevs/8169772.2/<http://cr.openjdk.java.net/%7Eclanger/webrevs/8169772.2/>
Could you please quickly check if my new test method "testBug8169772()" looks
as you want it? I've added some more detailed comments for the method. The
point is that without the fix you'd encounter an NPE. Or should there be some
special assertion around it?
If it's ok for you, I'd push tomorrow.
Thanks & Best regards
Christoph
From: Joe Wang [mailto:huizhe.w...@oracle.com]
Sent: Samstag, 19. November 2016 00:23
To: Langer, Christoph
<mailto:christoph.lan...@sap.com>
Cc: core-libs-dev@openjdk.java.net<mailto:core-libs-dev@openjdk.java.net>
Subject: Re: RFR: 8169772: [JAXP] XALAN: Transformation of DOM with null valued
text node causes NPE
On 11/18/16, 12:17 PM, Langer, Christoph wrote:
Hi Joe,
Yep, I'll conduct the testing before pushing.
With the "Expected Result", you mean some comment for the test method, right?
Yes, some comment for the test method would do. But I see that you've added
assertions? That would be even better. Comments help in this case since without
the patch, the process would have thrown an NPE.
Once a bug is fixed, SQE will take the new test and verify the fix. Knowing the
success/fail (before and after fix) conditions is helpful for them.
Thanks,
Joe
Thanks
Christoph
From: Joe Wang [mailto:huizhe.w...@oracle.com]
Sent: Freitag, 18. November 2016 20:00
To: Langer, Christoph
<mailto:christoph.lan...@sap.com>
Cc: core-libs-dev@openjdk.java.net<mailto:core-libs-dev@openjdk.java.net>
Subject: Re: RFR: 8169772: [JAXP] XALAN: Transformation of DOM with null valued
text node causes NPE
Looks good, Christoph.
I assume you'll do an all-test run (all in jaxp/test) before pushing. No need
for further review, but it'd be nice to add an "Expected result" for the new
test testBug8169772, with/without the fix (e.g. NPE).
Best regards,
Joe
On 11/18/16, 4:38 AM, Langer, Christoph wrote:
Hi Joe,
thanks for the feedback.
I've created a new version of the webrev working in your suggestions, adding
some further formatting cleanups in the files and I also moved a small
refactoring in TransformerTest.java to this changeset.
http://cr.openjdk.java.net/~clanger/webrevs/8169772.1/<http://cr.openjdk.java.net/%7Eclanger/webrevs/8169772.1/>
>From my end this one is ready for pushing - waiting for your final go.
Best regards
Christoph
From: Joe Wang [mailto:huizhe.w...@oracle.com]
Sent: Freitag, 18. November 2016 07:36
To: Langer, Christoph
<mailto:christoph.lan...@sap.com>
Cc: core-libs-dev@openjdk.java.net<mailto:core-libs-dev@openjdk.java.net>
Subject: Re: RFR: 8169772: [JAXP] XALAN: Transformation of DOM with null valued
text node causes NPE
Looks fine.
License header: in general, don't change / add Year if there's no material
change, removing the legacy $Id field in EmptySerializer.java for example, does
not constitute a change to the code, so just keep the original year (see below).
The initial years for the classes:
EmptySerializer.java 2012
SerializerBase.java 2012
ToSAXHandler.java none (meaning if you make changes to this class,
just add 2016)
ToStream.java 2006
ToUnknownStream.java 2007
XSLOutputAttributes.java none (so keep the original "DO NOT REMOVE
OR ALTER!" block)
Thanks,
Joe
On 11/16/16, 6:22 AM, Langer, Christoph wrote:
Hi,
please review another XALAN fix.
The Serializer should be able to handle text nodes with null input. There has
already been some discussion here:
http://mail.openjdk.java.net/pipermail/core-libs-dev/2016-November/044567.html
Bug: https://bugs.openjdk.java.net/browse/JDK-8169772
Webrev:
http://cr.openjdk.java.net/~clanger/webrevs/8169772.0/<http://cr.openjdk.java.net/%7Eclanger/webrevs/8169772.0/>
The actual fix is in ToUnknownStream.java, method "public void
characters(String chars) throws SAXException". I don't know if one should even
directly return after chars being null or of size() 0. The rest of this change
is cleanups and a test case.
Thanks for reviewing.
Best regards
Christoph
RE: RFR: 8169631: [JAXP] XALAN: transformation of XML via namespace-unaware SAX input yields a different result than namespace-unaware DOM input
Hi Joe, when trying to finish this up, I had a closer look again. I found out that it's necessary to look at attributes as well and handle the case where namespace prefixes are used. Here is a new version of the changeset that passes all jaxp jtreg tests: http://cr.openjdk.java.net/~clanger/webrevs/8169631.1/ Now I would also throw out prefixes in localName if we don't have namespace support - for both, elements and attributes (in SAX2DTM2.startElement()). I also removed some qname handling in DOM2SAX as it is not needed with my changes to SAX2DTM2 anymore. The test case was adopted. Thanks & best regards Christoph > -Original Message- > From: Joe Wang [mailto:huizhe.w...@oracle.com] > Sent: Dienstag, 22. November 2016 20:11 > To: Langer, Christoph > Cc: core-libs-dev@openjdk.java.net > Subject: Re: RFR: 8169631: [JAXP] XALAN: transformation of XML via > namespace-unaware SAX input yields a different result than namespace- > unaware DOM input > > Hi Christoph, > > Once you're able to run all tests, feel free to push the changeset. > Frank has fixed the Smoke test. > > Thanks, > Joe > > On 11/18/16, 3:37 PM, Joe Wang wrote: > > Hi Christoph, > > > > Thanks for explaining the customer's dilemma with regard to their > > legacy process. > > > > The testcase I sent you was extracted from an internal SQE smoke test. > > I agree with your analysis, the 'golden' file which has been in there > > for over 10 years turns out to be wrong and needs to be updated. > > > > To fix this issue, we need to get that test fixed, and the check-in of > > your patch and that of the test need needs to happen simultaneously. > > Would you mind wanting for me to go through an internal process to get > > a patch ready, then we can check in almost at the same time? > > > > Best, > > Joe > > > > On 11/18/16, 2:51 PM, Langer, Christoph wrote: > >> Hi Joe, > >> > >> thanks for the feedback. > >> > >> I've now understood the testcase that you've sent over and the reason > >> that it is reporting failure after my fix is that the output of its > >> transform operation is rather correct now. And before it was wrong. :) > >> The test is comparing the actual result to a "golden" result file in > >> the end and both of these were not looking healthy so far. The reason > >> is that your test is using a namespace unaware SAX Parser as input. > >> With the current JDK XALAN, you could already modify your smoketest > >> to use a namespace aware parser. > >> > >> E.g. replace lines > >> > >> 82// Use the JAXP way to get an XMLReader > >> 83XMLReader reader = > >> SAXParserFactory.newInstance().newSAXParser().getXMLReader(); > >> > >> with > >> > >> 82// Use the JAXP way to get an XMLReader > >> 83SAXParserFactory spf = SAXParserFactory.newInstance(); > >> 84spf.setNamespaceAware(true); > >> 85XMLReader reader = spf.newSAXParser().getXMLReader(); > >> > >> ...and you would already get correct results that also DOM input or > >> Stream Input would yield. > >> > >> So, are there other concerns/issues with this fix? Do you want me to > >> include a transformation operation like the one that your SmokeTest > >> does to TransformerTest which would illustrate the problem with > >> namespace unaware SAX input data? > >> > >> Best regards > >> Christoph > >> > >>> -Original Message- > >>> From: Joe Wang [mailto:huizhe.w...@oracle.com] > >>> Sent: Freitag, 18. November 2016 05:53 > >>> To: Langer, Christoph > >>> Cc: core-libs-dev@openjdk.java.net > >>> Subject: Re: RFR: 8169631: [JAXP] XALAN: transformation of XML via > >>> namespace-unaware SAX input yields a different result than namespace- > >>> unaware DOM input > >>> > >>> > >>> > >>> On 11/14/16, 11:43 PM, Langer, Christoph wrote: > >>>> Hi Joe, > >>>> > >>>> thanks for looking. > >>>> > >>>> Can you let me know which smoke test is failing? I didn't see > >>>> issues so far - I > >>> was merely running the jtreg unittests for transformer. > >>> > >>> I sent the test to your mailbox. > >>>> I stepped back from replacing Vector with ArrayList for > >>>> m_prefixMappings > >>
RE: RFR (JAXP) 8167340: XMLStreamReader.getElementText return corrupt content when size of element is > 8192
Hi Joe,
to me this looks good.
Did you already remove the cleanups from
http://cr.openjdk.java.net/~joehw/jdk9/8167340/webrev/ ? I can't see a lot of
them any more...
A few minor points:
It seems you still have left some debugging code in
test/javax/xml/jaxp/unittest/stream/XMLStreamReaderTest/StreamReaderTest.java:
59 while (xmlStreamReader.hasNext()) {
60 int event = xmlStreamReader.next();
61 if (event == XMLStreamConstants.START_ELEMENT) {
62 if (xmlStreamReader.getLocalName().equals("body"))// &&
bMessage)
-> remove the && bMessage)
63 {
64 String elementText = xmlStreamReader.getElementText();
65 //System.out.println("elementText=" + elementText +
"EndOfElementText");
-> the commented System.out.println statement should be removed at all, I
suggest
66 // fail if elementText contains ""
67 Assert.assertTrue(!elementText.contains(""),
"Fail: elementText contains ");
68 }
69 }
70 }
Other than that I'm wondering if the 80 chars line limit shall be held which is
not completely true in StreamReaderTest.java. But I know how difficult that is,
while keeping the code still readable. Especially with the long speaking Class
and method names ;-)
Best regards
Christoph
> -Original Message-
> From: core-libs-dev [mailto:core-libs-dev-boun...@openjdk.java.net] On Behalf
> Of Joe Wang
> Sent: Mittwoch, 30. November 2016 22:21
> To: core-libs-dev@openjdk.java.net
> Subject: RFR (JAXP) 8167340: XMLStreamReader.getElementText return corrupt
> content when size of element is > 8192
>
> Hi,
>
> Please review an one-line fix and a bunch of cleanups.
>
> The reported issue was caused by a missed setting when the
> XMLStreamReader initializes the XML 1.1 scanner, so while the changeset
> involved 350 lines, the fix is really just the following:
>
> XMLStreamReaderImpl.java:
> @@ -605,11 +604,12 @@
> ...
> +fEntityScanner.registerListener(fScanner);
>
>
> All other changes are cleanups, warnings. And BTW, warnings in the jaxp
> repo have come down from 5230 in 2015 to 3265, a result of a bit of
> cleanups here and there when we touch those classes. Still a long way to
> go, and it shows we may need to have a few dedicated patches.
>
> JBS: https://bugs.openjdk.java.net/browse/JDK-8167340
> webrevs: http://cr.openjdk.java.net/~joehw/jdk9/8167340/webrev/
>
> Thanks,
> Joe
RE: RFR (JAXP) 8167340: XMLStreamReader.getElementText return corrupt content when size of element is > 8192
Hi Joe,
thanks, looks fine now.
In
test/javax/xml/jaxp/unittest/stream/XMLStreamReaderTest/StreamReaderTest.java,
the brace in line 63 could still move one line up, but this really is
nit-picking ;-)
Best regards
Christoph
> -Original Message-
> From: Joe Wang [mailto:huizhe.w...@oracle.com]
> Sent: Donnerstag, 1. Dezember 2016 18:25
> To: Daniel Fuchs
> Cc: Langer, Christoph ; core-libs-
> d...@openjdk.java.net
> Subject: Re: RFR (JAXP) 8167340: XMLStreamReader.getElementText return
> corrupt content when size of element is > 8192
>
> Thanks Christoph, Daniel! I've updated the test, removing the comments.
>
> As for 80 chars line limit, or slightly longer than that is fine with
> me. Given our codebase, it would be an enormous undertaking. I'm
> therefore fine with taking care of only the extremely long ones or any
> that impedes the reviews, that is, side-by-side view.
>
> Best,
> Joe
>
> On 12/1/16, 2:25 AM, Daniel Fuchs wrote:
> > Hi Joe,
> >
> > I agree with Christoph's comments below.
> >
> > +1
> >
> > best regards,
> >
> > -- daniel
> >
> > On 01/12/16 07:40, Langer, Christoph wrote:
> >> Hi Joe,
> >>
> >> to me this looks good.
> >>
> >> Did you already remove the cleanups from
> >> http://cr.openjdk.java.net/~joehw/jdk9/8167340/webrev/ ? I can't see
> >> a lot of them any more...
> >>
> >> A few minor points:
> >>
> >> It seems you still have left some debugging code in
> >>
> test/javax/xml/jaxp/unittest/stream/XMLStreamReaderTest/StreamReaderTest
> .java:
> >>
> >> 59 while (xmlStreamReader.hasNext()) {
> >> 60 int event = xmlStreamReader.next();
> >> 61 if (event == XMLStreamConstants.START_ELEMENT) {
> >> 62 if
> >> (xmlStreamReader.getLocalName().equals("body"))// && bMessage)
> >>
> >> -> remove the && bMessage)
> >>
> >> 63 {
> >> 64 String elementText =
> >> xmlStreamReader.getElementText();
> >> 65 //System.out.println("elementText=" +
> >> elementText + "EndOfElementText");
> >>
> >> -> the commented System.out.println statement should be removed at
> >> all, I suggest
> >>
> >> 66 // fail if elementText contains ""
> >> 67
> >> Assert.assertTrue(!elementText.contains(""), "Fail:
> >> elementText contains ");
> >> 68 }
> >> 69 }
> >> 70 }
> >>
> >> Other than that I'm wondering if the 80 chars line limit shall be
> >> held which is not completely true in StreamReaderTest.java. But I
> >> know how difficult that is, while keeping the code still readable.
> >> Especially with the long speaking Class and method names ;-)
> >>
> >> Best regards
> >> Christoph
> >>
> >>
> >>> -Original Message-
> >>> From: core-libs-dev [mailto:core-libs-dev-boun...@openjdk.java.net]
> >>> On Behalf
> >>> Of Joe Wang
> >>> Sent: Mittwoch, 30. November 2016 22:21
> >>> To: core-libs-dev@openjdk.java.net
> >>> Subject: RFR (JAXP) 8167340: XMLStreamReader.getElementText return
> >>> corrupt
> >>> content when size of element is > 8192
> >>>
> >>> Hi,
> >>>
> >>> Please review an one-line fix and a bunch of cleanups.
> >>>
> >>> The reported issue was caused by a missed setting when the
> >>> XMLStreamReader initializes the XML 1.1 scanner, so while the changeset
> >>> involved 350 lines, the fix is really just the following:
> >>>
> >>> XMLStreamReaderImpl.java:
> >>> @@ -605,11 +604,12 @@
> >>> ...
> >>> +fEntityScanner.registerListener(fScanner);
> >>>
> >>>
> >>> All other changes are cleanups, warnings. And BTW, warnings in the jaxp
> >>> repo have come down from 5230 in 2015 to 3265, a result of a bit of
> >>> cleanups here and there when we touch those classes. Still a long
> >>> way to
> >>> go, and it shows we may need to have a few dedicated patches.
> >>>
> >>> JBS: https://bugs.openjdk.java.net/browse/JDK-8167340
> >>> webrevs: http://cr.openjdk.java.net/~joehw/jdk9/8167340/webrev/
> >>>
> >>> Thanks,
> >>> Joe
> >
RE: [9] RFR: 8170465, 8170466: JNI exception pending in jni_util.c:190
Hi Naoto, the new webrev looks ok to me, too. Thanks for fixing. And I agree to the point that the NULL check can be removed...if you like. Best regards Christoph > -Original Message- > From: core-libs-dev [mailto:core-libs-dev-boun...@openjdk.java.net] On Behalf > Of David Holmes > Sent: Mittwoch, 7. Dezember 2016 03:15 > To: Naoto Sato ; core-libs-dev d...@openjdk.java.net> > Subject: Re: [9] RFR: 8170465, 8170466: JNI exception pending in > jni_util.c:190 > > On 7/12/2016 11:04 AM, Naoto Sato wrote: > > Thanks, David. The fix is modified accordingly: > > > > http://cr.openjdk.java.net/~naoto/8170465.8170466/webrev.01/ > > Thanks for fixing that. > > The null checks after the exception checks are now redundant as you can > only get a null return from those methods if an exception becomes > pending. But leaving them in as a precaution is okay I guess. > > Thanks, > David > > > Naoto > > > > On 12/6/16 4:53 PM, David Holmes wrote: > >> Hi Naoto, > >> > >> On 7/12/2016 10:02 AM, Naoto Sato wrote: > >>> Hello, > >>> > >>> Please review the proposed fix to the following issues: > >>> > >>> https://bugs.openjdk.java.net/browse/JDK-8170465 > >>> https://bugs.openjdk.java.net/browse/JDK-8170466 > >>> > >>> The proposed fix is located at: > >>> > >>> http://cr.openjdk.java.net/~naoto/8170465.8170466/webrev.00/ > >>> > >>> These two JNI calls (NewStringUTF() and JNU_CallMethodByName()) in > >>> src/java.base/share/native/libjava/jni_util.c can throw an exception. > >>> Those possible exceptions need to be properly checked. > >> > >> JNU_CHECK_EXCEPTION will do a return from the method in which it is > >> placed, so you need to be careful to ensure you do all needed cleanup > >> before that eg: > >> > >> 203 JNU_CHECK_EXCEPTION(env); > >> 204 free(str1); > >> > >> needs to be reversed else we won't free str1. Similarly: > >> > >> 210 JNU_CHECK_EXCEPTION(env); > >> 211 (*env)->DeleteLocalRef(env, s2); > >> > >> needs to be reversed so we don't leak the local ref. (It is safe to call > >> DeleteLocalRef with an exception pending.) > >> > >> Thanks, > >> David > >> > >>> Naoto
RE: RFR 8169115, java/net/InetAddress/ptr/lookup.sh failed intermittently
Hi Felix,
looks good also for me.
Small typo:
85 throw new RuntimeException("Mistmatch between default"
Mistmatch -> Mismatch :)
Best regards
Christoph
> -Original Message-
> From: net-dev [mailto:net-dev-boun...@openjdk.java.net] On Behalf Of Felix
> Yang
> Sent: Donnerstag, 8. Dezember 2016 11:35
> To: Dmitry Samersoff ; core-libs-
> d...@openjdk.java.net; net-...@openjdk.java.net; CHRIS.HEGARTY
>
> Subject: Re: RFR 8169115, java/net/InetAddress/ptr/lookup.sh failed
> intermittently
>
> Hi Dmitry,
>
> I tested your suggested "icann.org" and it indeed works well. Please
> review the updated webrev:
>
> http://cr.openjdk.java.net/~xiaofeya/8169115/webrev.02/
>
> Thanks,
> Felix
> On 2016/12/6 19:16, Dmitry Samersoff wrote:
> > Felix,
> >
> > 1. I'm not sure that javaweb.sfbay.sun.com is the best domain name for
> > this test. Could we use different one (e.g. icann.org)
> >
> > 2. This test run JTREG -> Test VM -> Another VM. Could we optimize
> > process usage?
> >
> > It might be better to create a jtreg "same vm" process that
> >
> > 1. launch another process with -Djava.net.preferIPv4Stack=true
> > that do A and PRT lookup in one run.
> >
> > 2. Read results of process above, do PTR lookup with default
> > settings and compare results.
> >
> > -Dmitry
> >
> >
> > On 2016-12-06 12:06, Felix Yang wrote:
> >> Hi,
> >>
> >> please review the following patch. It generally coverts codes from
> >> shell to plain java.
> >>
> >> Bug:
> >>
> >> https://bugs.openjdk.java.net/browse/JDK-8169115
> >>
> >> Webrev:
> >>
> >> http://cr.openjdk.java.net/~xiaofeya/8169115/webrev.00/
> >>
> >> Thanks,
> >>
> >> Felix
> >>
> >
RFR (JAXP): 8169112: java.lang.VerifyError: (class: GregorSamsa, method: template-bash signature: (LGregorSamsa8; )V) Register 10 contains wrong type
Hi, this is the fix for the regression introduced with 8150704. Please review. Bug: https://bugs.openjdk.java.net/browse/JDK-8169112 Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8169112.0/ The problem occurs during "outlining" of a translet method. Outlining happens when the size of bytecode for a translet method exceeds the bytecode limit per method imposed by Java and means splitting the code into smaller methods. 8150704 added the new local variable "_domAdapter" to the implementation of "WithParam" without setting the end of its scope. This somehow leads to issues in outlining and the local variable in the new method might be loaded without being initialized. The problem is not observed for smaller translets where probably outlining is not performed. Shall I create a new jtreg test from the example attached to the bug? I would just run the sample transformation and the test is passed when no exception occurs. Best regards Christoph
RE: RFR (JAXP) 8170556: Warnings cleanup related to JDK-8167340
Hi Joe,
looks nice, thanks for doing that.
Here are a few findings:
src/java.xml/share/classes/com/sun/org/apache/xerces/internal/impl/XMLStreamReaderImpl.java:
-> import statements could be ordered alphabetically
262 fEntityScanner = fEntityManager.getEntityScanner() ;
-> spaces before ;
1317 protected List getEntityDecls(){
-> space before opening {
1322 if(entities.size() > 0){
-> spaces after if, before {
1344 protected List getNotationDecls(){
-> space before {
1352 if(notation!= null){
-> spaces
src/java.xml/share/classes/com/sun/xml/internal/stream/XMLEntityStorage.java
145 }
146 /**
-> insert blank line in between
src/java.xml/share/classes/com/sun/xml/internal/stream/dtd/nonvalidating/DTDGrammar.java
734 public List getNotationDecls(){
-> blank before {
src/java.xml/share/classes/com/sun/xml/internal/stream/events/DTDEvent.java
66 public void setEntities(List entites){
-> space before {; variable name entites -> entities
77 public void setNotations(List notations){
-> space
94 protected final void init(){
-> space
src/java.xml/share/classes/com/sun/xml/internal/stream/events/EndElementEvent.java
-> order import statements alphabetically
48 QName fQName ;
-> space
105 void addNamespace(Namespace ns){
-> space
106 if(ns != null){
-> spaces
src/java.xml/share/classes/com/sun/xml/internal/stream/events/StartElementEvent.java
-> import statements order, a few space issues
src/java.xml/share/classes/com/sun/xml/internal/stream/writers/XMLEventWriterImpl.java
68 public void add(javax.xml.stream.XMLEventReader xMLEventReader) throws
XMLStreamException {
80 public void add(javax.xml.stream.events.XMLEvent xMLEvent) throws
XMLStreamException {
-> you should be able to use unqualified names for parameters
src/java.xml/share/classes/com/sun/xml/internal/stream/writers/XMLStreamWriterImpl.java
906 ElementState elem;
907
908 while (!fElementStack.empty()) {
909 elem = fElementStack.pop();
-> I think elem can be declared in line 909 as well, scope is only within
while() block
Best regards
Christoph
> -Original Message-
> From: core-libs-dev [mailto:core-libs-dev-boun...@openjdk.java.net] On Behalf
> Of Joe Wang
> Sent: Montag, 12. Dezember 2016 20:14
> To: core-libs-dev@openjdk.java.net
> Subject: RFR (JAXP) 8170556: Warnings cleanup related to JDK-8167340
>
> Hi,
>
> This was the cleanup portion of the change for JDK-8167340. As Lance
> suggested, it was split from the original webrev. In addition to that
> cleanup, I've added coverage to the entire StAX packages. This cleanup
> will reduce 138 warnings.
>
> jbs: https://bugs.openjdk.java.net/browse/JDK-8170556
> webrev: http://cr.openjdk.java.net/~joehw/jdk9/8170556/webrev/
>
> Thanks,
> Joe
RE: RFR (JAXP) 8170556: Warnings cleanup related to JDK-8167340
Thanks, Joe.
Overall, this looks better.
> -Original Message-
> From: Joe Wang [mailto:huizhe.w...@oracle.com]
> Sent: Mittwoch, 14. Dezember 2016 01:42
> To: Langer, Christoph
> Cc: core-libs-dev@openjdk.java.net
> Subject: Re: RFR (JAXP) 8170556: Warnings cleanup related to JDK-8167340
>
> Thanks Christoph!
>
> I updated the webrev for the classes you mentioned below, in a few
> cases, used NetBeans' source format feature -- not for all of the
> classes though (esp. the crazily large
> XMLDocumentFragmentScannerImpl.java, it gets better though, overtime).
>
> http://cr.openjdk.java.net/~joehw/jdk9/8170556/webrev/
>
> Best regards,
> Joe
>
> On 12/13/16, 2:14 PM, Langer, Christoph wrote:
> > Hi Joe,
> >
> > looks nice, thanks for doing that.
> >
> > Here are a few findings:
> >
> >
> src/java.xml/share/classes/com/sun/org/apache/xerces/internal/impl/XMLStre
> amReaderImpl.java:
> > -> import statements could be ordered alphabetically
> > 262 fEntityScanner = fEntityManager.getEntityScanner() ;
> > -> spaces before ;
> > 1317 protected List getEntityDecls(){
> > -> space before opening {
> > 1322 if(entities.size()> 0){
> > -> spaces after if, before {
> > 1344 protected List getNotationDecls(){
> > -> space before {
> > 1352 if(notation!= null){
> > -> spaces
> >
> >
> src/java.xml/share/classes/com/sun/xml/internal/stream/XMLEntityStorage.jav
> a
> > 145 }
> > 146 /**
> > -> insert blank line in between
> >
> >
> src/java.xml/share/classes/com/sun/xml/internal/stream/dtd/nonvalidating/DT
> DGrammar.java
> > 734 public List getNotationDecls(){
> > -> blank before {
> >
> >
> src/java.xml/share/classes/com/sun/xml/internal/stream/events/DTDEvent.jav
> a
> > 66 public void setEntities(List entites){
> > -> space before {; variable name entites -> entities
> > 77 public void setNotations(List notations){
> > -> space
> > 94 protected final void init(){
> > -> space
> >
> >
> src/java.xml/share/classes/com/sun/xml/internal/stream/events/EndElementE
> vent.java
> > -> order import statements alphabetically
> > 48 QName fQName ;
> > -> space
> > 105 void addNamespace(Namespace ns){
> > -> space
> > 106 if(ns != null){
> > -> spaces
> >
> >
> src/java.xml/share/classes/com/sun/xml/internal/stream/events/StartElement
> Event.java
> > -> import statements order, a few space issues
> >
> >
> src/java.xml/share/classes/com/sun/xml/internal/stream/writers/XMLEventWr
> iterImpl.java
> > 68 public void add(javax.xml.stream.XMLEventReader xMLEventReader)
> throws XMLStreamException {
> > 80 public void add(javax.xml.stream.events.XMLEvent xMLEvent) throws
> XMLStreamException {
> > -> you should be able to use unqualified names for parameters
> >
> >
> src/java.xml/share/classes/com/sun/xml/internal/stream/writers/XMLStream
> WriterImpl.java
> > 906 ElementState elem;
> > 907
> > 908 while (!fElementStack.empty()) {
> > 909 elem = fElementStack.pop();
> > -> I think elem can be declared in line 909 as well, scope is only within
> while() block
> >
> > Best regards
> > Christoph
> >
> >
> >
> >> -Original Message-
> >> From: core-libs-dev [mailto:core-libs-dev-boun...@openjdk.java.net] On
> Behalf
> >> Of Joe Wang
> >> Sent: Montag, 12. Dezember 2016 20:14
> >> To: core-libs-dev@openjdk.java.net
> >> Subject: RFR (JAXP) 8170556: Warnings cleanup related to JDK-8167340
> >>
> >> Hi,
> >>
> >> This was the cleanup portion of the change for JDK-8167340. As Lance
> >> suggested, it was split from the original webrev. In addition to that
> >> cleanup, I've added coverage to the entire StAX packages. This cleanup
> >> will reduce 138 warnings.
> >>
> >> jbs: https://bugs.openjdk.java.net/browse/JDK-8170556
> >> webrev: http://cr.openjdk.java.net/~joehw/jdk9/8170556/webrev/
> >>
> >> Thanks,
> >> Joe
RE: RFR (JAXP): 8169112: java.lang.VerifyError: (class: GregorSamsa, method: template-bash signature: (LGregorSamsa8; )V) Register 10 contains wrong type
Hi Joe, ok, the test is added: http://cr.openjdk.java.net/~clanger/webrevs/8169112.1/ I'm ready to push, once you are ok with it. One question: Is the copyright year of WithParam.java really 2016 or should it be corrected? I guess I should also request a downport to jdk8 immediately, as it is a regression, right? Best regards Christoph From: Joe Wang [mailto:huizhe.w...@oracle.com] Sent: Dienstag, 13. Dezember 2016 23:18 To: Langer, Christoph Cc: core-libs-dev@openjdk.java.net; Aleks Efimov ; jeff Dinkins Subject: Re: RFR (JAXP): 8169112: java.lang.VerifyError: (class: GregorSamsa, method: template-bash signature: (LGregorSamsa8;)V) Register 10 contains wrong type Hi Christoph, For the test, yes, please add an unit test based on the test submitted, sanitize / remove any private information in the original test where necessary. Best regards, Joe On 12/13/16, 12:31 PM, Langer, Christoph wrote: Hi, this is the fix for the regression introduced with 8150704. Please review. Bug: https://bugs.openjdk.java.net/browse/JDK-8169112 Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8169112.0/<http://cr.openjdk.java.net/%7Eclanger/webrevs/8169112.0/> The problem occurs during "outlining" of a translet method. Outlining happens when the size of bytecode for a translet method exceeds the bytecode limit per method imposed by Java and means splitting the code into smaller methods. 8150704 added the new local variable "_domAdapter" to the implementation of "WithParam" without setting the end of its scope. This somehow leads to issues in outlining and the local variable in the new method might be loaded without being initialized. The problem is not observed for smaller translets where probably outlining is not performed. Shall I create a new jtreg test from the example attached to the bug? I would just run the sample transformation and the test is passed when no exception occurs. Best regards Christoph
RE: RFR (JAXP) 8146271: File system contention in debug print via XPathFactory.newInstance
+1 This is cool :) > -Original Message- > From: core-libs-dev [mailto:core-libs-dev-boun...@openjdk.java.net] On Behalf > Of Joe Wang > Sent: Mittwoch, 14. Dezember 2016 20:26 > To: Aleks Efimov > Cc: core-libs-dev > Subject: Re: RFR (JAXP) 8146271: File system contention in debug print via > XPathFactory.newInstance > > Hi Aleksej, > > Looks good. Thanks for covering the whole packages! > > Best, > Joe > > On 12/14/16, 10:04 AM, Aleks Efimov wrote: > > Hi Joe, > > > > Thank you for the suggestions. What about modifying the 'debugPrintln' > > and 'dPrint' functions to accept 'j.u.function.Supplier' > > instead of 'String'? Such approach will give us a possibility to do > > the output string calculation only when debugging is switched on. Such > > approach can be illustrated by this webrev: > > http://cr.openjdk.java.net/~aefimov/8146271/9/01 > > > > Best Regards, > > Aleksej > > > > > > On 14/12/16 03:35, Joe Wang wrote: > >> Hi Aleksej, > >> > >> You may want to improve the debugPrintln or its usage to remove the > >> String concatenations or method calls such as f.getClass().getName() > >> that are unnecessary when debug == false. Where we are here, could > >> you expand the patch to cover other jaxp packages (e.g. > >> javax.xml.parsers) where similar problems exist. > >> > >> Best, > >> Joe > >> > >> On 12/13/16, 3:02 PM, Aleks Efimov wrote: > >>> Hello, > >>> > >>> Please, help to review the changes that addresses the file system > >>> contention caused by debug code in XPathFactoryFinder and > >>> XPathFactoryFinder classes [1]. Proposed fix wraps the debug output > >>> code in "if(debug)" block: > >>> http://cr.openjdk.java.net/~aefimov/8146271/9/00/ > >>> > >>> Best Regards, > >>> Aleksej > >>> > >>> [1] https://bugs.openjdk.java.net/browse/JDK-8146271 > >>> > >
RE: RFR (JAXP) JDK-8087303 LSSerializer pretty print does not work anymore and JDK-8114834 LSSerializerImpl always serializes an entity reference node to" &entityName;" even if "entities" property is
Hi Frank,
this is awesome.
Some minor things I saw while looking through the webrev:
src/java.xml/share/classes/com/sun/org/apache/xalan/internal/xsltc/runtime/AbstractTranslet.java
Update copryright year
Remove:
20 /*
21 * $Id: AbstractTranslet.java,v 1.6 2006/06/19 19:49:03 spericas Exp $
22 */
src/java.xml/share/classes/com/sun/org/apache/xalan/internal/xsltc/trax/TransformerImpl.java
Remove:
20 /*
21 * $Id: TransformerImpl.java,v 1.10 2007/06/13 01:57:09 joehw Exp $
22 */
src/java.xml/share/classes/com/sun/org/apache/xml/internal/serializer/SerializerBase.java
462 protected boolean isInEntityRef()
463 {
Put the brace on line 462 as well
src/java.xml/share/classes/com/sun/org/apache/xml/internal/serializer/ToHTMLStream.java
-> sort import statements
Method
773 public void startElement(
-> use SAXException without package name. It is imported on top. This can be
done in various places throughout the file.
780 //will add extra one if having namespace but no matter
-> space between // and will...
821 if ((null != elemContext.m_elementName)
-> For me it reads better if it were if ((elemContext.m_elementName != null)
1971 private void initToHTMLStream()
1972 {
1973 //m_elementDesc = null;
1974 m_isprevblock = false;
1975 m_inDTD = false;
1976 //m_isRawStack.clear();
1977 m_omitMetaTag = false;
1978 m_specialEscapeURLs = true;
1979 }
-> I guess you could remove the commented statements
src/java.xml/share/classes/com/sun/org/apache/xml/internal/serializer/ToStream.java
116 protected boolean m_ispreserveSpace = false;
117
118
-> remove one empty line (117)
1894 m_ispreserve = false;
1895
1896
1897
-> newly inserted lines 1896 and 1897 should be removed
src/java.xml/share/classes/com/sun/org/apache/xml/internal/serializer/output_html.properties
-> maybe the Oracle copyright header can be inserted and the "$Id:
output_html.properties..." part can be removed?
Best regards
Christoph
> -Original Message-
> From: core-libs-dev [mailto:core-libs-dev-boun...@openjdk.java.net] On Behalf
> Of Joe Wang
> Sent: Mittwoch, 14. Dezember 2016 04:09
> To: Frank Yuan
> Cc: core-libs-dev@openjdk.java.net
> Subject: Re: RFR (JAXP) JDK-8087303 LSSerializer pretty print does not work
> anymore and JDK-8114834 LSSerializerImpl always serializes an entity
> reference node to" &entityName;" even if "entities" property is false
>
> Hi Frank,
>
> Thanks for the diligent work! I think we've made a great improvement
> over the PrettyPrint (tremendous ;-) )
>
> Could you look into extracting the XML literal strings in the test into
> plain files (similar to the other 'gold' files)? What I'm hoping to see
> is that they'd look easily prettier in a text editor, and for that
> matter, the original xml files were a mess.
>
> The tests set PrettyPrint, and by default for html. It would be good to
> test the cases when it's turned off, that would help verify the
> non-pretty format was not changed.
>
> Thanks,
> Joe
>
> On 12/13/16, 6:55 AM, Frank Yuan wrote:
> > Hi all
> >
> > Webrev http://cr.openjdk.java.net/~fyuan/8087303_8114834/webrev.00/ is
> for JDK-8087303 and JDK-8114834, I have to combine the fix
> > because there is some interaction between them.
> > Bugs: https://bugs.openjdk.java.net/browse/JDK-8087303
> https://bugs.openjdk.java.net/browse/JDK-8114834
> >
> > Besides fixed some issues in xml serializer, I made the following changes in
> this patch:
> > 1. refined the handling for whitespace text
> > 2. added support for xml:space attribute
> > 3. changed the default indentation to 4
> > 4. refined the handling for HTML block element and inline element
> >
> > Would you like to have a look at this review?
> >
> > Thanks,
> >
> > Frank
> >
> >
RE: RFR (JAXP) JDK-8087303 LSSerializer pretty print does not work anymore and JDK-8114834 LSSerializerImpl always serializes an entity reference node to" &entityName;" even if "entities" property is
Hi Frank,
thanks, looks good :)
Best
Christoph
> -Original Message-
> From: Frank Yuan [mailto:frank.y...@oracle.com]
> Sent: Donnerstag, 15. Dezember 2016 11:52
> To: Langer, Christoph ; 'Joe Wang'
>
> Cc: core-libs-dev@openjdk.java.net
> Subject: RE: RFR (JAXP) JDK-8087303 LSSerializer pretty print does not work
> anymore and JDK-8114834 LSSerializerImpl always serializes an entity
> reference node to" &entityName;" even if "entities" property is false
>
> Hi Christoph
>
> Thank you for the review!
>
> Please check http://cr.openjdk.java.net/~fyuan/8087303_8114834/webrev.01/.
>
> I have updated the code as your comments except output_html.properties, I am
> not sure for the license of this file.
>
> To Joe
> Could you confirm Christoph's comment for output_html.properties?
>
>
> Thanks
> Frank
>
> > -Original Message-
> > From: Langer, Christoph [mailto:christoph.lan...@sap.com]
> > Subject: RE: RFR (JAXP) JDK-8087303 LSSerializer pretty print does not work
> anymore and JDK-8114834 LSSerializerImpl always
> serializes an
> > entity reference node to" &entityName;" even if "entities" property is false
> >
> > Hi Frank,
> >
> > this is awesome.
> >
> > Some minor things I saw while looking through the webrev:
> >
> >
> src/java.xml/share/classes/com/sun/org/apache/xalan/internal/xsltc/runtime/
> AbstractTranslet.java
> > Update copryright year
> > Remove:
> > 20 /*
> > 21 * $Id: AbstractTranslet.java,v 1.6 2006/06/19 19:49:03 spericas Exp $
> > 22 */
> >
> >
> src/java.xml/share/classes/com/sun/org/apache/xalan/internal/xsltc/trax/Tran
> sformerImpl.java
> > Remove:
> > 20 /*
> > 21 * $Id: TransformerImpl.java,v 1.10 2007/06/13 01:57:09 joehw Exp $
> > 22 */
> >
> >
> src/java.xml/share/classes/com/sun/org/apache/xml/internal/serializer/Seriali
> zerBase.java
> > 462 protected boolean isInEntityRef()
> > 463 {
> > Put the brace on line 462 as well
> >
> >
> src/java.xml/share/classes/com/sun/org/apache/xml/internal/serializer/ToHT
> MLStream.java
> > -> sort import statements
> > Method
> > 773 public void startElement(
> > -> use SAXException without package name. It is imported on top. This can be
> done in various places throughout the file.
> > 780 //will add extra one if having namespace but no matter
> > -> space between // and will...
> > 821 if ((null != elemContext.m_elementName)
> > -> For me it reads better if it were if ((elemContext.m_elementName != null)
> >
> > 1971 private void initToHTMLStream()
> > 1972 {
> > 1973 //m_elementDesc = null;
> > 1974 m_isprevblock = false;
> > 1975 m_inDTD = false;
> > 1976 //m_isRawStack.clear();
> > 1977 m_omitMetaTag = false;
> > 1978 m_specialEscapeURLs = true;
> > 1979 }
> > -> I guess you could remove the commented statements
> >
> >
> src/java.xml/share/classes/com/sun/org/apache/xml/internal/serializer/ToStre
> am.java
> > 116 protected boolean m_ispreserveSpace = false;
> > 117
> > 118
> > -> remove one empty line (117)
> >
> > 1894 m_ispreserve = false;
> > 1895
> > 1896
> > 1897
> > -> newly inserted lines 1896 and 1897 should be removed
> >
> >
> src/java.xml/share/classes/com/sun/org/apache/xml/internal/serializer/output
> _html.properties
> > -> maybe the Oracle copyright header can be inserted and the "$Id:
> output_html.properties..." part can be removed?
> >
> > Best regards
> > Christoph
> >
> > > -Original Message-
> > > From: core-libs-dev [mailto:core-libs-dev-boun...@openjdk.java.net] On
> Behalf
> > > Of Joe Wang
> > > Sent: Mittwoch, 14. Dezember 2016 04:09
> > > To: Frank Yuan
> > > Cc: core-libs-dev@openjdk.java.net
> > > Subject: Re: RFR (JAXP) JDK-8087303 LSSerializer pretty print does not
> > > work
> > > anymore and JDK-8114834 LSSerializerImpl always serializes an entity
> > > reference node to" &entityName;" even if "entities" property is false
> > >
> > > Hi Frank,
> > >
> > > Thanks for the diligent work! I think we've made a great improvement
> > > over the PrettyPrint (tremendous ;-) )
> > >
> > > Could you look into extracting the XML literal strings in
8u-dev: RFR (JAXP): 8169112: java.lang.VerifyError: (class: GregorSamsa, method: template-bash signature: (LGregorSamsa8; )V) Register 10 contains wrong type
Hi, please review(@Joe) & approve the downport of this regression fix. As it is a JAXP issue and includes a testcase, the change needs to be split into a jaxp part and a jdk part. The jaxp part applies cleanly after unshuffling, the JDK part had to be modified to fit into the jdk8 layout. Bug: https://bugs.openjdk.java.net/browse/JDK-8169112 JAXP: http://cr.openjdk.java.net/~clanger/webrevs/8169112_jaxp.8udev/ JDK: http://cr.openjdk.java.net/~clanger/webrevs/8169112_jdk.8udev/ Thanks & best regards Christoph From: Joe Wang [mailto:huizhe.w...@oracle.com] Sent: Mittwoch, 14. Dezember 2016 21:52 To: Langer, Christoph Cc: core-libs-dev@openjdk.java.net; Aleks Efimov ; jeff Dinkins Subject: Re: RFR (JAXP): 8169112: java.lang.VerifyError: (class: GregorSamsa, method: template-bash signature: (LGregorSamsa8;)V) Register 10 contains wrong type I guess I should also request a downport to jdk8 immediately, as it is a regression, right? Yes, that would be great. Please create a patch for JDK 8 or work with Aleksej (Aleksej backported your previous patch), and ask for approval through the jdk8-dev alias. Best, Joe
RFR(S): 8171906: Changes for 8148023 break AIX build
Hi, after the fix for 8148023 was pushed, the AIX build is broken. Please review a fix for this: Bug: https://bugs.openjdk.java.net/browse/JDK-8171906 Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8171906.0/ The build breaks because NAME_MAX is not defined on AIX (just like Solaris, I guess) and we have to define it in UnixFileSystem_md.c. When working on this fix, I spotted some compiler warnings indicating that if one is working with the APIs readdir64 and friends on AIX, one has to use the type DIR64 instead of DIR and to open/close with opendir64/closedir64. I guess that's specific to AIX, see IBM's documentation [1]. Furthermore, although fdopendir64 is not mentioned there but just fdopendir, both APIs exist from AIX 7.1 onwards (look at on an AIX 7.1). So I added some defines for AIX to redirect the API calls. Thanks and best regards Christoph [1] http://www.ibm.com/support/knowledgecenter/ssw_aix_71/com.ibm.aix.basetrf1/opendir.htm
RE: RFR(S): 8171906: Changes for 8148023 break AIX build
Hi Goetz, thanks for your comments. The platform sorting I'm using usually (in libnet) is Linux, AIX, Solaris, BSD. As we are libnio, are there other wishes ore guidelines? As for fdopendir in UnixNativeDispatcher: This function is resolved via dlsym to a function pointer. So I thought it's ok to just load fdopendir64 at dlsym time to the fdopendir ptr and we don't have to have #ifdefs in the function where fdopendir is called. Best regards Christoph > -Original Message- > From: Lindenmaier, Goetz > Sent: Donnerstag, 22. Dezember 2016 16:14 > To: Langer, Christoph ; core-libs- > d...@openjdk.java.net; nio-...@openjdk.java.net; ppc-aix-port- > d...@openjdk.java.net > Cc: Brian Burkhalter ; Roger Riggs > ; Volker Simonis > Subject: RE: RFR(S): 8171906: Changes for 8148023 break AIX build > > Hi Christoph, > > good to get this fixed. > > As you move the solaris section in UnixFileSystem_md.c, you could move it > behind BSD > to sort it alphabetically as in other places. > > Why did you choose to define opendir to opendir64, but not so for fdopendir in > UnixNativeDispatcher.c? > > Looks good otherwise. > > Best regards, > Goetz. > > > -Original Message- > > From: Langer, Christoph > > Sent: Donnerstag, 22. Dezember 2016 16:04 > > To: core-libs-dev@openjdk.java.net; nio-...@openjdk.java.net; ppc-aix-port- > > d...@openjdk.java.net > > Cc: Brian Burkhalter ; Roger Riggs > > ; Volker Simonis ; > > Lindenmaier, Goetz > > Subject: RFR(S): 8171906: Changes for 8148023 break AIX build > > > > Hi, > > > > > > > > after the fix for 8148023 was pushed, the AIX build is broken. Please > > review a > > fix for this: > > > > > > > > Bug: https://bugs.openjdk.java.net/browse/JDK-8171906 > > > > Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8171906.0/ > > > > > > > > The build breaks because NAME_MAX is not defined on AIX (just like Solaris, > > I > > guess) and we have to define it in UnixFileSystem_md.c. > > > > > > > > When working on this fix, I spotted some compiler warnings indicating that > > if > > one is working with the APIs readdir64 and friends on AIX, one has to use > > the > > type DIR64 instead of DIR and to open/close with opendir64/closedir64. I > guess > > that's specific to AIX, see IBM's documentation [1]. > > > > Furthermore, although fdopendir64 is not mentioned there but just fdopendir, > > both APIs exist from AIX 7.1 onwards (look at on an AIX 7.1). > > So I > > added some defines for AIX to redirect the API calls. > > > > > > > > Thanks and best regards > > > > Christoph > > > > > > > > [1] > > > http://www.ibm.com/support/knowledgecenter/ssw_aix_71/com.ibm.aix.baset > > rf1/opendir.htm > > > >
RE: RFR(S): 8171906: Changes for 8148023 break AIX build
Hi Brian, no problem. So, may I consider this reviewed? Thanks Christoph From: Brian Burkhalter [mailto:brian.burkhal...@oracle.com] Sent: Donnerstag, 22. Dezember 2016 16:56 To: Langer, Christoph Cc: core-libs-dev@openjdk.java.net; nio-...@openjdk.java.net; ppc-aix-port-...@openjdk.java.net; Roger Riggs ; Volker Simonis ; Lindenmaier, Goetz Subject: Re: RFR(S): 8171906: Changes for 8148023 break AIX build Sorry about that. There seems to a surprising lack of consistency for this across the platforms. Regards, Brian On Dec 22, 2016, at 7:03 AM, Langer, Christoph mailto:christoph.lan...@sap.com>> wrote: The build breaks because NAME_MAX is not defined on AIX (just like Solaris, I guess) and we have to define it in UnixFileSystem_md.c.
RE: RFR(S): 8171906: Changes for 8148023 break AIX build
Thanks, Brian. I've also added it to our nightly verification run and if all goes well I'll push it tomorrow. Best regards Christoph > -Original Message- > From: Brian Burkhalter [mailto:brian.burkhal...@oracle.com] > Sent: Donnerstag, 22. Dezember 2016 18:17 > To: Langer, Christoph > Cc: core-libs-dev@openjdk.java.net; ppc-aix-port-...@openjdk.java.net; nio- > d...@openjdk.java.net > Subject: Re: RFR(S): 8171906: Changes for 8148023 break AIX build > > Hi Christoph, > > I ran this patch through our regression test suite against the NIO tests and > it > built and passed the test on all platforms (Linux, OS X, Solaris, Windows). > So it's > +1 on the review. > > Thanks, > > Brian > > On Dec 22, 2016, at 8:27 AM, Brian Burkhalter > wrote: > > > Hi Christoph, > > > > It looks OK, but have you built it on the other Unix platforms? If > > necessary I > could run it through our regression suite first. > > > > Thanks, > > > > Brian > > > > On Dec 22, 2016, at 7:58 AM, Langer, Christoph > wrote: > > > >> no problem. So, may I consider this reviewed? > >> > >> From: Brian Burkhalter [mailto:brian.burkhal...@oracle.com] > >> Sorry about that. There seems to a surprising lack of consistency for this > across the platforms.
RE: RFR(S): 8171906: Changes for 8148023 break AIX build
Hi Volker, I think this is no concern. Either both, fdopendir and fdopendir64, exist (in AIX 7.1) or none (earlier AIX releases). And the code can live with both situations, as the functions are dynamically loaded and these capabilities are registered at runtime. Best regards Christoph > -Original Message- > From: Volker Simonis [mailto:volker.simo...@gmail.com] > Sent: Freitag, 23. Dezember 2016 11:46 > To: Langer, Christoph > Cc: core-libs-dev@openjdk.java.net; nio-...@openjdk.java.net; ppc-aix-port- > d...@openjdk.java.net; Brian Burkhalter ; > Roger Riggs ; Lindenmaier, Goetz > > Subject: Re: RFR(S): 8171906: Changes for 8148023 break AIX build > > On Thu, Dec 22, 2016 at 4:03 PM, Langer, Christoph > wrote: > > Hi, > > > > > > > > after the fix for 8148023 was pushed, the AIX build is broken. Please review > > a fix for this: > > > > > > > > Bug: https://bugs.openjdk.java.net/browse/JDK-8171906 > > > > Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8171906.0/ > > > > > > > > The build breaks because NAME_MAX is not defined on AIX (just like Solaris, > > I guess) and we have to define it in UnixFileSystem_md.c. > > > > > > > > When working on this fix, I spotted some compiler warnings indicating that > > if one is working with the APIs readdir64 and friends on AIX, one has to use > > the type DIR64 instead of DIR and to open/close with opendir64/closedir64. I > > guess that’s specific to AIX, see IBM’s documentation [1]. > > > > Furthermore, although fdopendir64 is not mentioned there but just fdopendir, > > both APIs exist from AIX 7.1 onwards (look at on an AIX 7.1). So > > I added some defines for AIX to redirect the API calls. > > > > Not sure if we've already agreed to only support AIX 7.1 and higher with Java > 9. > If yes, the change is fine, otherwise we may check the result of > dlsym(RTLD_DEFAULT, "fdopendir64") and fallback to dlsym(RTLD_DEFAULT, > "fdopendir") if the former is NULL. > > Regards, > Volker > > > > > > > Thanks and best regards > > > > Christoph > > > > > > > > [1] > > > http://www.ibm.com/support/knowledgecenter/ssw_aix_71/com.ibm.aix.baset > rf1/opendir.htm > > > >
RE: RFR: 8169631: [JAXP] XALAN: transformation of XML via namespace-unaware SAX input yields a different result than namespace-unaware DOM input
Hi Joe, I finally got back to this one and pushed the change with some minor whitespace diffs. Thanks Christoph > -Original Message- > From: Joe Wang [mailto:huizhe.w...@oracle.com] > Sent: Montag, 28. November 2016 20:06 > To: Langer, Christoph > Cc: core-libs-dev@openjdk.java.net > Subject: Re: RFR: 8169631: [JAXP] XALAN: transformation of XML via > namespace-unaware SAX input yields a different result than namespace- > unaware DOM input > > Hi Christoph, > > The changes look good. I also run the other tests (smoke test and etc.), > and they all passed. > > Best, > Joe > > On 11/25/16, 4:25 AM, Langer, Christoph wrote: > > Hi Joe, > > > > when trying to finish this up, I had a closer look again. > > > > I found out that it's necessary to look at attributes as well and handle > > the case > where namespace prefixes are used. > > > > Here is a new version of the changeset that passes all jaxp jtreg tests: > > http://cr.openjdk.java.net/~clanger/webrevs/8169631.1/ > > > > Now I would also throw out prefixes in localName if we don't have > namespace support - for both, elements and attributes (in > SAX2DTM2.startElement()). > > > > I also removed some qname handling in DOM2SAX as it is not needed with > my changes to SAX2DTM2 anymore. > > > > The test case was adopted. > > > > Thanks& best regards > > Christoph > > > >> -Original Message- > >> From: Joe Wang [mailto:huizhe.w...@oracle.com] > >> Sent: Dienstag, 22. November 2016 20:11 > >> To: Langer, Christoph > >> Cc: core-libs-dev@openjdk.java.net > >> Subject: Re: RFR: 8169631: [JAXP] XALAN: transformation of XML via > >> namespace-unaware SAX input yields a different result than namespace- > >> unaware DOM input > >> > >> Hi Christoph, > >> > >> Once you're able to run all tests, feel free to push the changeset. > >> Frank has fixed the Smoke test. > >> > >> Thanks, > >> Joe > >> > >> On 11/18/16, 3:37 PM, Joe Wang wrote: > >>> Hi Christoph, > >>> > >>> Thanks for explaining the customer's dilemma with regard to their > >>> legacy process. > >>> > >>> The testcase I sent you was extracted from an internal SQE smoke test. > >>> I agree with your analysis, the 'golden' file which has been in there > >>> for over 10 years turns out to be wrong and needs to be updated. > >>> > >>> To fix this issue, we need to get that test fixed, and the check-in of > >>> your patch and that of the test need needs to happen simultaneously. > >>> Would you mind wanting for me to go through an internal process to get > >>> a patch ready, then we can check in almost at the same time? > >>> > >>> Best, > >>> Joe > >>> > >>> On 11/18/16, 2:51 PM, Langer, Christoph wrote: > >>>> Hi Joe, > >>>> > >>>> thanks for the feedback. > >>>> > >>>> I've now understood the testcase that you've sent over and the reason > >>>> that it is reporting failure after my fix is that the output of its > >>>> transform operation is rather correct now. And before it was wrong. :) > >>>> The test is comparing the actual result to a "golden" result file in > >>>> the end and both of these were not looking healthy so far. The reason > >>>> is that your test is using a namespace unaware SAX Parser as input. > >>>> With the current JDK XALAN, you could already modify your smoketest > >>>> to use a namespace aware parser. > >>>> > >>>> E.g. replace lines > >>>> > >>>> 82// Use the JAXP way to get an XMLReader > >>>> 83XMLReader reader = > >>>> SAXParserFactory.newInstance().newSAXParser().getXMLReader(); > >>>> > >>>> with > >>>> > >>>> 82// Use the JAXP way to get an XMLReader > >>>> 83SAXParserFactory spf = SAXParserFactory.newInstance(); > >>>> 84spf.setNamespaceAware(true); > >>>> 85XMLReader reader = spf.newSAXParser().getXMLReader(); > >>>> > >>>> ...and you would already get correct results that also DOM input or > >>>> Stream Input would yield. > >>>> > >>>> So, are there oth
RFR 8172974 [JAXP] XALAN: Wrong result when transforming namespace unaware StAX Input
Hi, please review a change for JAXP. Bug: https://bugs.openjdk.java.net/browse/JDK-8172974 Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8172974.0/ When enhancing the test for https://bugs.openjdk.java.net/browse/JDK-8023653, I saw that there's still an issue with StAXInputSource which is not namespace aware. This needs a small update in src/java.xml/share/classes/com/sun/org/apache/xml/internal/dtm/ref/sax2dtm/SAX2DTM2.java. Furthermore, I added the fixed warnings and formattings from the proposal http://cr.openjdk.java.net/~clanger/webrevs/8023653.0/ to this webrev, as 8023653 is an enhancement and might not go in in the near future. I also enhanced the TransformerTest to utilize data providers now and test a comprehensive matrix of XALAN input. Thanks in advance and Best regards Christoph
RFR: 8023653: xalan inconsistently parses DOMSource and StreamSource
Hi, I have created a new webrev for this enhancement: http://cr.openjdk.java.net/~clanger/webrevs/8023653.1/ As it bases on the change proposal for the new bug 8172974 [1], it is much smaller and really only contains the modifications needed for honoring the feature "namespace-aware" on TransformerFactory. As to your comments, Joe: I can't just use JdkXmlUtils.setXMLReaderPropertyIfSupport, as this one is a feature. So I added a JdkXmlUtils.setXMLReaderFeatureIfSupport. Is that what you want? The test was already improved in the webrev for 8172974 [2], I just add two more scenarios for StreamSources on Transformers with "namespace-aware == false". I would certainly also need help from someone in Oracle to run me through the ccc process, as this probably is a behavioral change for TransformerFactory. Thanks & Best regards Christoph [1] https://bugs.openjdk.java.net/browse/JDK-8172974 [2] http://cr.openjdk.java.net/~clanger/webrevs/8172974.0/ > -Original Message- > From: Joe Wang [mailto:huizhe.w...@oracle.com] > Sent: Samstag, 19. November 2016 01:50 > To: Langer, Christoph > Cc: core-libs-dev@openjdk.java.net > Subject: Re: RFR: 8023653: xalan inconsistently parses DOMSource and > StreamSource > > Hi Christoph, > > XMLReaderManager, 175 - 186 can be replace with: > JdkXmlUtils.setXMLReaderPropertyIfSupport(reader, > NAMESPACES_FEATURE, m_NamespaceAware, false); > JdkXmlUtils.setXMLReaderPropertyIfSupport(reader, > NAMESPACE_PREFIXES_FEATURE, !m_NamespaceAware, false); > > > The new test can be improved: > Test all 4 source types > Use a dataprovider (with data fields: Source, Transformer or > NamespaceAware, Expected String, etc) > Test also Templates to verify the namespace feature is propagated > properly > > Thanks, > Joe > > On 11/17/16, 9:14 PM, Joe Wang wrote: > > Together with 8169631, I think we need to understand your customer's > > requirement before spending time on a change that would otherwise be > > unnecesary. Why would turning off namespace-aware help him? As far as > > the xml standards and parser/processor are concerned, it's a backward > > development. A namespace-aware parser is compatible with the ones that > > don't know about namespaces. The norm then is to always turn it on > > (although the API had to default to off because of the legacy). Is the > > user processing some legacy xml code? > > > > Thanks, > > Joe > > > > On 11/16/16, 5:41 AM, Langer, Christoph wrote: > >> Hi, > >> > >> to address the long standing bug 8023653 I propose the following change: > >> > >> webrev: http://cr.openjdk.java.net/~clanger/webrevs/8023653.0/ > >> bug: https://bugs.openjdk.java.net/browse/JDK-8023653 > >> > >> I think the bug report in general complains that the default for > >> DOMSource input is to parse the input "namespace-unaware" whereas, if > >> XALAN gets a StreamSource as input, it is parsed "namespace-aware". > >> > >> Generally I'm against changing either of these defaults (at this time > >> at least) - XALAN StreamSource should be handled namespace aware and > >> changing the default for DocumentBuilderFactory is maybe an option > >> for the Java 10 timeframe, I'd say. However, at the moment it is not > >> possible at all to configure XALAN's handling of StreamSource input > >> to be namespace unaware. That's what I'd like to address with my > >> change. With my modifications it's possible to set the namespace > >> feature on the TransformerFactory (to false). It bases on my other > >> open fix 8169631 which is discussed here: > >> http://mail.openjdk.java.net/pipermail/core-libs-dev/2016- > November/044662.html > >> Only with the corrections for that fix, the SAX parsing that is used > >> for stream sources will be handled correctly. > >> > >> The proposed change from the bugreport (change options for the parser > >> of the XSL input before compilation) would not help with the reported > >> issue and also is not a good idea anyway as xsl input needs to be > >> handled with namespaces obviously. > >> > >> I did other cleanups in my change as well, the real functional > >> changes are in TransformerFactoryImpl.java, XSLTCDTMManager.java and > >> XMLReaderManager.java. > >> > >> Thanks in advance for reviewing. > >> > >> Best regards > >> Christoph > >>
RE: RFR 8172974 [JAXP] XALAN: Wrong result when transforming namespace unaware StAX Input
Hi Joe, generally, you are right, XSLT requires namespace support. For parsing the stylesheet, it is definitely a hard requirement. Otherwise this would not make sense at all. For instance xsl directives are in the xsl namespace. That requirement is what the FAQ you are referencing [1] is talking about. As for the InputSource which is to be processed, it is probably also not a right to use non namespace aware parsing. But still it's not forbidden. For instance, the JavaDoc for SAXSource [2] states this: "Note that XSLT requires namespace support. Attempting to transform an input source that is not generated with a namespace-aware parser may result in errors. Parsers can be made namespace aware by calling the SAXParserFactory.setNamespaceAware(boolean awareness) method." So, I agree, we are in the error space here. But still I think the result of non namespace aware parsing should be the same for all types of input source. And at the moment it is the same for DOMSource and SAXSource but not for StAXSource. From that point of view I think my fix makes sense (along with the other cleanups). Best regards Christoph [1] https://xml.apache.org/xalan-j/faq.html#faq-N10207 [2] https://docs.oracle.com/javase/8/docs/api/javax/xml/transform/sax/SAXSource.html > -Original Message- > From: huizhe wang [mailto:huizhe.w...@oracle.com] > Sent: Mittwoch, 18. Januar 2017 22:12 > To: Langer, Christoph ; core-libs- > d...@openjdk.java.net > Subject: Re: RFR 8172974 [JAXP] XALAN: Wrong result when transforming > namespace unaware StAX Input > > Hi Christoph, > > Xalan requires the underlying parser to be namespace aware. Please refer > to https://xml.apache.org/xalan-j/faq.html#faq-N10207 > > Thanks, > Joe > > On 1/18/2017 8:26 AM, Langer, Christoph wrote: > > Hi, > > > > please review a change for JAXP. > > > > Bug: https://bugs.openjdk.java.net/browse/JDK-8172974 > > Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8172974.0/ > > > > When enhancing the test for https://bugs.openjdk.java.net/browse/JDK- > 8023653, I saw that there's still an issue with StAXInputSource which is not > namespace aware. This needs a small update in > src/java.xml/share/classes/com/sun/org/apache/xml/internal/dtm/ref/sax2dt > m/SAX2DTM2.java. Furthermore, I added the fixed warnings and formattings > from the proposal http://cr.openjdk.java.net/~clanger/webrevs/8023653.0/ to > this webrev, as 8023653 is an enhancement and might not go in in the near > future. > > > > I also enhanced the TransformerTest to utilize data providers now and test a > comprehensive matrix of XALAN input. > > > > Thanks in advance and Best regards > > Christoph > >
RFR (JAXP) 8169827: javax/xml/jaxp/isolatedjdk/catalog/PropertiesTest.sh copied JDK failed
Hi, while working on jaxp changes and running jtreg tests I found that test javax/xml/jaxp/isolatedjdk/catalog/PropertiesTest.sh does not work. I then saw that this was already reported with bug 8169827. But, as I had already spent some time to fix this test I'd like to contribute my fix: Bug: https://bugs.openjdk.java.net/browse/JDK-8169827 Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8169827.0/ I converted the test to Java and removed the shell script PropertiesTest.sh. This resolves the compilation issue. However, the test needs to copy an isolated jdk as it modifies files in the JDK. So I'm still using the copy script to first copy the jdk and afterwards remove the copy. These are separate 'shell' test steps. And in the actual test I'm running a child process with the isolated JDK. I also don't know if the test should be kept in the problem list and/or also be tagged as 'intermittent' as the whole jdk copying procedure by means of a shell script seems error prone. In case we keep the entry in the problem list, I can also open a separate bug for my change. @Frank: I don't know if you have some larger change in mind which improves the isolated jdk type testing greatly, otherwise I think this fix could at least make things better than they are at the moment. Thanks & Best regards Christoph
RE: RFR (JAXP) 8169827: javax/xml/jaxp/isolatedjdk/catalog/PropertiesTest.sh copied JDK failed
Hi Daniel, thanks for checking/reviewing. So I'll submit with removing the ProblemList.txt entry and I'll also remove the intermittent flag. Sounds fair to check later if problems will still show up. Although I have the feeling that the issue of https://bugs.openjdk.java.net/browse/JDK-8147431 might appear again... Best regards Christoph > -Original Message- > From: Daniel Fuchs [mailto:daniel.fu...@oracle.com] > Sent: Montag, 23. Januar 2017 17:12 > To: Langer, Christoph ; Frank Yuan > ; core-libs-dev@openjdk.java.net > Subject: Re: RFR (JAXP) 8169827: > javax/xml/jaxp/isolatedjdk/catalog/PropertiesTest.sh copied JDK failed > > Hi Christoph, > > Thanks for fixing this test! > > I imported your patch, modified ProblemList.txt to not skip the test, > and sent it through our test system, and I'm happy to report that > the test was run on all available platforms with no failure. > > So I think you should simply remove the line from ProblemList.txt > (no need for a new webrev). > If it fails again on more exotic platform we'll simply add it > back to ProblemList.txt for those platforms where it fails > (I guess it could happen if there's not enough disk space). > > Otherwise I have looked over the changes you proposed and they > do seem OK to me. > > +1 > > best regards, > > -- daniel > > On 23/01/17 10:03, Langer, Christoph wrote: > > Hi, > > > > while working on jaxp changes and running jtreg tests I found that test > javax/xml/jaxp/isolatedjdk/catalog/PropertiesTest.sh does not work. I then saw > that this was already reported with bug 8169827. But, as I had already spent > some time to fix this test I'd like to contribute my fix: > > > > Bug: https://bugs.openjdk.java.net/browse/JDK-8169827 > > Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8169827.0/ > > > > I converted the test to Java and removed the shell script PropertiesTest.sh. > This resolves the compilation issue. > > > > However, the test needs to copy an isolated jdk as it modifies files in the > > JDK. > So I'm still using the copy script to first copy the jdk and afterwards > remove the > copy. These are separate 'shell' test steps. And in the actual test I'm > running a > child process with the isolated JDK. > > > > I also don't know if the test should be kept in the problem list and/or > > also be > tagged as 'intermittent' as the whole jdk copying procedure by means of a > shell > script seems error prone. In case we keep the entry in the problem list, I can > also open a separate bug for my change. > > > > @Frank: I don't know if you have some larger change in mind which improves > the isolated jdk type testing greatly, otherwise I think this fix could at > least > make things better than they are at the moment. > > > > Thanks & Best regards > > Christoph > >
RE: (JAXP) RFR: 8173111: Excessive recursion in EventFilterSupport when filtering over large number of XML events can cause StackOverflow
Hi Daniel,
your fix looks good, pretty straightforward, +1.
A minor thing,
src/java.xml/share/classes/com/sun/xml/internal/stream/EventFilterSupport.java:
67 while (super.hasNext()){
As you are touching this line, you could add a blank between ')' and '{'.
Disclaimer: I'm no reviewer.
Best regards
Christoph
> -Original Message-
> From: core-libs-dev [mailto:core-libs-dev-boun...@openjdk.java.net] On Behalf
> Of Daniel Fuchs
> Sent: Montag, 23. Januar 2017 18:48
> To: core-libs-dev
> Subject: (JAXP) RFR: 8173111: Excessive recursion in EventFilterSupport when
> filtering over large number of XML events can cause StackOverflow
>
> Hi,
>
> Please find below a fix for:
>
> 8173111: Excessive recursion in EventFilterSupport when filtering
> over large number of XML events can cause StackOverflow
> https://bugs.openjdk.java.net/browse/JDK-8173111
>
> The fix is almost trivial: it replaces a recursion by a loop in
> two places.
>
> http://cr.openjdk.java.net/~dfuchs/webrev_8173111/webrev.00/
>
> The test will fail without the fix (at least on my machine)
> and pass with it (on my machine and in our test system).
>
> best regards,
>
> -- daniel
RE: RFR (JAXP) 8169827: javax/xml/jaxp/isolatedjdk/catalog/PropertiesTest.sh copied JDK failed
Hi Frank, thanks for your comments. I already thought it might be better to create a new bug for my repair work - as 8169827 actually is about the intermittent failure which will perhaps not be solved with my changes. So I'll create a new item and submit my changes for that one. Furthermore, I would also support creating Java code to do the copy work instead of using the shell script, though doing this is rather out of scope for me, at the moment. This should be done in a common library, e.g. /test/javax/xml/jaxp/libs/jaxp/library. I also see that the same requirement exists for tests in the jdk repo - unfortunately there is no shared testing library with 'jdk'. So we'll probably end up with some duplicated code :( By the way: In the jaxp test tree, there exists a lib /test/javax/xml/jaxp/libs/jdk/testlibrary out of which at least some stuff seems to be obsolete for jaxp testing and/or significantly behind jdk's test/lib/testlibrary. I guess here's some room for cleanup/modernization. As for the idea with the links - it sounds interesting, but it also adds more complexity. And maybe it is not such a good idea for Windows systems... Best regards Christoph > -Original Message- > From: Frank Yuan [mailto:frank.y...@oracle.com] > Sent: Dienstag, 24. Januar 2017 08:59 > To: Langer, Christoph ; 'Daniel Fuchs' > ; core-libs-dev@openjdk.java.net > Subject: RE: RFR (JAXP) 8169827: > javax/xml/jaxp/isolatedjdk/catalog/PropertiesTest.sh copied JDK failed > > Hi Christoph > > Many thanks for your effort! > > It's really better than the old version! However I have 2 comments although I > am not a reviewer(as you often stated :)) > 1. we could also write java code to copy/delete JDK. > 2. 8169827 is to track PropertiesTest failed in copying JDK intermittently. I > suggest to use another bug for your improvement > because I am not sure if this patch can improve the issue of 8169827. > > To Christoph and Daniel > > Btw, Christoph's patch inspired me. You know, PropertiesTest copies a JDK in > order to isolate the file change of the JDK, actually > there are some other similar tests(to test some other JDK property or config > files) in JDK repo, they take the same way as well as > have similar code... > I have another idea for this test, that is we can only make symbolic link to > the > JDK files/directories except conf directory, create > a directory named with "conf" under the new jdk directory, and then make > symbolic link to the files/directories in conf to the real > jdk/conf except jaxp.properties. This will reduce the most of copying work and > may reduce the risk of copying work. What's your > opinion? > > Thanks > Frank > > > -Original Message- > > From: Langer, Christoph [mailto:christoph.lan...@sap.com] > > To: Daniel Fuchs; Frank Yuan; core-libs-dev@openjdk.java.net > > Subject: RE: RFR (JAXP) 8169827: > javax/xml/jaxp/isolatedjdk/catalog/PropertiesTest.sh copied JDK failed > > > > Hi Daniel, > > > > thanks for checking/reviewing. So I'll submit with removing the > ProblemList.txt entry and I'll also remove the intermittent flag. > > > > Sounds fair to check later if problems will still show up. Although I have > > the > feeling that the issue of > > https://bugs.openjdk.java.net/browse/JDK-8147431 might appear again... > > > > Best regards > > Christoph > > > > > -Original Message- > > > From: Daniel Fuchs [mailto:daniel.fu...@oracle.com] > > > Sent: Montag, 23. Januar 2017 17:12 > > > To: Langer, Christoph ; Frank Yuan > > > ; core-libs-dev@openjdk.java.net > > > Subject: Re: RFR (JAXP) 8169827: > > > javax/xml/jaxp/isolatedjdk/catalog/PropertiesTest.sh copied JDK failed > > > > > > Hi Christoph, > > > > > > Thanks for fixing this test! > > > > > > I imported your patch, modified ProblemList.txt to not skip the test, > > > and sent it through our test system, and I'm happy to report that > > > the test was run on all available platforms with no failure. > > > > > > So I think you should simply remove the line from ProblemList.txt > > > (no need for a new webrev). > > > If it fails again on more exotic platform we'll simply add it > > > back to ProblemList.txt for those platforms where it fails > > > (I guess it could happen if there's not enough disk space). > > > > > > Otherwise I have looked over the changes you proposed and they > > > do seem OK to me. > > > > > > +1 > > > > > > best r
RE: RFR (JAXP) 8169827: javax/xml/jaxp/isolatedjdk/catalog/PropertiesTest.sh copied JDK failed
Hi, I submitted my fix under Bug https://bugs.openjdk.java.net/browse/JDK-8173261: http://hg.openjdk.java.net/jdk9/dev/jaxp/rev/620d0c38581f I also added a comment to https://bugs.openjdk.java.net/browse/JDK-8169827 Best regards Christoph > -Original Message- > From: Frank Yuan [mailto:frank.y...@oracle.com] > Sent: Dienstag, 24. Januar 2017 09:45 > To: Langer, Christoph > Cc: 'Daniel Fuchs' ; core-libs-dev@openjdk.java.net > Subject: RE: RFR (JAXP) 8169827: > javax/xml/jaxp/isolatedjdk/catalog/PropertiesTest.sh copied JDK failed > > > From: Langer, Christoph [mailto:christoph.lan...@sap.com] > > To: Frank Yuan > > Cc: 'Daniel Fuchs'; core-libs-dev@openjdk.java.net > > Subject: RE: RFR (JAXP) 8169827: > javax/xml/jaxp/isolatedjdk/catalog/PropertiesTest.sh copied JDK failed > > > > Hi Frank, > > > > thanks for your comments. > > > > I already thought it might be better to create a new bug for my repair work > > - > as 8169827 actually is about the intermittent > failure which will > > perhaps not be solved with my changes. So I'll create a new item and submit > my changes for that one. > > > > Furthermore, I would also support creating Java code to do the copy work > instead of using the shell script, though doing this is > rather out of > > scope for me, at the moment. > > Just a suggestion, please feel free to leave it:) > > >This should be done in a common library, e.g. > /test/javax/xml/jaxp/libs/jaxp/library. I also see that the same > > requirement exists for tests in the jdk repo - unfortunately there is no > > shared > testing library with 'jdk'. So we'll probably end > up with some > > duplicated code :( > > > > By the way: In the jaxp test tree, there exists a lib > /test/javax/xml/jaxp/libs/jdk/testlibrary out of which at least some stuff > seems to be > > obsolete for jaxp testing and/or significantly behind jdk's > > test/lib/testlibrary. I > guess here's some room for > cleanup/modernization. > > > > Yes, testlibrary is a copy from jdk repo, there are a few JAXP tests using the > library, so we didn't keep it synced with JDK. In my > mind, we don't need spend too much effort on it unless it impacts JAXP tests. > > > As for the idea with the links - it sounds interesting, but it also adds > > more > complexity. And maybe it is not such a good idea for > Windows > > systems... > > I didn't test it in Windows, please leave it to me. I will update to you and > Joe > when I have the result. Thanks > > Frank > > > > > Best regards > > Christoph > > > > > -Original Message- > > > From: Frank Yuan [mailto:frank.y...@oracle.com] > > > Sent: Dienstag, 24. Januar 2017 08:59 > > > To: Langer, Christoph ; 'Daniel Fuchs' > > > ; core-libs-dev@openjdk.java.net > > > Subject: RE: RFR (JAXP) 8169827: > > > javax/xml/jaxp/isolatedjdk/catalog/PropertiesTest.sh copied JDK failed > > > > > > Hi Christoph > > > > > > Many thanks for your effort! > > > > > > It's really better than the old version! However I have 2 comments > > > although > I > > > am not a reviewer(as you often stated :)) > > > 1. we could also write java code to copy/delete JDK. > > > 2. 8169827 is to track PropertiesTest failed in copying JDK > > > intermittently. I > > > suggest to use another bug for your improvement > > > because I am not sure if this patch can improve the issue of 8169827. > > > > > > To Christoph and Daniel > > > > > > Btw, Christoph's patch inspired me. You know, PropertiesTest copies a JDK > in > > > order to isolate the file change of the JDK, actually > > > there are some other similar tests(to test some other JDK property or > > > config > > > files) in JDK repo, they take the same way as well as > > > have similar code... > > > I have another idea for this test, that is we can only make symbolic link > > > to > the > > > JDK files/directories except conf directory, create > > > a directory named with "conf" under the new jdk directory, and then make > > > symbolic link to the files/directories in conf to the real > > > jdk/conf except jaxp.properties. This will reduce the most of copying work > and > > > may reduce the risk of copying work. What's your > > > opinion? > > > > > > Thanks > > > Frank > > > > >
[JAXP] RFR: 8173602: TESTBUG: javax/xml/jaxp/unittest/transform/TransformerTest.java needs refactoring
Hi, please review a test fix for javax/xml/jaxp/unittest/transform/TransformerTest.java: Bug: https://bugs.openjdk.java.net/browse/JDK-8173602 Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8173602.0/ The main motivation for this refactoring is to remove the dependency to the JDK internal classes com.sun.org.apache.xml.internal.serialize.OutputFormat and com.sun.org.apache.xml.internal.serialize.XMLSerializer. Generally, TransformerTest.java contains several subtests which do similar things (test XALAN transformations) but the implementations differ. I created a common template with helper methods which all tests take advantage of now. I also introduced data sources to some tests to make them more readable and adaptable. Thanks Christoph
RFR (XS): 8173634: libjli/cmdtoargs.c does not compile with VS2010
Hi, please review a tiny fix. The change for JDK-8170832 (Add a new launcher environment variable JAVA_OPTIONS) has introduced a build error when you build with Visual Studio 2010. Yes, we know it is not officially supported for JDK9 - however, it's easy and clean to keep compatibility. :) Bug: https://bugs.openjdk.java.net/browse/JDK-8173634 Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8173634.0/ Thanks Christoph
RE: RFR (XS): 8173634: libjli/cmdtoargs.c does not compile with VS2010
Thanks, Roger, Chris. It's submitted: http://hg.openjdk.java.net/jdk9/dev/jdk/rev/65a1441c9c80 Best Christoph > -Original Message- > From: Chris Hegarty [mailto:chris.hega...@oracle.com] > Sent: Montag, 30. Januar 2017 16:33 > To: Langer, Christoph ; core-libs- > d...@openjdk.java.net > Subject: Re: RFR (XS): 8173634: libjli/cmdtoargs.c does not compile with > VS2010 > > ++1 > > I ran into this myself earlier today. > > -Chris. > > On 30/01/17 14:59, Roger Riggs wrote: > > Hi Christoph, > > > > Looks fine, > > > > Thanks, Roger > > > > > > On 1/30/2017 9:52 AM, Langer, Christoph wrote: > >> Hi, > >> > >> please review a tiny fix. The change for JDK-8170832 (Add a new > >> launcher environment variable JAVA_OPTIONS) has introduced a build > >> error when you build with Visual Studio 2010. > >> > >> Yes, we know it is not officially supported for JDK9 - however, it's > >> easy and clean to keep compatibility. :) > >> > >> Bug: https://bugs.openjdk.java.net/browse/JDK-8173634 > >> Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8173634.0/ > >> > >> Thanks > >> Christoph > >> > >
Update: RFR 8172974 [JAXP] XALAN: Wrong result when transforming namespace unaware StAX Input
Hi Joe, I've updated my webrev for 8172974 after pulling out the refactoring of javax/xml/jaxp/unittest/transform/TransformerTest.java to Bug 8173602: http://cr.openjdk.java.net/~clanger/webrevs/8172974.1/ After exchanging some off-list mails with Lance and Daniel, we came to the conclusion that you should decide about this one when you are back. The changes are merely cleanups apart from com/sun/org/apache/xml/internal/dtm/ref/sax2dtm/SAX2DTM2.startElement(), which to me seem natural after 8169631, though. Thanks, Christoph > -Original Message- > From: Langer, Christoph > Sent: Mittwoch, 18. Januar 2017 22:55 > To: 'huizhe wang' ; core-libs-dev@openjdk.java.net > Subject: RE: RFR 8172974 [JAXP] XALAN: Wrong result when transforming > namespace unaware StAX Input > > Hi Joe, > > generally, you are right, XSLT requires namespace support. For parsing the > stylesheet, it is definitely a hard requirement. Otherwise this would not make > sense at all. For instance xsl directives are in the xsl namespace. That > requirement is what the FAQ you are referencing [1] is talking about. > > As for the InputSource which is to be processed, it is probably also not a > right to > use non namespace aware parsing. But still it's not forbidden. For instance, > the > JavaDoc for SAXSource [2] states this: > "Note that XSLT requires namespace support. Attempting to transform an input > source that is not generated with a namespace-aware parser may result in > errors. Parsers can be made namespace aware by calling the > SAXParserFactory.setNamespaceAware(boolean awareness) method." > > So, I agree, we are in the error space here. But still I think the result of > non > namespace aware parsing should be the same for all types of input source. And > at the moment it is the same for DOMSource and SAXSource but not for > StAXSource. From that point of view I think my fix makes sense (along with the > other cleanups). > > Best regards > Christoph > > [1] https://xml.apache.org/xalan-j/faq.html#faq-N10207 > [2] > https://docs.oracle.com/javase/8/docs/api/javax/xml/transform/sax/SAXSource > .html > > > > -Original Message- > > From: huizhe wang [mailto:huizhe.w...@oracle.com] > > Sent: Mittwoch, 18. Januar 2017 22:12 > > To: Langer, Christoph ; core-libs- > > d...@openjdk.java.net > > Subject: Re: RFR 8172974 [JAXP] XALAN: Wrong result when transforming > > namespace unaware StAX Input > > > > Hi Christoph, > > > > Xalan requires the underlying parser to be namespace aware. Please refer > > to https://xml.apache.org/xalan-j/faq.html#faq-N10207 > > > > Thanks, > > Joe > > > > On 1/18/2017 8:26 AM, Langer, Christoph wrote: > > > Hi, > > > > > > please review a change for JAXP. > > > > > > Bug: https://bugs.openjdk.java.net/browse/JDK-8172974 > > > Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8172974.0/ > > > > > > When enhancing the test for https://bugs.openjdk.java.net/browse/JDK- > > 8023653, I saw that there's still an issue with StAXInputSource which is not > > namespace aware. This needs a small update in > > > src/java.xml/share/classes/com/sun/org/apache/xml/internal/dtm/ref/sax2dt > > m/SAX2DTM2.java. Furthermore, I added the fixed warnings and formattings > > from the proposal http://cr.openjdk.java.net/~clanger/webrevs/8023653.0/ to > > this webrev, as 8023653 is an enhancement and might not go in in the near > > future. > > > > > > I also enhanced the TransformerTest to utilize data providers now and test > a > > comprehensive matrix of XALAN input. > > > > > > Thanks in advance and Best regards > > > Christoph > > >
Updated Webrev: [JAXP] RFR: 8173602: TESTBUG: javax/xml/jaxp/unittest/transform/TransformerTest.java needs refactoring
Hi, I made a small update to the webrev (in place). I externalized class TransformerTestTemplate now to be able to use it in other tests besides TransformerTest.java. Best regards Christoph From: Langer, Christoph Sent: Montag, 30. Januar 2017 15:28 To: core-libs-dev@openjdk.java.net Subject: [JAXP] RFR: 8173602: TESTBUG: javax/xml/jaxp/unittest/transform/TransformerTest.java needs refactoring Hi, please review a test fix for javax/xml/jaxp/unittest/transform/TransformerTest.java: Bug: https://bugs.openjdk.java.net/browse/JDK-8173602 Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8173602.0/ The main motivation for this refactoring is to remove the dependency to the JDK internal classes com.sun.org.apache.xml.internal.serialize.OutputFormat and com.sun.org.apache.xml.internal.serialize.XMLSerializer. Generally, TransformerTest.java contains several subtests which do similar things (test XALAN transformations) but the implementations differ. I created a common template with helper methods which all tests take advantage of now. I also introduced data sources to some tests to make them more readable and adaptable. Thanks Christoph
RFR - Update: 8168664 [JAXP] XALAN: xsl:element generates namespace prefixes if namespace is given as URI only
Hi Joe, evenutally I have updated my webrev for this issue: http://cr.openjdk.java.net/~clanger/webrevs/8168664.1/ I've implemented your suggestion to add a property "jdk.xml.generatePrefix" which is true by default. I also added a testcase which tests the correct behavior with "-Djdk.xml.generatePrefix=false". Generally, I think namespace prefix generation at this place is not needed. Or would you know of or be able to construct a case where it is really required? If not, I rather think namespace generation at this place should be thrown out unconditionally. Maybe for JDK10? In case it is removed, it would render the test javax/xml/jaxp/unittest/transform/NamespacePrefixTest.java which was implemented recently by Alex for bug https://bugs.openjdk.java.net/browse/JDK-8167179 obsolete in its current form. However, the namespace generation function is still used some place for attribute namespaces. For JDK9 I can imagine the change of this webrev going in - however, as we already discussed, you would have to do a ccc for me. Thanks & Best regards Christoph From: Langer, Christoph Sent: Dienstag, 25. Oktober 2016 15:39 To: core-libs-dev@openjdk.java.net Subject: RFR (JAXP): 8168664 [JAXP] XALAN: xsl:element generates namespace prefixes if namespace is given as URI only Hi JAXP experts, please review a fix for a new issue regarding namespace handling in Xalan with xsl:element. Bug: https://bugs.openjdk.java.net/browse/JDK-8168664 Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8168664.0/ I'm not 100% sure if this is the right way to go or if it would break some correct behavior elsewhere. But I think the current behavior is either not correct or at least it is not required to generate new xsl namespace prefixes if the namespace comes in as URI only to an xsl:element node. The interpretative transformer from the Apache Xalan project will also produce the expected output, different to the compiled XSLTC here. In the webrev, my changes to XslElement.java are only cosmetical/comments, the behavior does not change. In BasisLibrary.java I also took the opportunity to remove the $Id tag and the unused method "nodeList2IteratorUsingHandleFromNode". If you think my change is good, I'll also add a test that runs the transformation which can be found in the bug... Thanks & best regards Christoph
Ping: [JAXP] RFR: 8173602: TESTBUG: javax/xml/jaxp/unittest/transform/TransformerTest.java needs refactoring
Hi, Ping - anyone out there who could have a look at my JAXP test update? Thanks Christoph From: Langer, Christoph Sent: Montag, 30. Januar 2017 15:28 To: core-libs-dev@openjdk.java.net Subject: [JAXP] RFR: 8173602: TESTBUG: javax/xml/jaxp/unittest/transform/TransformerTest.java needs refactoring Hi, please review a test fix for javax/xml/jaxp/unittest/transform/TransformerTest.java: Bug: https://bugs.openjdk.java.net/browse/JDK-8173602 Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8173602.0/ The main motivation for this refactoring is to remove the dependency to the JDK internal classes com.sun.org.apache.xml.internal.serialize.OutputFormat and com.sun.org.apache.xml.internal.serialize.XMLSerializer. Generally, TransformerTest.java contains several subtests which do similar things (test XALAN transformations) but the implementations differ. I created a common template with helper methods which all tests take advantage of now. I also introduced data sources to some tests to make them more readable and adaptable. Thanks Christoph
RE: Ping: [JAXP] RFR: 8173602: TESTBUG: javax/xml/jaxp/unittest/transform/TransformerTest.java needs refactoring
Hi Daniel, thanks for taking care of this. As for the pretty printing: That's not something which is explicitly tested here. I think it was me who introduced this for new tests I had added. I found it quite useful when analyzing test results. So now with the public APIs I guess this code is more independent and portable to other Java versions. I was only looking at JDK9 with that so far. My understanding was that pushing test fixes is still ok. Let me know if you have objections. Best regards Christoph > -Original Message- > From: Daniel Fuchs [mailto:daniel.fu...@oracle.com] > Sent: Freitag, 3. Februar 2017 13:54 > To: Langer, Christoph ; core-libs- > d...@openjdk.java.net > Subject: Re: Ping: [JAXP] RFR: 8173602: TESTBUG: > javax/xml/jaxp/unittest/transform/TransformerTest.java needs refactoring > > Hi Christoph, > > I had a look at your patch, it seems OK. > I didn't know that testng supported @Test > annotations on methods of inner classes, but > that looks useful. > > The patch is a bit difficult to review because the diff > seems a bit lost sometime - but as far as I could see > what was there before seems to be there after too :-) > > I noticed you changed the calls for pretty-printing > the results - using public APIs now. Hopefully that > doesn't matter to test (I mean that's hopefully not > what the test was testing). > > I ran your new test locally on my machine, and sent it > through our test system as well, and everything passed, > so I guess that's a +1 for me. > > Will you push that to the new 10 forest or did you > plan to push it in 9? > > best regards, > > -- daniel > > On 03/02/17 10:11, Langer, Christoph wrote: > > Hi, > > > > Ping - anyone out there who could have a look at my JAXP test update? > > > > Thanks > > Christoph > > > > From: Langer, Christoph > > Sent: Montag, 30. Januar 2017 15:28 > > To: core-libs-dev@openjdk.java.net > > Subject: [JAXP] RFR: 8173602: TESTBUG: > javax/xml/jaxp/unittest/transform/TransformerTest.java needs refactoring > > > > Hi, > > > > please review a test fix for > javax/xml/jaxp/unittest/transform/TransformerTest.java: > > Bug: https://bugs.openjdk.java.net/browse/JDK-8173602 > > Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8173602.0/ > > > > The main motivation for this refactoring is to remove the dependency to the > JDK internal classes com.sun.org.apache.xml.internal.serialize.OutputFormat > and com.sun.org.apache.xml.internal.serialize.XMLSerializer. > > > > Generally, TransformerTest.java contains several subtests which do similar > things (test XALAN transformations) but the implementations differ. I created > a > common template with helper methods which all tests take advantage of now. I > also introduced data sources to some tests to make them more readable and > adaptable. > > > > Thanks > > Christoph > >
RE: Ping: [JAXP] RFR: 8173602: TESTBUG: javax/xml/jaxp/unittest/transform/TransformerTest.java needs refactoring
Thanks, Daniel. I pushed it: http://hg.openjdk.java.net/jdk9/dev/jaxp/rev/e930c373aaa4 > -Original Message- > From: Daniel Fuchs [mailto:daniel.fu...@oracle.com] > Sent: Freitag, 3. Februar 2017 14:56 > To: Langer, Christoph > Cc: core-libs-dev@openjdk.java.net > Subject: Re: Ping: [JAXP] RFR: 8173602: TESTBUG: > javax/xml/jaxp/unittest/transform/TransformerTest.java needs refactoring > > Hi Christoph, > > No objections. > > best regards, > > -- daniel > > On 03/02/17 13:01, Langer, Christoph wrote: > > Hi Daniel, > > > > thanks for taking care of this. > > > > As for the pretty printing: That's not something which is explicitly tested > > here. > I think it was me who introduced this for new tests I had added. I found it > quite > useful when analyzing test results. So now with the public APIs I guess this > code > is more independent and portable to other Java versions. > > > > I was only looking at JDK9 with that so far. My understanding was that > pushing test fixes is still ok. Let me know if you have objections. > > > > Best regards > > Christoph > > > >> -Original Message- > >> From: Daniel Fuchs [mailto:daniel.fu...@oracle.com] > >> Sent: Freitag, 3. Februar 2017 13:54 > >> To: Langer, Christoph ; core-libs- > >> d...@openjdk.java.net > >> Subject: Re: Ping: [JAXP] RFR: 8173602: TESTBUG: > >> javax/xml/jaxp/unittest/transform/TransformerTest.java needs refactoring > >> > >> Hi Christoph, > >> > >> I had a look at your patch, it seems OK. > >> I didn't know that testng supported @Test > >> annotations on methods of inner classes, but > >> that looks useful. > >> > >> The patch is a bit difficult to review because the diff > >> seems a bit lost sometime - but as far as I could see > >> what was there before seems to be there after too :-) > >> > >> I noticed you changed the calls for pretty-printing > >> the results - using public APIs now. Hopefully that > >> doesn't matter to test (I mean that's hopefully not > >> what the test was testing). > >> > >> I ran your new test locally on my machine, and sent it > >> through our test system as well, and everything passed, > >> so I guess that's a +1 for me. > >> > >> Will you push that to the new 10 forest or did you > >> plan to push it in 9? > >> > >> best regards, > >> > >> -- daniel > >> > >> On 03/02/17 10:11, Langer, Christoph wrote: > >>> Hi, > >>> > >>> Ping - anyone out there who could have a look at my JAXP test update? > >>> > >>> Thanks > >>> Christoph > >>> > >>> From: Langer, Christoph > >>> Sent: Montag, 30. Januar 2017 15:28 > >>> To: core-libs-dev@openjdk.java.net > >>> Subject: [JAXP] RFR: 8173602: TESTBUG: > >> javax/xml/jaxp/unittest/transform/TransformerTest.java needs refactoring > >>> > >>> Hi, > >>> > >>> please review a test fix for > >> javax/xml/jaxp/unittest/transform/TransformerTest.java: > >>> Bug: https://bugs.openjdk.java.net/browse/JDK-8173602 > >>> Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8173602.0/ > >>> > >>> The main motivation for this refactoring is to remove the dependency to > the > >> JDK internal classes com.sun.org.apache.xml.internal.serialize.OutputFormat > >> and com.sun.org.apache.xml.internal.serialize.XMLSerializer. > >>> > >>> Generally, TransformerTest.java contains several subtests which do similar > >> things (test XALAN transformations) but the implementations differ. I > created a > >> common template with helper methods which all tests take advantage of > now. I > >> also introduced data sources to some tests to make them more readable > and > >> adaptable. > >>> > >>> Thanks > >>> Christoph > >>> > >
RE: RFR - Update: 8168664 [JAXP] XALAN: xsl:element generates namespace prefixes if namespace is given as URI only
Hi Joe, welcome back. :) This sounds reasonable. I'll prepare a change for JDK10. Thanks Christoph From: Joe Wang [mailto:huizhe.w...@oracle.com] Sent: Mittwoch, 8. Februar 2017 01:39 To: Langer, Christoph Cc: core-libs-dev@openjdk.java.net; Aleksej Efimov Subject: Re: RFR - Update: 8168664 [JAXP] XALAN: xsl:element generates namespace prefixes if namespace is given as URI only Hi Christoph, As we discussed, let's target this for JDK 10 as a spec change at this stage for JDK 9 is too late. Removing the namespace generation would be another reason to do this in JDK 10 since then, we wouldn't need the additional property. Thanks, Joe On 1/31/17, 11:02 AM, Langer, Christoph wrote: Hi Joe, evenutally I have updated my webrev for this issue: http://cr.openjdk.java.net/~clanger/webrevs/8168664.1/<http://cr.openjdk.java.net/%7Eclanger/webrevs/8168664.1/> I've implemented your suggestion to add a property "jdk.xml.generatePrefix" which is true by default. I also added a testcase which tests the correct behavior with "-Djdk.xml.generatePrefix=false". Generally, I think namespace prefix generation at this place is not needed. Or would you know of or be able to construct a case where it is really required? If not, I rather think namespace generation at this place should be thrown out unconditionally. Maybe for JDK10? In case it is removed, it would render the test javax/xml/jaxp/unittest/transform/NamespacePrefixTest.java which was implemented recently by Alex for bug https://bugs.openjdk.java.net/browse/JDK-8167179 obsolete in its current form. However, the namespace generation function is still used some place for attribute namespaces. For JDK9 I can imagine the change of this webrev going in - however, as we already discussed, you would have to do a ccc for me. Thanks & Best regards Christoph From: Langer, Christoph Sent: Dienstag, 25. Oktober 2016 15:39 To: core-libs-dev@openjdk.java.net<mailto:core-libs-dev@openjdk.java.net> Subject: RFR (JAXP): 8168664 [JAXP] XALAN: xsl:element generates namespace prefixes if namespace is given as URI only Hi JAXP experts, please review a fix for a new issue regarding namespace handling in Xalan with xsl:element. Bug: https://bugs.openjdk.java.net/browse/JDK-8168664 Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8168664.0/<http://cr.openjdk.java.net/%7Eclanger/webrevs/8168664.0/> I'm not 100% sure if this is the right way to go or if it would break some correct behavior elsewhere. But I think the current behavior is either not correct or at least it is not required to generate new xsl namespace prefixes if the namespace comes in as URI only to an xsl:element node. The interpretative transformer from the Apache Xalan project will also produce the expected output, different to the compiled XSLTC here. In the webrev, my changes to XslElement.java are only cosmetical/comments, the behavior does not change. In BasisLibrary.java I also took the opportunity to remove the $Id tag and the unused method "nodeList2IteratorUsingHandleFromNode". If you think my change is good, I'll also add a test that runs the transformation which can be found in the bug... Thanks & best regards Christoph
RE: Update: RFR 8172974 [JAXP] XALAN: Wrong result when transforming namespace unaware StAX Input
Hi Joe, that's unfortunate but if these are the numbers then we have to deal with it. It's a bit odd, though, that this does not cause a regression on all platforms. So, you are really sure the regression roots in my current proposal for 8172974? One thing I'd want to have been looked at is if the regression really roots to my change in SAX2DTM2.java. This is the real modification. Everything else is just cleanup. And looking at SAX2DTM2, I really can't see where I should have introduced something bad. Can you retest this? Also, moving this to JDK10 would be fine for me. Shall I submit my fix there as is or do you want to conduct some deeper analysis about the performance regression's root cause? Can I somehow assist with that? Thanks Christoph > -Original Message- > From: huizhe wang [mailto:huizhe.w...@oracle.com] > Sent: Donnerstag, 9. Februar 2017 23:15 > To: Langer, Christoph > Cc: core-libs-dev@openjdk.java.net > Subject: Re: Update: RFR 8172974 [JAXP] XALAN: Wrong result when > transforming namespace unaware StAX Input > > Hi Christoph, > > The performance results showed two regressions, one on Linux and another > Solaris SPARC, while at the same time a bit more than half of the > results across 5 platforms were negative. This is unfortunate. We're too > close to the release date, it's super sensitive for jaxp on the > performance front. I'd like to suggest that we move this patch to a > 9-update or 10 when we'd have plenty of time to deal with any possible > fallout. I consulted our performance expert and this is also his > recommendation. I hope you'd understand. > > Thanks, > Joe > > On 2/7/2017 4:54 PM, Joe Wang wrote: > > Hi Christoph, > > > > We'll double-check whether there's any performance implications. > > Please give us a couple of (or few) days. > > > > Thanks, > > Joe > > > > On 1/30/17, 7:28 AM, Langer, Christoph wrote: > >> Hi Joe, > >> > >> I've updated my webrev for 8172974 after pulling out the refactoring > >> of javax/xml/jaxp/unittest/transform/TransformerTest.java to Bug > >> 8173602: http://cr.openjdk.java.net/~clanger/webrevs/8172974.1/ > >> > >> After exchanging some off-list mails with Lance and Daniel, we came > >> to the conclusion that you should decide about this one when you are > >> back. The changes are merely cleanups apart from > >> > com/sun/org/apache/xml/internal/dtm/ref/sax2dtm/SAX2DTM2.startEleme > nt(), > >> which to me seem natural after 8169631, though. > >> > >> Thanks, > >> Christoph > >> > >>> -Original Message- > >>> From: Langer, Christoph > >>> Sent: Mittwoch, 18. Januar 2017 22:55 > >>> To: 'huizhe wang'; > >>> core-libs-dev@openjdk.java.net > >>> Subject: RE: RFR 8172974 [JAXP] XALAN: Wrong result when > transforming > >>> namespace unaware StAX Input > >>> > >>> Hi Joe, > >>> > >>> generally, you are right, XSLT requires namespace support. For > >>> parsing the > >>> stylesheet, it is definitely a hard requirement. Otherwise this > >>> would not make > >>> sense at all. For instance xsl directives are in the xsl namespace. > >>> That > >>> requirement is what the FAQ you are referencing [1] is talking about. > >>> > >>> As for the InputSource which is to be processed, it is probably also > >>> not a right to > >>> use non namespace aware parsing. But still it's not forbidden. For > >>> instance, the > >>> JavaDoc for SAXSource [2] states this: > >>> "Note that XSLT requires namespace support. Attempting to transform > >>> an input > >>> source that is not generated with a namespace-aware parser may > >>> result in > >>> errors. Parsers can be made namespace aware by calling the > >>> SAXParserFactory.setNamespaceAware(boolean awareness) method." > >>> > >>> So, I agree, we are in the error space here. But still I think the > >>> result of non > >>> namespace aware parsing should be the same for all types of input > >>> source. And > >>> at the moment it is the same for DOMSource and SAXSource but not for > >>> StAXSource. From that point of view I think my fix makes sense > >>> (along with the > >>> other cleanups). > >>> > >>> Best regards > >>> Christoph > >>> >
RE: RFR(XS): 8033909: Objects.requireNonNull(T, Supplier) doesn't specify what happens if the passed supplier is null itself
Hi
> FWIW I think a null Supplier should simply be ignored.
...which is what Volker's change does.
After reading through all the discussion, I'm pro adding the null check as
proposed in Volker's webrev.
Best regards
Christoph
> On 13/02/2017 6:04 PM, Volker Simonis wrote:
> > Ping...
> >
> > On Thu, Feb 9, 2017 at 5:32 PM, Jeff Dinkins
> wrote:
> >>
> >> Adding Mike’s current email.
> >>
> >> -jeff
> >>
> >>> On Feb 9, 2017, at 10:18 AM, Volker Simonis
> wrote:
> >>>
> >>> Hi,
> >>>
> >>> I want to finally resolve this long standing issue (or close it as
> >>> "will not fix" if that's not possible):
> >>>
> >>> http://cr.openjdk.java.net/~simonis/webrevs/2017/8033909/
> >>>
> >>> This change has already been discussed in length on the mailing list:
> >>>
> >>> http://mail.openjdk.java.net/pipermail/core-libs-dev/2014-
> February/thread.html#24989
> >>>
> >>> and in the bug comments:
> >>>
> >>> https://bugs.openjdk.java.net/browse/JDK-8033909
> >>>
> >>> So I'll give just a short summary here:
> >>>
> >>> - Objects.requireNonNull(T, Supplier) does not check for the Supplier
> >>> argument being null. Instead it relies on the fact, that the VM will
> >>> implicitly throw a NullPointerException when it calls .get on the
> >>> Supplier argument during the creation of the explicit
> >>> NullPointerException which it is supposed to throw.
> >>>
> >>> - this behavior slightly differs from Objects.requireNonNull(T,
> >>> String) which simply creates a NullPointerException with a null
> >>> message in the case where the String argument is null.
> >>>
> >>> - the difference is not evident in the OpenJDK, because the HotSpot VM
> >>> creates a NPE with a null message by default if we call a method on a
> >>> null object.
> >>>
> >>> - however creating such a NPE with a null message when invoking a
> >>> method on a null object is not enforced by the standard, and other
> >>> implementations can do better :) For the following trivial program:
> >>>
> >>> public class NonNull {
> >>> public static void main(String[] args) {
> >>>Supplier ss = null;
> >>>Object o = Objects.requireNonNull(null, ss);
> >>> }
> >>> }
> >>>
> >>> the current OpenJDK implementation returns:
> >>>
> >>> Exception in thread "main" java.lang.NullPointerException
> >>>at java.util.Objects.requireNonNull(Objects.java:290)
> >>>at NonNull.main(NonNull.java:8)
> >>>
> >>> but the SAP JVM will print:
> >>>
> >>> Exception in thread "main" java.lang.NullPointerException: while
> >>> trying to invoke the method java.util.function.Supplier.get() of a
> >>> null object loaded from local variable 'messageSupplier'
> >>>at java.util.Objects.requireNonNull(Objects.java:290)
> >>>at NonNull.main(NonNull.java:8)
> >>>
> >>> which is at least confusing for a call to Objects.requireNonNul() with
> >>> a null Supplier. We think the exception should be the same like the
> >>> one we get when invoking Objects.requireNonNul() with a null String.
> >>>
> >>> - because of this difference (and because we like our extended
> >>> Exception messages :) the JCK test for Objects.requireNonNul(T,
> >>> Supplier) (i.e.
> >>> api/java_util/Objects/index.html#RequireNonNullMessageSupplier)
> was
> >>> removed from TCK 8 and is still commented out in TCK 9
> >>> (npe_checkingNullSupplier() in RequireNonNullMessageSupplier.java).
> >>>
> >>> I really think that the simplest and most natural fix for this problem
> >>> is to simply check for a null Supplier argument and create the NPE
> >>> with an explicit null message in that case (as outlined in the
> >>> webrev). This:
> >>> - makes the two requireNonNul() overloads for String and Supplier
> >>> behave the same (which I think was the initial intention).
> >>> - doesn't require documentation changes
> >>> - makes it possible to write a simple and conforming TCK test
> >>>
> >>> If we can't agree on this quickly, I suggest to close the issue as
> >>> "will-not-fix" and leave it untested by the TCK.
> >>>
> >>> Thank you and best regards,
> >>> Volker
> >>>
> >>> PS: the 'XS' obviously only applies to the fix, not to this message :)
> >>
RE: RFR - Update: 8168664 [JAXP] XALAN: xsl:element generates namespace prefixes if namespace is given as URI only
Hi Joe, here is the change, rebased for JDK10: http://cr.openjdk.java.net/~clanger/webrevs/8168664-10.0/ This version would never generate prefixes for xsl:element and hence I removed the check for the property. Also Aleksej's test test/javax/xml/jaxp/unittest/transform/NamespacePrefixTest.java needs to go then as it would be obsolete and would fail. Other than that, I ran the jtreg tests and found no other issues. Thanks Christoph From: huizhe wang [mailto:huizhe.w...@oracle.com] Sent: Donnerstag, 9. Februar 2017 17:25 To: Langer, Christoph Cc: core-libs-dev@openjdk.java.net; Aleksej Efimov Subject: Re: RFR - Update: 8168664 [JAXP] XALAN: xsl:element generates namespace prefixes if namespace is given as URI only On 2/7/2017 11:28 PM, Langer, Christoph wrote: Hi Joe, welcome back. Thanks! This sounds reasonable. I'll prepare a change for JDK10. Hopefully it'd open soon so that you don't have to keep it for too long. Best, Joe Thanks Christoph From: Joe Wang [mailto:huizhe.w...@oracle.com] Sent: Mittwoch, 8. Februar 2017 01:39 To: Langer, Christoph <mailto:christoph.lan...@sap.com> Cc: core-libs-dev@openjdk.java.net<mailto:core-libs-dev@openjdk.java.net>; Aleksej Efimov <mailto:aleksej.efi...@oracle.com> Subject: Re: RFR - Update: 8168664 [JAXP] XALAN: xsl:element generates namespace prefixes if namespace is given as URI only Hi Christoph, As we discussed, let's target this for JDK 10 as a spec change at this stage for JDK 9 is too late. Removing the namespace generation would be another reason to do this in JDK 10 since then, we wouldn't need the additional property. Thanks, Joe On 1/31/17, 11:02 AM, Langer, Christoph wrote: Hi Joe, evenutally I have updated my webrev for this issue: http://cr.openjdk.java.net/~clanger/webrevs/8168664.1/<http://cr.openjdk.java.net/%7Eclanger/webrevs/8168664.1/> I've implemented your suggestion to add a property "jdk.xml.generatePrefix" which is true by default. I also added a testcase which tests the correct behavior with "-Djdk.xml.generatePrefix=false". Generally, I think namespace prefix generation at this place is not needed. Or would you know of or be able to construct a case where it is really required? If not, I rather think namespace generation at this place should be thrown out unconditionally. Maybe for JDK10? In case it is removed, it would render the test javax/xml/jaxp/unittest/transform/NamespacePrefixTest.java which was implemented recently by Alex for bug https://bugs.openjdk.java.net/browse/JDK-8167179 obsolete in its current form. However, the namespace generation function is still used some place for attribute namespaces. For JDK9 I can imagine the change of this webrev going in - however, as we already discussed, you would have to do a ccc for me. Thanks & Best regards Christoph From: Langer, Christoph Sent: Dienstag, 25. Oktober 2016 15:39 To: core-libs-dev@openjdk.java.net<mailto:core-libs-dev@openjdk.java.net> Subject: RFR (JAXP): 8168664 [JAXP] XALAN: xsl:element generates namespace prefixes if namespace is given as URI only Hi JAXP experts, please review a fix for a new issue regarding namespace handling in Xalan with xsl:element. Bug: https://bugs.openjdk.java.net/browse/JDK-8168664 Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8168664.0/<http://cr.openjdk.java.net/%7Eclanger/webrevs/8168664.0/> I'm not 100% sure if this is the right way to go or if it would break some correct behavior elsewhere. But I think the current behavior is either not correct or at least it is not required to generate new xsl namespace prefixes if the namespace comes in as URI only to an xsl:element node. The interpretative transformer from the Apache Xalan project will also produce the expected output, different to the compiled XSLTC here. In the webrev, my changes to XslElement.java are only cosmetical/comments, the behavior does not change. In BasisLibrary.java I also took the opportunity to remove the $Id tag and the unused method "nodeList2IteratorUsingHandleFromNode". If you think my change is good, I'll also add a test that runs the transformation which can be found in the bug... Thanks & best regards Christoph
RE: Update: RFR 8172974 [JAXP] XALAN: Wrong result when transforming namespace unaware StAX Input
Hi Joe, this is an updated webrev with only the change in namespace handling and the testcase: http://cr.openjdk.java.net/~clanger/webrevs/8172974.1-SAX2DTM2only/ I'd really be interested in the performance results for that one before I turn to JDK10. Thanks Christoph > -Original Message- > From: Langer, Christoph > Sent: Samstag, 11. Februar 2017 15:50 > To: 'huizhe wang' > Cc: core-libs-dev@openjdk.java.net > Subject: RE: Update: RFR 8172974 [JAXP] XALAN: Wrong result when > transforming namespace unaware StAX Input > > Hi Joe, > > that's unfortunate but if these are the numbers then we have to deal with it. > It's a bit odd, though, that this does not cause a regression on all > platforms. > So, you are really sure the regression roots in my current proposal for > 8172974? > > One thing I'd want to have been looked at is if the regression really roots to > my change in SAX2DTM2.java. This is the real modification. Everything else is > just cleanup. And looking at SAX2DTM2, I really can't see where I should have > introduced something bad. Can you retest this? > > Also, moving this to JDK10 would be fine for me. Shall I submit my fix there > as > is or do you want to conduct some deeper analysis about the performance > regression's root cause? Can I somehow assist with that? > > Thanks > Christoph > > > -Original Message- > > From: huizhe wang [mailto:huizhe.w...@oracle.com] > > Sent: Donnerstag, 9. Februar 2017 23:15 > > To: Langer, Christoph > > Cc: core-libs-dev@openjdk.java.net > > Subject: Re: Update: RFR 8172974 [JAXP] XALAN: Wrong result when > > transforming namespace unaware StAX Input > > > > Hi Christoph, > > > > The performance results showed two regressions, one on Linux and > another > > Solaris SPARC, while at the same time a bit more than half of the > > results across 5 platforms were negative. This is unfortunate. We're too > > close to the release date, it's super sensitive for jaxp on the > > performance front. I'd like to suggest that we move this patch to a > > 9-update or 10 when we'd have plenty of time to deal with any possible > > fallout. I consulted our performance expert and this is also his > > recommendation. I hope you'd understand. > > > > Thanks, > > Joe > > > > On 2/7/2017 4:54 PM, Joe Wang wrote: > > > Hi Christoph, > > > > > > We'll double-check whether there's any performance implications. > > > Please give us a couple of (or few) days. > > > > > > Thanks, > > > Joe > > > > > > On 1/30/17, 7:28 AM, Langer, Christoph wrote: > > >> Hi Joe, > > >> > > >> I've updated my webrev for 8172974 after pulling out the refactoring > > >> of javax/xml/jaxp/unittest/transform/TransformerTest.java to Bug > > >> 8173602: http://cr.openjdk.java.net/~clanger/webrevs/8172974.1/ > > >> > > >> After exchanging some off-list mails with Lance and Daniel, we came > > >> to the conclusion that you should decide about this one when you are > > >> back. The changes are merely cleanups apart from > > >> > > > com/sun/org/apache/xml/internal/dtm/ref/sax2dtm/SAX2DTM2.startEleme > > nt(), > > >> which to me seem natural after 8169631, though. > > >> > > >> Thanks, > > >> Christoph > > >> > > >>> -Original Message- > > >>> From: Langer, Christoph > > >>> Sent: Mittwoch, 18. Januar 2017 22:55 > > >>> To: 'huizhe wang'; > > >>> core-libs-dev@openjdk.java.net > > >>> Subject: RE: RFR 8172974 [JAXP] XALAN: Wrong result when > > transforming > > >>> namespace unaware StAX Input > > >>> > > >>> Hi Joe, > > >>> > > >>> generally, you are right, XSLT requires namespace support. For > > >>> parsing the > > >>> stylesheet, it is definitely a hard requirement. Otherwise this > > >>> would not make > > >>> sense at all. For instance xsl directives are in the xsl namespace. > > >>> That > > >>> requirement is what the FAQ you are referencing [1] is talking about. > > >>> > > >>> As for the InputSource which is to be processed, it is probably also > > >>> not a right to > > >>> use non namespace aware parsing. But still it's not forbidden. For > > >>
RE: RFR (JAXP) 8169450: StAX parse error if there is a newline in xml declaration
Hi Joe, this looks right to me, quite straightforward. I just have some formatting nits for the test: 1. Line 107 (XMLInputFactory xif=): space missing before '=' 2. Method comments on test methods would look nicer if formatted like: /* * line 1 * line 2 * ... */ ... I want to say that the '*' is missing in the beginning of the middle lines 3. Several whitespace at the end of lines - but jcheck should find out Best regards Christoph > -Original Message- > From: core-libs-dev [mailto:core-libs-dev-boun...@openjdk.java.net] On > Behalf Of huizhe wang > Sent: Dienstag, 14. Februar 2017 03:28 > To: core-libs-dev@openjdk.java.net > Subject: RFR (JAXP) 8169450: StAX parse error if there is a newline in xml > declaration > > A quick fix for the error parsing xml declaration. This is one of the > three outstanding issues in the xml area that must be addressed for the > coming development deadline. > > JBS: https://bugs.openjdk.java.net/browse/JDK-8169450 > webrev: http://cr.openjdk.java.net/~joehw/jdk9/8169450/webrev/ > > Checking whether a space follows " which the base parser was originally created since the xml declaration > was normalized during version detection. StAX, sharing the base parser > after it was added to the JDK, however, does not use the same version > detection/normalization process and would incorrectly parse the > declaration if there are other whitespaces. > > Thanks, > Joe >
rmid on Unix fails with Exception - maybe aftermath of JDK-8173607 ??
Hi Daniel, when starting "rmid" from current builds on Linux (and probably other Unix platforms as well) we are currently getting the exception below. It looks as if on that path libnio was not loaded and hence we're getting an UnsatisfiedLinkError. In UnixNativeDispatcher.java one can find a static initialization block which should do that, however, it needs elevated access. Maybe rmid is missing a permission for that? Any thoughts? Thanks & best regards Christoph ...>/jdk9-build-dir/images/jdk/bin/rmid Exception in thread "main" java.lang.UnsatisfiedLinkError: sun.nio.fs.UnixNativeDispatcher.getcwd()[B at java.base/sun.nio.fs.UnixNativeDispatcher.getcwd(Native Method) at java.base/sun.nio.fs.UnixFileSystem.(UnixFileSystem.java:66) at java.base/sun.nio.fs.LinuxFileSystem.(LinuxFileSystem.java:39) at java.base/sun.nio.fs.LinuxFileSystemProvider.newFileSystem(LinuxFileSystemProvider.java:46) at java.base/sun.nio.fs.LinuxFileSystemProvider.newFileSystem(LinuxFileSystemProvider.java:39) at java.base/sun.nio.fs.UnixFileSystemProvider.(UnixFileSystemProvider.java:56) at java.base/sun.nio.fs.LinuxFileSystemProvider.(LinuxFileSystemProvider.java:41) at java.base/sun.nio.fs.DefaultFileSystemProvider.create(DefaultFileSystemProvider.java:41) at java.base/java.io.FilePermission.(FilePermission.java:206) at java.base/java.lang.SecurityManager.checkRead(SecurityManager.java:899) at java.base/java.io.File.exists(File.java:815) at java.base/java.lang.ClassLoader$2.run(ClassLoader.java:2513) at java.base/java.lang.ClassLoader$2.run(ClassLoader.java:2510) at java.base/java.security.AccessController.doPrivileged(Native Method) at java.base/java.lang.ClassLoader.loadLibrary0(ClassLoader.java:2509) at java.base/java.lang.ClassLoader.loadLibrary(ClassLoader.java:2478) at java.base/java.lang.Runtime.loadLibrary0(Runtime.java:873) at java.base/java.lang.System.loadLibrary(System.java:1818) at java.base/sun.nio.fs.UnixNativeDispatcher$1.run(UnixNativeDispatcher.java:611) at java.base/sun.nio.fs.UnixNativeDispatcher$1.run(UnixNativeDispatcher.java:609) at java.base/java.security.AccessController.doPrivileged(Native Method) at java.base/sun.nio.fs.UnixNativeDispatcher.(UnixNativeDispatcher.java:609) at java.base/sun.nio.fs.UnixFileSystem.(UnixFileSystem.java:66) at java.base/sun.nio.fs.LinuxFileSystem.(LinuxFileSystem.java:39) at java.base/sun.nio.fs.LinuxFileSystemProvider.newFileSystem(LinuxFileSystemProvider.java:46) at java.base/sun.nio.fs.LinuxFileSystemProvider.newFileSystem(LinuxFileSystemProvider.java:39) at java.base/sun.nio.fs.UnixFileSystemProvider.(UnixFileSystemProvider.java:56) at java.base/sun.nio.fs.LinuxFileSystemProvider.(LinuxFileSystemProvider.java:41) at java.base/sun.nio.fs.DefaultFileSystemProvider.create(DefaultFileSystemProvider.java:41) at java.base/java.nio.file.FileSystems.(FileSystems.java:91) at java.base/java.nio.file.Paths.get(Paths.java:84) at java.base/jdk.internal.jimage.ImageReaderFactory.(ImageReaderFactory.java:50) at java.base/java.security.AccessController.doPrivileged(Native Method) at java.base/sun.net.www.protocol.jrt.JavaRuntimeURLConnection.(JavaRuntimeURLConnection.java:59) at java.base/sun.net.www.protocol.jrt.Handler.openConnection(Handler.java:42) at java.base/java.net.URL.openConnection(URL.java:1049) at java.base/jdk.internal.module.SystemModuleFinder$ImageModuleReader.checkPermissionToConnect(SystemModuleFinder.java:287) at java.base/jdk.internal.module.SystemModuleFinder$ImageModuleReader.(SystemModuleFinder.java:296) at java.base/jdk.internal.module.SystemModuleFinder$1.get(SystemModuleFinder.java:245) at java.base/jdk.internal.module.SystemModuleFinder$1.get(SystemModuleFinder.java:242) at java.base/jdk.internal.module.ModuleReferenceImpl.open(ModuleReferenceImpl.java:83) at java.base/jdk.internal.loader.BuiltinClassLoader.createModuleReader(BuiltinClassLoader.java:934) at java.base/java.util.concurrent.ConcurrentHashMap.computeIfAbsent(ConcurrentHashMap.java:1719) at java.base/jdk.internal.loader.BuiltinClassLoader.moduleReaderFor(BuiltinClassLoader.java:926) at java.base/jdk.internal.loader.BuiltinClassLoader.access$100(BuiltinClassLoader.java:92) at java.base/jdk.internal.loader.BuiltinClassLoader$1.run(BuiltinClassLoader.java:356) at java.base/jdk.internal.loader.BuiltinClassLoader$1.run(BuiltinClassLoader.java:351) at java.base/java.security.AccessController.doPrivileged(Native Method) at java.base/jdk.internal.loader.BuiltinClassLoader.findMiscResource(BuiltinClassLoader.java:350) at java.base/jdk.internal.loader.BuiltinClassLoader.findRe
RE: rmid on Unix fails with Exception - maybe aftermath of JDK-8173607 ??
Hi Daniel, thanks for your hints - I should probably have seen that myself if I would have had a more thorough look... I'm cc-ing nio-dev, maybe somebody has an idea. I probably won't be able to analyze this short term. Best regards Christoph > -Original Message- > From: Daniel Fuchs [mailto:daniel.fu...@oracle.com] > Sent: Donnerstag, 16. Februar 2017 15:53 > To: Langer, Christoph > Cc: Zeller, Arno ; Baesken, Matthias > ; core-libs-dev@openjdk.java.net > Subject: Re: rmid on Unix fails with Exception - maybe aftermath of JDK- > 8173607 ?? > > Hi Christoph, > > It looks like one of the dreaded class initialization cycle > issues. > > If you look at the stack trace, you will see that > UnixNativeDispatcher.:609 > calls System.loadLibrary at line 611 > which later down the road calls the > native UnixNativeDispatcher.getcwd command > from sun.nio.fs.UnixFileSystem. > > But at this point UnixNativeDispatcher has not > finished initializing: we're still trying to load > the nio lib... > > I don't think this has anything to do with JDK-8173607. > JDK-8173607 just splitted the JMX RMI Connector out of > java.management - and rmid has nothing to do with JMX. > > rmid doesn't depend on either java.management or > java.management.rmi, and I don't see anything > on the exception path that would involve either > of these modules. > So I suspect the culprit is probably elsewhere. > > best regards, > > -- daniel > > > > On 16/02/17 13:53, Langer, Christoph wrote: > > Hi Daniel, > > > > > > > > when starting "rmid" from current builds on Linux (and probably other > > Unix platforms as well) we are currently getting the exception below. > > > > > > > > It looks as if on that path libnio was not loaded and hence we're > > getting an UnsatisfiedLinkError. In UnixNativeDispatcher.java one can > > find a static initialization block which should do that, however, it > > needs elevated access. Maybe rmid is missing a permission for that? > > > > > > > > Any thoughts? > > > > > > > > Thanks & best regards > > > > Christoph > > > > > > > > > > > > ...>/jdk9-build-dir/images/jdk/bin/rmid > > > > Exception in thread "main" java.lang.UnsatisfiedLinkError: > > sun.nio.fs.UnixNativeDispatcher.getcwd()[B > > > > at java.base/sun.nio.fs.UnixNativeDispatcher.getcwd(Native Method) > > > > at > > java.base/sun.nio.fs.UnixFileSystem.(UnixFileSystem.java:66) > > > > at > > java.base/sun.nio.fs.LinuxFileSystem.(LinuxFileSystem.java:39) > > > > at > > > java.base/sun.nio.fs.LinuxFileSystemProvider.newFileSystem(LinuxFileSyste > mProvider.java:46) > > > > at > > > java.base/sun.nio.fs.LinuxFileSystemProvider.newFileSystem(LinuxFileSyste > mProvider.java:39) > > > > at > > > java.base/sun.nio.fs.UnixFileSystemProvider.(UnixFileSystemProvider. > java:56) > > > > at > > > java.base/sun.nio.fs.LinuxFileSystemProvider.(LinuxFileSystemProvide > r.java:41) > > > > at > > > java.base/sun.nio.fs.DefaultFileSystemProvider.create(DefaultFileSystemPr > ovider.java:41) > > > > at > > java.base/java.io.FilePermission.(FilePermission.java:206) > > > > at > > > java.base/java.lang.SecurityManager.checkRead(SecurityManager.java:899) > > > > at java.base/java.io.File.exists(File.java:815) > > > > at java.base/java.lang.ClassLoader$2.run(ClassLoader.java:2513) > > > > at java.base/java.lang.ClassLoader$2.run(ClassLoader.java:2510) > > > > at java.base/java.security.AccessController.doPrivileged(Native > > Method) > > > > at > > java.base/java.lang.ClassLoader.loadLibrary0(ClassLoader.java:2509) > > > > at > > java.base/java.lang.ClassLoader.loadLibrary(ClassLoader.java:2478) > > > > at java.base/java.lang.Runtime.loadLibrary0(Runtime.java:873) > > > > at java.base/java.lang.System.loadLibrary(System.java:1818) > > > > at > > > java.base/sun.nio.fs.UnixNativeDispatcher$1.run(UnixNativeDispatcher.java > :611) > > > > at > > > java.base/sun.nio.fs.UnixNativeDispatcher$1.run(UnixNativeDispatcher.java > :609) > > > > at java.base/java.security.AccessController.doPrivileged(Native > >
RE: rmid on Unix fails with Exception - maybe aftermath of JDK-8173607 ??
The colleagues tell me that it was about two weeks ago. But I guess it's rather a felt experience than a concrete date ;-) > -Original Message- > From: Daniel Fuchs [mailto:daniel.fu...@oracle.com] > Sent: Donnerstag, 16. Februar 2017 17:02 > To: Langer, Christoph ; nio- > d...@openjdk.java.net > Cc: Zeller, Arno ; Baesken, Matthias > ; core-libs-dev@openjdk.java.net > Subject: Re: rmid on Unix fails with Exception - maybe aftermath of JDK- > 8173607 ?? > > On 16/02/17 15:22, Langer, Christoph wrote: > > Hi Daniel, > > > > thanks for your hints - I should probably have seen that myself if I would > have had a more thorough look... > > > > I'm cc-ing nio-dev, maybe somebody has an idea. I probably won't be able > to analyze this short term. > > No problem. Do you know when that started showing up? > > best regards, > > -- daniel > > > > > Best regards > > Christoph > > > >> -Original Message- > >> From: Daniel Fuchs [mailto:daniel.fu...@oracle.com] > >> Sent: Donnerstag, 16. Februar 2017 15:53 > >> To: Langer, Christoph > >> Cc: Zeller, Arno ; Baesken, Matthias > >> ; core-libs-dev@openjdk.java.net > >> Subject: Re: rmid on Unix fails with Exception - maybe aftermath of JDK- > >> 8173607 ?? > >> > >> Hi Christoph, > >> > >> It looks like one of the dreaded class initialization cycle > >> issues. > >> > >> If you look at the stack trace, you will see that > >> UnixNativeDispatcher.:609 > >> calls System.loadLibrary at line 611 > >> which later down the road calls the > >> native UnixNativeDispatcher.getcwd command > >> from sun.nio.fs.UnixFileSystem. > >> > >> But at this point UnixNativeDispatcher has not > >> finished initializing: we're still trying to load > >> the nio lib... > >> > >> I don't think this has anything to do with JDK-8173607. > >> JDK-8173607 just splitted the JMX RMI Connector out of > >> java.management - and rmid has nothing to do with JMX. > >> > >> rmid doesn't depend on either java.management or > >> java.management.rmi, and I don't see anything > >> on the exception path that would involve either > >> of these modules. > >> So I suspect the culprit is probably elsewhere. > >> > >> best regards, > >> > >> -- daniel > >> > >> > >> > >> On 16/02/17 13:53, Langer, Christoph wrote: > >>> Hi Daniel, > >>> > >>> > >>> > >>> when starting "rmid" from current builds on Linux (and probably other > >>> Unix platforms as well) we are currently getting the exception below. > >>> > >>> > >>> > >>> It looks as if on that path libnio was not loaded and hence we're > >>> getting an UnsatisfiedLinkError. In UnixNativeDispatcher.java one can > >>> find a static initialization block which should do that, however, it > >>> needs elevated access. Maybe rmid is missing a permission for that? > >>> > >>> > >>> > >>> Any thoughts? > >>> > >>> > >>> > >>> Thanks & best regards > >>> > >>> Christoph > >>> > >>> > >>> > >>> > >>> > >>> ...>/jdk9-build-dir/images/jdk/bin/rmid > >>> > >>> Exception in thread "main" java.lang.UnsatisfiedLinkError: > >>> sun.nio.fs.UnixNativeDispatcher.getcwd()[B > >>> > >>> at java.base/sun.nio.fs.UnixNativeDispatcher.getcwd(Native > Method) > >>> > >>> at > >>> java.base/sun.nio.fs.UnixFileSystem.(UnixFileSystem.java:66) > >>> > >>> at > >>> java.base/sun.nio.fs.LinuxFileSystem.(LinuxFileSystem.java:39) > >>> > >>> at > >>> > >> > java.base/sun.nio.fs.LinuxFileSystemProvider.newFileSystem(LinuxFileSyste > >> mProvider.java:46) > >>> > >>> at > >>> > >> > java.base/sun.nio.fs.LinuxFileSystemProvider.newFileSystem(LinuxFileSyste > >> mProvider.java:39) > >>> > >>> at > >>> > >> > java.base/sun.nio.fs.UnixFileSystemProvider.(UnixFileSystemProvider. > >> java:56) > >>> > >>>
