Integrated: 8251466: test/java/io/File/GetXSpace.java fails on Windows with mapped network drives.
On Thu, 20 Jan 2022 21:10:39 GMT, Andrey Turbanov wrote: > Test `test/java/io/File/GetXSpace.java` always failed on my machine with this > output: > > --System.out:(12/489)-- > --- Testing df > C:/Programs/cygwin64 292848636 49695320 243153316 17% / > D: 59672 59672 0 100% /cygdrive/d > > > SecurityManager = null > C:/Programs/cygwin64: > df total= 299877003264 free = 0 usable = 248988995584 > getX total= 299877003264 free = 248988995584 usable = 248988995584 > :: > df total= 61104128 free = 0 usable = 0 > getX total= 0 free = 0 usable = 0 > --System.err:(23/1617)-- > java.nio.file.InvalidPathException: Illegal char <:> at index 0: : > at > java.base/sun.nio.fs.WindowsPathParser.normalize(WindowsPathParser.java:182) > at java.base/sun.nio.fs.WindowsPathParser.parse(WindowsPathParser.java:153) > at java.base/sun.nio.fs.WindowsPathParser.parse(WindowsPathParser.java:77) > at java.base/sun.nio.fs.WindowsPath.parse(WindowsPath.java:92) > at java.base/sun.nio.fs.WindowsFileSystem.getPath(WindowsFileSystem.java:232) > at java.base/java.io.File.toPath(File.java:2396) > at GetXSpace.compare(GetXSpace.java:223) > at GetXSpace.testDF(GetXSpace.java:403) > at GetXSpace.main(GetXSpace.java:437) > at > java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:104) > at java.base/java.lang.reflect.Method.invoke(Method.java:577) > at > com.sun.javatest.regtest.agent.MainWrapper$MainThread.run(MainWrapper.java:127) > at java.base/java.lang.Thread.run(Thread.java:833) > > Parsing of df output is incorrect. It skips first symbols after matching of > line. > https://github.com/openjdk/jdk/blob/293fb46f7cd28f2a08055e3eb8ec9459d64e9688/test/jdk/java/io/File/GetXSpace.java#L160 > > It means that after matching first line: > > C:/Programs/cygwin64 292848636 49695320 243153316 17% / > > > It skips first symbols of next line - symbol `D` and then proceeds to match > _corrupted_ line: > > : 59672 59672 0 100% /cygdrive/d > > > Problems affects only Windows, because only in Windows case first group of > matcher is used: > > https://github.com/openjdk/jdk/blob/293fb46f7cd28f2a08055e3eb8ec9459d64e9688/test/jdk/java/io/File/GetXSpace.java#L155-L156 > > > Testing: > * Windows x64 - always failed before. No errors after fix > * Linux x64 This pull request has now been integrated. Changeset: 178ac746 Author:Andrey Turbanov URL: https://git.openjdk.java.net/jdk/commit/178ac7465360729628521a0d555253b9fb2ad7bf Stats: 6 lines in 1 file changed: 0 ins; 1 del; 5 mod 8251466: test/java/io/File/GetXSpace.java fails on Windows with mapped network drives. Reviewed-by: bpb - PR: https://git.openjdk.java.net/jdk/pull/7170
Re: RFR: 8251466: test/java/io/File/GetXSpace.java fails on Windows with mapped network drives.
On Thu, 20 Jan 2022 21:10:39 GMT, Andrey Turbanov wrote: > Test `test/java/io/File/GetXSpace.java` always failed on my machine with this > output: > > --System.out:(12/489)-- > --- Testing df > C:/Programs/cygwin64 292848636 49695320 243153316 17% / > D: 59672 59672 0 100% /cygdrive/d > > > SecurityManager = null > C:/Programs/cygwin64: > df total= 299877003264 free = 0 usable = 248988995584 > getX total= 299877003264 free = 248988995584 usable = 248988995584 > :: > df total= 61104128 free = 0 usable = 0 > getX total= 0 free = 0 usable = 0 > --System.err:(23/1617)-- > java.nio.file.InvalidPathException: Illegal char <:> at index 0: : > at > java.base/sun.nio.fs.WindowsPathParser.normalize(WindowsPathParser.java:182) > at java.base/sun.nio.fs.WindowsPathParser.parse(WindowsPathParser.java:153) > at java.base/sun.nio.fs.WindowsPathParser.parse(WindowsPathParser.java:77) > at java.base/sun.nio.fs.WindowsPath.parse(WindowsPath.java:92) > at java.base/sun.nio.fs.WindowsFileSystem.getPath(WindowsFileSystem.java:232) > at java.base/java.io.File.toPath(File.java:2396) > at GetXSpace.compare(GetXSpace.java:223) > at GetXSpace.testDF(GetXSpace.java:403) > at GetXSpace.main(GetXSpace.java:437) > at > java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:104) > at java.base/java.lang.reflect.Method.invoke(Method.java:577) > at > com.sun.javatest.regtest.agent.MainWrapper$MainThread.run(MainWrapper.java:127) > at java.base/java.lang.Thread.run(Thread.java:833) > > Parsing of df output is incorrect. It skips first symbols after matching of > line. > https://github.com/openjdk/jdk/blob/293fb46f7cd28f2a08055e3eb8ec9459d64e9688/test/jdk/java/io/File/GetXSpace.java#L160 > > It means that after matching first line: > > C:/Programs/cygwin64 292848636 49695320 243153316 17% / > > > It skips first symbols of next line - symbol `D` and then proceeds to match > _corrupted_ line: > > : 59672 59672 0 100% /cygdrive/d > > > Problems affects only Windows, because only in Windows case first group of > matcher is used: > > https://github.com/openjdk/jdk/blob/293fb46f7cd28f2a08055e3eb8ec9459d64e9688/test/jdk/java/io/File/GetXSpace.java#L155-L156 > > > Testing: > * Windows x64 - always failed before. No errors after fix > * Linux x64 Thank you for review! - PR: https://git.openjdk.java.net/jdk/pull/7170
Re: RFR: JDK-8280534: Enable compile-time doclint reference checking [v2]
> The changes in this PR on top of the out-for-review changes in > https://git.openjdk.java.net/jdk/pull/7222 allow compile-time doclint > checking to be enabled in all JDK modules. > > Typically, a @SuppressWarnings("doclint:refernce") annotation is added to > declaration with javadoc blocks that have already had distinguished > cross-module links added (JDK-8280492). > > One exception is in src/java.base/share/classes/java/net/package-info.java > where the cross-module link was (for now) removed. Currently the > SuppressWarnings annotation type is not declared to allow its annotations to > be applied to package declarations. I'll look into amending that, but in the > mean time, I think it is beneficial for the JDK build, and the base module in > particular, to have compile-time doclint protections turned on. Joe Darcy has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains four additional commits since the last revision: - Use capabilities of JDK-8280744. - Merge branch 'master' into JDK-8280534 - Cover java.base. - JDK-8280534: Enable compile-time doclint reference checking - Changes: - all: https://git.openjdk.java.net/jdk/pull/7237/files - new: https://git.openjdk.java.net/jdk/pull/7237/files/70e9fb4a..d03401c6 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=7237=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7237=00-01 Stats: 2743 lines in 129 files changed: 1556 ins; 659 del; 528 mod Patch: https://git.openjdk.java.net/jdk/pull/7237.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7237/head:pull/7237 PR: https://git.openjdk.java.net/jdk/pull/7237
Re: Fix proposal for bug JDK-8221642
I see how NPE is thrown (from `AccessibleObject::setAccessible` and `trySetAccessible`). The proper fix should follow the rule as the access check that it can set the accessible flag only on public members of a public type that is exported unconditionally. The fix is straight forward but involves spec change. I'll post PR soon. Mandy On 1/27/22 8:45 AM, Mandy Chung wrote: Hi Andreas, What methods are you calling that throws NPE? Do you have the stack trace to share? The spec of AccessibleObject was updated for JDK-8221530 if there is no caller frame when calling from JNI: "The check when invoked by JNI code with no Java class on the stack only succeeds if the member and the declaring class are public, and the class is in a package that is exported to all modules." I think AccessibleObject::canAccess, setAccessible, trySetAccessible should follow the same rule. Mandy On 1/27/22 2:19 AM, Andreas Rosenberg wrote: Hi, this is my first posting regarding to JDK contribution, so this may be the wrong place to ask. Please point me in the right direction in this case. We are using Java rather heavily via JNI on a custom application. For a long time we did stick to JRE 1.8 for various reasons. My task is to plan an upgrade to a more recent JDK version and while doing some test I encountered bugs related to this: JDK-8227491 (JNI - caller sensitive methods). We are parsing Java class files to auto gen the JNI code for our application, and are also using reflection. The workaround given is clumsy and needs manual intervention, so I was looking for a more elegant solution. The problem is: a caller sensitive method wants to determine the caller class for security checks. In case of a JNI call no Java stack frame exists, so the JVM function "jclass JVM_GetCallerClass(JNIEnv* env)" answers NULL which leads to NPEs. My idea is this: create an internal proxy class inside "java.base" that reflects this case (e.g. "java.lang.NativeCall" or "java.lang.NativeCode"). This class is final and implements nothing. Then "jclass JVM_GetCallerClass(JNIEnv* env)" (jvm.cpp) could be modified and instead of answering NULL in case of a JNI call, it should do this to answer the class proxy: return JVM_FindClassFromBootLoader(env, "java/lang/NativeCall"); This would have the following advantages: - JNI code could again simply call "caller sensitive methods" without the need to make an additional wrapper class - it would be more a expressive way on the Java side to detect "the callee is native code" than checking for null - it would fit better into the framework I already applied this fix on my own copy of the JDK 17 sources and it works pretty well for us. As there are probably security considerations involved, advice from experts is required. But from my understanding the Java security model is designed for the main app being writing in Java. In this case there are always Java stacks frames available as parents for caller sensitive methods, so the proposed fix would not affect the behavior. This assumes that "GetCallerClass" only answers NULL for the JNI case. This needs verification. If the main app is native code which uses JNI, the Java security model can only affect the Java part and as soon as an additional Java stack frame has been generated a regular Java class will be found and the "standard behavior" should apply again. Comments appreciated. It this fix looks reasonable, what are the steps to get it implemented and integrated into the official source tree? Best regards, Andy
Re: RFR: JDK-8280534: Enable compile-time doclint reference checking
On Wed, 26 Jan 2022 20:05:07 GMT, Joe Darcy wrote: > The changes in this PR on top of the out-for-review changes in > https://git.openjdk.java.net/jdk/pull/7222 allow compile-time doclint > checking to be enabled in all JDK modules. > > Typically, a @SuppressWarnings("doclint:refernce") annotation is added to > declaration with javadoc blocks that have already had distinguished > cross-module links added (JDK-8280492). > > One exception is in src/java.base/share/classes/java/net/package-info.java > where the cross-module link was (for now) removed. Currently the > SuppressWarnings annotation type is not declared to allow its annotations to > be applied to package declarations. I'll look into amending that, but in the > mean time, I think it is beneficial for the JDK build, and the base module in > particular, to have compile-time doclint protections turned on. Marked as reviewed by mchung (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/7237
Re: RFR: 8279598: Provide adapter from RandomGenerator to Random [v2]
On Fri, 28 Jan 2022 00:36:57 GMT, Yasser Bazzi wrote: >> Suggest: >> >>> Returns an instance of {@link java.util.Random} based on this {@code >>> RandomGenerator}. >>> If this generator is already an instance of {@code Random}, it is returned. >>> Otherwise, this method >>> returns an instance of {@code Random} that delegates all methods except >>> `setSeed` to this >>> generator. Its `setSeed` method always throws {@link >>> UnsupportedOperationException}. >> >> (Note no link is necessary for RandomGenerator since we are in that class >> already.) > > Commited the change on df78e05e3e692e2189c9d318fbd4892a4b96a55f Will we gradually phase out the `Random` class? If yes, we should put this conversion method in `Random` so that we don't break the newer API shall `Random` be retired. - PR: https://git.openjdk.java.net/jdk/pull/7001
Re: RFR: 8279598: Provide adapter from RandomGenerator to Random
On Wed, 26 Jan 2022 17:49:42 GMT, Stuart Marks wrote: > This will also need a regression test. It should have a case to ensure that > `asRandom` returns `this` if it's already an instance of `Random`. Maybe also > a simple case of shuffling a list, similar to the gist. Also see if there are > some tests for Random already that this can be plugged into. Will be doing the tests as soon as i can, thanks for the review - PR: https://git.openjdk.java.net/jdk/pull/7001
Integrated: JDK-8280744: Allow SuppressWarnings to be used in all declaration contexts
On Wed, 26 Jan 2022 21:22:58 GMT, Joe Darcy wrote: > The SuppressWarnings annotation type is declared have multiple targets, > including modules; however, "package" is left off of its target list. As > package-info file are another kind of declaration where a compiler could give > warnings, allowing SuppressWarnings in that case is reasonable. Wanting > SuppressWarnings in package-info file came up while working on doclint > warnings (JDK-8280534). > > Please also review the companion CSR: > https://bugs.openjdk.java.net/browse/JDK-8280745 > > While the SuppressWarnings annotation does have a JLS section (9.6.4.5. > @SuppressWarnings), the section doesn't mention the target list therefore > doesn't appear to need to be updated. This pull request has now been integrated. Changeset: 78574057 Author:Joe Darcy URL: https://git.openjdk.java.net/jdk/commit/78574057a4758fc3da5f39af77df09dc2232a9a1 Stats: 56 lines in 1 file changed: 37 ins; 12 del; 7 mod 8280744: Allow SuppressWarnings to be used in all declaration contexts Reviewed-by: iris - PR: https://git.openjdk.java.net/jdk/pull/7239
Re: RFR: 8279598: Provide adapter from RandomGenerator to Random [v2]
On Wed, 26 Jan 2022 17:31:27 GMT, Stuart Marks wrote: >> Yasser Bazzi has updated the pull request incrementally with four additional >> commits since the last revision: >> >> - make sure setseed its initialized and throw >> - remove tabs >> - Change name of function from wrapRandom to wrap >> - Change variable name and wording in javadocs > > src/java.base/share/classes/jdk/internal/util/random/RandomWrapper.java line > 41: > >> 39: @SuppressWarnings("serial") >> 40: public class RandomWrapper extends Random implements RandomGenerator { >> 41: private final RandomGenerator randomToWrap; > > Suggest renaming the field to "generator" and replacing subsequent calls to > `this.randomToWrap.foo()` with `generator.foo()`. Changed to generator on this commit df78e05e3e692e2189c9d318fbd4892a4b96a55f > src/java.base/share/classes/jdk/internal/util/random/RandomWrapper.java line > 47: > >> 45: } >> 46: >> 47: public static Random wrapRandom(RandomGenerator random) { > > Probably better if the method is just `wrap` since the class name is > descriptive enough already. Commited this change on aadef6f465c0b776f4b8025e2655110b424c6801 - PR: https://git.openjdk.java.net/jdk/pull/7001
Re: RFR: 8279598: Provide adapter from RandomGenerator to Random [v2]
On Wed, 26 Jan 2022 17:43:09 GMT, Stuart Marks wrote: >> Is something like this better? >> /** >> * Returns a {@link java.util.Random} from this {@link >> java.util.RandomGenerator}. >> * The resulting Random acts in all respects except that setSeed is not >> available. >> * >> * @return {@link java.util.Random} >> */ > > Suggest: > >> Returns an instance of {@link java.util.Random} based on this {@code >> RandomGenerator}. >> If this generator is already an instance of {@code Random}, it is returned. >> Otherwise, this method >> returns an instance of {@code Random} that delegates all methods except >> `setSeed` to this >> generator. Its `setSeed` method always throws {@link >> UnsupportedOperationException}. > > (Note no link is necessary for RandomGenerator since we are in that class > already.) Commited the change on df78e05e3e692e2189c9d318fbd4892a4b96a55f >> throwing UnsupportedOperationException was my first thought but when i call >> asRandom() it will always throw it, i could create something like what >> ThreadLocalRandom does (check if it initialized and throw) > > Yes, probably following ThreadLocalRandom is the right thing. Pretty clunky > but I think it's necessary. ok so i made the change to check if its initialized and throw if the user tries to use setSeet() c297d42f4505b03780bd9ab2e169546fd6cffcb9 - PR: https://git.openjdk.java.net/jdk/pull/7001
Re: RFR: 8279598: Provide adapter from RandomGenerator to Random [v2]
> Hi, could i get a review on this implementation proposed by Stuart Marks, i > decided to use the > https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/util/random/RandomGenerator.html > interface to create the default method `asRandom()` that wraps around the > newer algorithms to be used on classes that do not accept the new interface. > > Some things to note as proposed by the bug report, the protected method > next(int bits) is not overrided and setSeed() method if left blank up to > discussion on what to do with it. > > Small test done on > https://gist.github.com/YShow/da678561419cda8e32fccf3a27a649d4 Yasser Bazzi has updated the pull request incrementally with four additional commits since the last revision: - make sure setseed its initialized and throw - remove tabs - Change name of function from wrapRandom to wrap - Change variable name and wording in javadocs - Changes: - all: https://git.openjdk.java.net/jdk/pull/7001/files - new: https://git.openjdk.java.net/jdk/pull/7001/files/fbdf4969..c297d42f Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=7001=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7001=00-01 Stats: 51 lines in 2 files changed: 18 ins; 0 del; 33 mod Patch: https://git.openjdk.java.net/jdk/pull/7001.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7001/head:pull/7001 PR: https://git.openjdk.java.net/jdk/pull/7001
Re: RFR: JDK-8280744: Allow SuppressWarnings to be used in package declarations [v5]
> The SuppressWarnings annotation type is declared have multiple targets, > including modules; however, "package" is left off of its target list. As > package-info file are another kind of declaration where a compiler could give > warnings, allowing SuppressWarnings in that case is reasonable. Wanting > SuppressWarnings in package-info file came up while working on doclint > warnings (JDK-8280534). > > Please also review the companion CSR: > https://bugs.openjdk.java.net/browse/JDK-8280745 > > While the SuppressWarnings annotation does have a JLS section (9.6.4.5. > @SuppressWarnings), the section doesn't mention the target list therefore > doesn't appear to need to be updated. Joe Darcy has updated the pull request incrementally with one additional commit since the last revision: Reflow updated paragraphs. - Changes: - all: https://git.openjdk.java.net/jdk/pull/7239/files - new: https://git.openjdk.java.net/jdk/pull/7239/files/7b04321c..d912b928 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=7239=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7239=03-04 Stats: 24 lines in 1 file changed: 2 ins; 1 del; 21 mod Patch: https://git.openjdk.java.net/jdk/pull/7239.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7239/head:pull/7239 PR: https://git.openjdk.java.net/jdk/pull/7239
Re: RFR: JDK-8280534: Enable compile-time doclint reference checking
On Fri, 28 Jan 2022 00:04:34 GMT, Naoto Sato wrote: > Looks fine. Nit: some files need copyright year updates. Acknowledged; I'll run a copyright update script before pushing (I tend to run that close to pushing to avoid spurious, if minor, merge conflicts). Thanks. - PR: https://git.openjdk.java.net/jdk/pull/7237
Re: RFR: JDK-8280534: Enable compile-time doclint reference checking
On Wed, 26 Jan 2022 20:05:07 GMT, Joe Darcy wrote: > The changes in this PR on top of the out-for-review changes in > https://git.openjdk.java.net/jdk/pull/7222 allow compile-time doclint > checking to be enabled in all JDK modules. > > Typically, a @SuppressWarnings("doclint:refernce") annotation is added to > declaration with javadoc blocks that have already had distinguished > cross-module links added (JDK-8280492). > > One exception is in src/java.base/share/classes/java/net/package-info.java > where the cross-module link was (for now) removed. Currently the > SuppressWarnings annotation type is not declared to allow its annotations to > be applied to package declarations. I'll look into amending that, but in the > mean time, I think it is beneficial for the JDK build, and the base module in > particular, to have compile-time doclint protections turned on. Looks fine. Nit: some files need copyright year updates. - Marked as reviewed by naoto (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/7237
Re: RFR: JDK-8280744: Allow SuppressWarnings to be used in package declarations [v4]
On Thu, 27 Jan 2022 23:27:54 GMT, Joe Darcy wrote: >> The SuppressWarnings annotation type is declared have multiple targets, >> including modules; however, "package" is left off of its target list. As >> package-info file are another kind of declaration where a compiler could >> give warnings, allowing SuppressWarnings in that case is reasonable. Wanting >> SuppressWarnings in package-info file came up while working on doclint >> warnings (JDK-8280534). >> >> Please also review the companion CSR: >> https://bugs.openjdk.java.net/browse/JDK-8280745 >> >> While the SuppressWarnings annotation does have a JLS section (9.6.4.5. >> @SuppressWarnings), the section doesn't mention the target list therefore >> doesn't appear to need to be updated. > > Joe Darcy has updated the pull request with a new target base due to a merge > or a rebase. The incremental webrev excludes the unrelated changes brought in > by the merge/rebase. The pull request contains five additional commits since > the last revision: > > - Respond to review feedback. > - Merge branch 'master' into JDK-8280744 > - Respond to CSR feedback. > - Respond to review feedback. > - JDK-8280745: Allow SuppressWarnings to be used in package declarations The current spec changes correspond to the approved CSR. - Marked as reviewed by iris (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/7239
Re: RFR: JDK-8280744: Allow SuppressWarnings to be used in package declarations [v4]
> The SuppressWarnings annotation type is declared have multiple targets, > including modules; however, "package" is left off of its target list. As > package-info file are another kind of declaration where a compiler could give > warnings, allowing SuppressWarnings in that case is reasonable. Wanting > SuppressWarnings in package-info file came up while working on doclint > warnings (JDK-8280534). > > Please also review the companion CSR: > https://bugs.openjdk.java.net/browse/JDK-8280745 > > While the SuppressWarnings annotation does have a JLS section (9.6.4.5. > @SuppressWarnings), the section doesn't mention the target list therefore > doesn't appear to need to be updated. Joe Darcy has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains five additional commits since the last revision: - Respond to review feedback. - Merge branch 'master' into JDK-8280744 - Respond to CSR feedback. - Respond to review feedback. - JDK-8280745: Allow SuppressWarnings to be used in package declarations - Changes: - all: https://git.openjdk.java.net/jdk/pull/7239/files - new: https://git.openjdk.java.net/jdk/pull/7239/files/2339a085..7b04321c Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=7239=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7239=02-03 Stats: 2656 lines in 124 files changed: 1495 ins; 647 del; 514 mod Patch: https://git.openjdk.java.net/jdk/pull/7239.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7239/head:pull/7239 PR: https://git.openjdk.java.net/jdk/pull/7239
Integrated: JDK-8270476: Make floating-point test infrastructure more lambda and method reference friendly
On Tue, 25 Jan 2022 21:40:15 GMT, Joe Darcy wrote: > Update the java.lang.{Math, StrictMath} regression test helper library to > accept method references. This allows the test programs to be DRY-er as the > inputs to the method under test don't have to be repeated. The float test > method were not updated due to limitations in type inference if both float > and double methods with functional interface parameters are present. > > Also did some general tidying up for the test code. This pull request has now been integrated. Changeset: 40a2ce20 Author:Joe Darcy URL: https://git.openjdk.java.net/jdk/commit/40a2ce20334207b542d18f52e26bf418bf29c9ca Stats: 467 lines in 27 files changed: 121 ins; 147 del; 199 mod 8270476: Make floating-point test infrastructure more lambda and method reference friendly Reviewed-by: bpb - PR: https://git.openjdk.java.net/jdk/pull/7216
Re: RFR: JDK-8270476: Make floating-point test infrastructure more lambda and method reference friendly [v2]
On Wed, 26 Jan 2022 00:20:07 GMT, Joe Darcy wrote: >> Update the java.lang.{Math, StrictMath} regression test helper library to >> accept method references. This allows the test programs to be DRY-er as the >> inputs to the method under test don't have to be repeated. The float test >> method were not updated due to limitations in type inference if both float >> and double methods with functional interface parameters are present. >> >> Also did some general tidying up for the test code. > > Joe Darcy has updated the pull request with a new target base due to a merge > or a rebase. The incremental webrev excludes the unrelated changes brought in > by the merge/rebase. The pull request contains six additional commits since > the last revision: > > - Use consistent syntax for main method. > - Merge branch 'master' into JDK-8270476 > - Appease jcheck. > - Further refactoring. > - Update copyright year, remove author tags. > - JDK-8270476: Make floating-point test infrastructure more lambda and > method reference friendly Approved. Minor changes noted may be at integration. - Marked as reviewed by bpb (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/7216
Re: RFR: JDK-8270476: Make floating-point test infrastructure more lambda and method reference friendly [v3]
> Update the java.lang.{Math, StrictMath} regression test helper library to > accept method references. This allows the test programs to be DRY-er as the > inputs to the method under test don't have to be repeated. The float test > method were not updated due to limitations in type inference if both float > and double methods with functional interface parameters are present. > > Also did some general tidying up for the test code. Joe Darcy has updated the pull request incrementally with one additional commit since the last revision: Respond to review feedback. - Changes: - all: https://git.openjdk.java.net/jdk/pull/7216/files - new: https://git.openjdk.java.net/jdk/pull/7216/files/789782ce..30987b71 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=7216=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7216=01-02 Stats: 7 lines in 3 files changed: 3 ins; 2 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/7216.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7216/head:pull/7216 PR: https://git.openjdk.java.net/jdk/pull/7216
Re: RFR: JDK-8270476: Make floating-point test infrastructure more lambda and method reference friendly [v2]
On Wed, 26 Jan 2022 00:20:07 GMT, Joe Darcy wrote: >> Update the java.lang.{Math, StrictMath} regression test helper library to >> accept method references. This allows the test programs to be DRY-er as the >> inputs to the method under test don't have to be repeated. The float test >> method were not updated due to limitations in type inference if both float >> and double methods with functional interface parameters are present. >> >> Also did some general tidying up for the test code. > > Joe Darcy has updated the pull request with a new target base due to a merge > or a rebase. The incremental webrev excludes the unrelated changes brought in > by the merge/rebase. The pull request contains six additional commits since > the last revision: > > - Use consistent syntax for main method. > - Merge branch 'master' into JDK-8270476 > - Appease jcheck. > - Further refactoring. > - Update copyright year, remove author tags. > - JDK-8270476: Make floating-point test infrastructure more lambda and > method reference friendly test/jdk/java/lang/Math/Tests.java line 24: > 22: */ > 23: > 24: import java.util.function.*; List all interfaces instead of "*"? - PR: https://git.openjdk.java.net/jdk/pull/7216
Re: RFR: JDK-8270476: Make floating-point test infrastructure more lambda and method reference friendly [v2]
On Wed, 26 Jan 2022 00:20:07 GMT, Joe Darcy wrote: >> Update the java.lang.{Math, StrictMath} regression test helper library to >> accept method references. This allows the test programs to be DRY-er as the >> inputs to the method under test don't have to be repeated. The float test >> method were not updated due to limitations in type inference if both float >> and double methods with functional interface parameters are present. >> >> Also did some general tidying up for the test code. > > Joe Darcy has updated the pull request with a new target base due to a merge > or a rebase. The incremental webrev excludes the unrelated changes brought in > by the merge/rebase. The pull request contains six additional commits since > the last revision: > > - Use consistent syntax for main method. > - Merge branch 'master' into JDK-8270476 > - Appease jcheck. > - Further refactoring. > - Update copyright year, remove author tags. > - JDK-8270476: Make floating-point test infrastructure more lambda and > method reference friendly test/jdk/java/lang/Math/CubeRootTests.java line 48: > 46: int failures=0; > 47: > 48: double minus_input = -input; Unused variable? test/jdk/java/lang/StrictMath/Tests.java line 24: > 22: */ > 23: > 24: import java.util.function.*; Maybe list each interface separately instead of using "*"? - PR: https://git.openjdk.java.net/jdk/pull/7216
Re: Additional Date-Time Formats
On Thu, 27 Jan 2022 at 19:23, Naoto Sato wrote: > Now come to think of it, I came up with the draft based on `ofPattern()` > methods. One of them is a overload method that takes a Locale argument. > Why is it so? There is a case for the Locale on the static method (as a convenience, and to remind callers that the locale matters). But there is no case for it on the builder. > > The spec Javadoc doesn't explain what repeating the pattern letter > > actually does/means. Is "M" the same as ""? > > That depends on the locale and the availability of the formats. For > example, 'M' translates into these patterns in each locale with > Gregorian calendar: > > https://unicode-org.github.io/cldr-staging/charts/40/by_type/date_&_time.gregorian.html#959cbb42bb2962f As things stand, the Javadoc isn't a standalone spec. I don't know how much that matters, but I think there should be some indication in Javadoc as to why the letter should be repeated. Stephen
Re: RFR: JDK-8280745: Allow SuppressWarnings to be used in package declarations [v3]
> The SuppressWarnings annotation type is declared have multiple targets, > including modules; however, "package" is left off of its target list. As > package-info file are another kind of declaration where a compiler could give > warnings, allowing SuppressWarnings in that case is reasonable. Wanting > SuppressWarnings in package-info file came up while working on doclint > warnings (JDK-8280534). > > Please also review the companion CSR: > https://bugs.openjdk.java.net/browse/JDK-8280745 > > While the SuppressWarnings annotation does have a JLS section (9.6.4.5. > @SuppressWarnings), the section doesn't mention the target list therefore > doesn't appear to need to be updated. Joe Darcy has updated the pull request incrementally with one additional commit since the last revision: Respond to CSR feedback. - Changes: - all: https://git.openjdk.java.net/jdk/pull/7239/files - new: https://git.openjdk.java.net/jdk/pull/7239/files/653a14ae..2339a085 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=7239=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7239=01-02 Stats: 27 lines in 1 file changed: 9 ins; 8 del; 10 mod Patch: https://git.openjdk.java.net/jdk/pull/7239.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7239/head:pull/7239 PR: https://git.openjdk.java.net/jdk/pull/7239
Re: RFR: JDK-8277795: ldap connection timeout not honoured under contention [v3]
On Tue, 25 Jan 2022 15:30:46 GMT, Mark Sheppard wrote: >> yes a redeclaration of timeout with a type long across the component >> would be a consistent approach, also > > so just to clarify, we're not taking the approach to homogenise the timeout > declarations, throughout the component, to be of type long? > > which would see LdapClientFactory constructor take a long timeout and > timeout member varaiables be redefined as long Not in this issue. I plan to file a follow up bug to make a slight change to the test. I would like to get this issue fixed ASAP and would appreciate the time to take a good look at the transition to a long timeout. (i.e. I'll handle it in that follow up issue) - PR: https://git.openjdk.java.net/jdk/pull/6568
Re: Additional Date-Time Formats
Hi Stephen, On 1/27/22 1:00 AM, Stephen Colebourne wrote: Hi, This would be a useful addition. Some comments: There is no need for the method overload that takes Locale. The other similar methods all operate using the locale of the formatter, and have this Javadoc: * The locale is determined from the formatter. The formatter returned directly by * this method will use the {@link Locale#getDefault() default FORMAT locale}. * The locale can be controlled using {@link DateTimeFormatter#withLocale(Locale) withLocale(Locale)} * on the result of this method. And `appendLocalizedPattern` should not take a Locale either. Again , it would use the locale of the formatter instance, calculating the actual pattern on-demand when the formatter is run. That makes sense. Will revise the spec. Now come to think of it, I came up with the draft based on `ofPattern()` methods. One of them is a overload method that takes a Locale argument. Why is it so? The spec Javadoc doesn't explain what repeating the pattern letter actually does/means. Is "M" the same as ""? That depends on the locale and the availability of the formats. For example, 'M' translates into these patterns in each locale with Gregorian calendar: https://unicode-org.github.io/cldr-staging/charts/40/by_type/date_&_time.gregorian.html#959cbb42bb2962f Naoto
Re: RFR: JDK-8280745: Allow SuppressWarnings to be used in package declarations [v2]
On Thu, 27 Jan 2022 18:54:06 GMT, Joe Darcy wrote: >> The SuppressWarnings annotation type is declared have multiple targets, >> including modules; however, "package" is left off of its target list. As >> package-info file are another kind of declaration where a compiler could >> give warnings, allowing SuppressWarnings in that case is reasonable. Wanting >> SuppressWarnings in package-info file came up while working on doclint >> warnings (JDK-8280534). >> >> Please also review the companion CSR: >> https://bugs.openjdk.java.net/browse/JDK-8280745 >> >> While the SuppressWarnings annotation does have a JLS section (9.6.4.5. >> @SuppressWarnings), the section doesn't mention the target list therefore >> doesn't appear to need to be updated. > > Joe Darcy has updated the pull request incrementally with one additional > commit since the last revision: > > Respond to review feedback. Furthering on a suggestion from Alex in the CSR review, I've reworked the change to remove the @Target meta-annotation from SuppressWarnings, which allows SuppressWarnings to be used in any declaration context. I've also synced the list of strings required to be recognized for suppressed warnings from JLS 9.6.4.5 @SuppressWarnings; the phrasing is such that any additions from JLS 9.6.4.5 will be implicitly included even if the javadoc is not updated. If, say, the jdk.compiler module listed the strings javac accepted for suppressed warnings, I'd like to that from the implNote, but until such a list is added, I just gave instructions from how to get a list. - PR: https://git.openjdk.java.net/jdk/pull/7239
Re: RFR: JDK-8280745: Allow SuppressWarnings to be used in package declarations [v2]
> The SuppressWarnings annotation type is declared have multiple targets, > including modules; however, "package" is left off of its target list. As > package-info file are another kind of declaration where a compiler could give > warnings, allowing SuppressWarnings in that case is reasonable. Wanting > SuppressWarnings in package-info file came up while working on doclint > warnings (JDK-8280534). > > Please also review the companion CSR: > https://bugs.openjdk.java.net/browse/JDK-8280745 > > While the SuppressWarnings annotation does have a JLS section (9.6.4.5. > @SuppressWarnings), the section doesn't mention the target list therefore > doesn't appear to need to be updated. Joe Darcy has updated the pull request incrementally with one additional commit since the last revision: Respond to review feedback. - Changes: - all: https://git.openjdk.java.net/jdk/pull/7239/files - new: https://git.openjdk.java.net/jdk/pull/7239/files/317e6a5c..653a14ae Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=7239=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7239=00-01 Stats: 33 lines in 1 file changed: 26 ins; 6 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/7239.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7239/head:pull/7239 PR: https://git.openjdk.java.net/jdk/pull/7239
Re: JarIndex not following the specification
Hi Tobias, Regardless of whether this is a bug or not, JarIndex has been disabled by default in JDK 18, in the hope to eventually remove it in the future. See https://bugs.openjdk.java.net/browse/JDK-8273473 best regards, -- daniel On 27/01/2022 18:41, Tobias Stadler wrote: Hello everyone, According to https://docs.oracle.com/en/java/javase/17/docs/specs/jar/jar.html, sections in META-INF/INDEX.LIST are divided by blank lines. The write method of jdk.internal.util.jar.JarIndex does write those blank lines. But the read method begins a new section whenever it sees a line that ends with .jar. Does anybody know why JarIndex#read does not follow the specification? Is this a bug? E.g. some.jar META-INF META-INF/another.jar org Will be be read as two sections instead of just one. Also the package org will be mapped to META-INF/another.jar instead of some.jar. Best reagrds Tobias
Re: RFR: 8279842: HTTPS Channel Binding support for Java GSS/Kerberos [v12]
On Thu, 27 Jan 2022 18:05:25 GMT, Michael McMahon wrote: >> Hi, >> >> This change adds Channel Binding Token (CBT) support to HTTPS >> (java.net.HttpsURLConnection) when used with the Negotiate (SPNEGO, >> Kerberos) authentication scheme. When enabled, the implementation >> preemptively includes a CBT with authentication requests over Kerberos. The >> feature is enabled as follows: >> >> A system property "jdk.spnego.cbt" is defined which can have the values >> "never" (default), which means the feature is disabled, "always", which >> means the CBT is included for all https Negotiate authentications, or it can >> take the form "domain:a,b.c,*.d.com" which is a comma separated list of >> domains/hosts where the feature is enabled, and disabled everywhere else. In >> the given example, the CBT would be included in authentication requests for >> hosts "a", "b.c" and all hosts under the domain "d.com" and all of its >> sub-domains. >> >> A test will be added separately to the implementation. >> >> Bug report: https://bugs.openjdk.java.net/browse/JDK-8279842 >> >> Thanks, >> Michael > > Michael McMahon has updated the pull request with a new target base due to a > merge or a rebase. The incremental webrev excludes the unrelated changes > brought in by the merge/rebase. The pull request contains 15 additional > commits since the last revision: > > - test update > - Merge branch 'master' into spnego > - test update > - removed ^M from test > - Added unit test and comment update > - final review update (pre CSR) > - more updates > - fixed failing test issue and update for latest comments > - Merge branch 'master' into spnego > - added root cause to NamingException > - ... and 5 more: > https://git.openjdk.java.net/jdk/compare/ceb44101...59f703da LGTM! - Marked as reviewed by dfuchs (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/7065
Re: RFR: 8279842: HTTPS Channel Binding support for Java GSS/Kerberos [v10]
On Thu, 27 Jan 2022 16:47:52 GMT, Daniel Fuchs wrote: >> It's `java.net.SocketException: Unexpected end of file from server`. Does >> not include any CBT words so don't know if it's worth parsing. > > Thanks. Then it would be better to catch only `SocketException` here rather > than `Exception`. I'll make it catch `IOException` - PR: https://git.openjdk.java.net/jdk/pull/7065
Re: RFR: 8279842: HTTPS Channel Binding support for Java GSS/Kerberos [v12]
> Hi, > > This change adds Channel Binding Token (CBT) support to HTTPS > (java.net.HttpsURLConnection) when used with the Negotiate (SPNEGO, Kerberos) > authentication scheme. When enabled, the implementation preemptively includes > a CBT with authentication requests over Kerberos. The feature is enabled as > follows: > > A system property "jdk.spnego.cbt" is defined which can have the values > "never" (default), which means the feature is disabled, "always", which means > the CBT is included for all https Negotiate authentications, or it can take > the form "domain:a,b.c,*.d.com" which is a comma separated list of > domains/hosts where the feature is enabled, and disabled everywhere else. In > the given example, the CBT would be included in authentication requests for > hosts "a", "b.c" and all hosts under the domain "d.com" and all of its > sub-domains. > > A test will be added separately to the implementation. > > Bug report: https://bugs.openjdk.java.net/browse/JDK-8279842 > > Thanks, > Michael Michael McMahon has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 15 additional commits since the last revision: - test update - Merge branch 'master' into spnego - test update - removed ^M from test - Added unit test and comment update - final review update (pre CSR) - more updates - fixed failing test issue and update for latest comments - Merge branch 'master' into spnego - added root cause to NamingException - ... and 5 more: https://git.openjdk.java.net/jdk/compare/35ce454c...59f703da - Changes: - all: https://git.openjdk.java.net/jdk/pull/7065/files - new: https://git.openjdk.java.net/jdk/pull/7065/files/d604ee7f..59f703da Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=7065=11 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7065=10-11 Stats: 4735 lines in 368 files changed: 2835 ins; 809 del; 1091 mod Patch: https://git.openjdk.java.net/jdk/pull/7065.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7065/head:pull/7065 PR: https://git.openjdk.java.net/jdk/pull/7065
Re: RFR: 8279842: HTTPS Channel Binding support for Java GSS/Kerberos [v10]
On Wed, 26 Jan 2022 19:00:14 GMT, Weijun Wang wrote: >> test/jdk/sun/security/krb5/auto/HttpsCB.java line 201: >> >>> 199: return reader.readLine().equals(CONTENT); >>> 200: } catch (Exception e) { >>> 201: return false; >> >> Should we log that we have received the excepted exception here? Shouldn't >> the catch clause only list the exceptions that we are expecting? > > It's `java.net.SocketException: Unexpected end of file from server`. Does not > include any CBT words so don't know if it's worth parsing. Thanks. Then it would be better to catch only `SocketException` here rather than `Exception`. - PR: https://git.openjdk.java.net/jdk/pull/7065
Re: Fix proposal for bug JDK-8221642
Hi Andreas, What methods are you calling that throws NPE? Do you have the stack trace to share? The spec of AccessibleObject was updated for JDK-8221530 if there is no caller frame when calling from JNI: "The check when invoked by JNI code with no Java class on the stack only succeeds if the member and the declaring class are public, and the class is in a package that is exported to all modules." I think AccessibleObject::canAccess, setAccessible, trySetAccessible should follow the same rule. Mandy On 1/27/22 2:19 AM, Andreas Rosenberg wrote: Hi, this is my first posting regarding to JDK contribution, so this may be the wrong place to ask. Please point me in the right direction in this case. We are using Java rather heavily via JNI on a custom application. For a long time we did stick to JRE 1.8 for various reasons. My task is to plan an upgrade to a more recent JDK version and while doing some test I encountered bugs related to this: JDK-8227491 (JNI - caller sensitive methods). We are parsing Java class files to auto gen the JNI code for our application, and are also using reflection. The workaround given is clumsy and needs manual intervention, so I was looking for a more elegant solution. The problem is: a caller sensitive method wants to determine the caller class for security checks. In case of a JNI call no Java stack frame exists, so the JVM function "jclass JVM_GetCallerClass(JNIEnv* env)" answers NULL which leads to NPEs. My idea is this: create an internal proxy class inside "java.base" that reflects this case (e.g. "java.lang.NativeCall" or "java.lang.NativeCode"). This class is final and implements nothing. Then "jclass JVM_GetCallerClass(JNIEnv* env)" (jvm.cpp) could be modified and instead of answering NULL in case of a JNI call, it should do this to answer the class proxy: return JVM_FindClassFromBootLoader(env, "java/lang/NativeCall"); This would have the following advantages: - JNI code could again simply call "caller sensitive methods" without the need to make an additional wrapper class - it would be more a expressive way on the Java side to detect "the callee is native code" than checking for null - it would fit better into the framework I already applied this fix on my own copy of the JDK 17 sources and it works pretty well for us. As there are probably security considerations involved, advice from experts is required. But from my understanding the Java security model is designed for the main app being writing in Java. In this case there are always Java stacks frames available as parents for caller sensitive methods, so the proposed fix would not affect the behavior. This assumes that "GetCallerClass" only answers NULL for the JNI case. This needs verification. If the main app is native code which uses JNI, the Java security model can only affect the Java part and as soon as an additional Java stack frame has been generated a regular Java class will be found and the "standard behavior" should apply again. Comments appreciated. It this fix looks reasonable, what are the steps to get it implemented and integrated into the official source tree? Best regards, Andy
Re: RFR: 8279842: HTTPS Channel Binding support for Java GSS/Kerberos [v11]
> Hi, > > This change adds Channel Binding Token (CBT) support to HTTPS > (java.net.HttpsURLConnection) when used with the Negotiate (SPNEGO, Kerberos) > authentication scheme. When enabled, the implementation preemptively includes > a CBT with authentication requests over Kerberos. The feature is enabled as > follows: > > A system property "jdk.spnego.cbt" is defined which can have the values > "never" (default), which means the feature is disabled, "always", which means > the CBT is included for all https Negotiate authentications, or it can take > the form "domain:a,b.c,*.d.com" which is a comma separated list of > domains/hosts where the feature is enabled, and disabled everywhere else. In > the given example, the CBT would be included in authentication requests for > hosts "a", "b.c" and all hosts under the domain "d.com" and all of its > sub-domains. > > A test will be added separately to the implementation. > > Bug report: https://bugs.openjdk.java.net/browse/JDK-8279842 > > Thanks, > Michael Michael McMahon has updated the pull request incrementally with one additional commit since the last revision: test update - Changes: - all: https://git.openjdk.java.net/jdk/pull/7065/files - new: https://git.openjdk.java.net/jdk/pull/7065/files/b44184de..d604ee7f Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=7065=10 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7065=09-10 Stats: 4 lines in 1 file changed: 0 ins; 0 del; 4 mod Patch: https://git.openjdk.java.net/jdk/pull/7065.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7065/head:pull/7065 PR: https://git.openjdk.java.net/jdk/pull/7065
Withdrawn: JDK-8277375: jdeps errors on a class path with a file path with no permission
On Wed, 24 Nov 2021 01:12:01 GMT, Mandy Chung wrote: > This changes jdeps -cp to ignore files/directories with no permission to > access. This is consistent with the runtime behavior. This pull request has been closed without being integrated. - PR: https://git.openjdk.java.net/jdk/pull/6531
Re: Additional Date-Time Formats
Hi, This would be a useful addition. Some comments: There is no need for the method overload that takes Locale. The other similar methods all operate using the locale of the formatter, and have this Javadoc: * The locale is determined from the formatter. The formatter returned directly by * this method will use the {@link Locale#getDefault() default FORMAT locale}. * The locale can be controlled using {@link DateTimeFormatter#withLocale(Locale) withLocale(Locale)} * on the result of this method. And `appendLocalizedPattern` should not take a Locale either. Again , it would use the locale of the formatter instance, calculating the actual pattern on-demand when the formatter is run. The spec Javadoc doesn't explain what repeating the pattern letter actually does/means. Is "M" the same as ""? thanks Stephen On Thu, 20 Jan 2022 at 21:47, Naoto Sato wrote: > > Hello, > > I am proposing a couple of new factory methods in > java.time.format.DateTimeFormatter that produce flexible localized > date/time formats, other than the existing pre-defined > (FULL/LONG/MEDIUM/SHORT) styles. For example, if the user wants a year > and month only string, such as the title for the calendar, currently > they would have to use DateTimeFormatter.ofPattern() with explicit > pattern argument, such as "MMM y". This is problematic because it is > correct in locales such as US, but not correct in other locales. > > So, I propose methods that are parallel to ofPattern(), which take > pattern template. This is based on the CLDR's skeleton: > https://www.unicode.org/reports/tr35/tr35-dates.html#availableFormats_appendItems > > Detailed design can be found in the draft CSR: > https://bugs.openjdk.java.net/browse/JDK-8243445 > > Comments are welcome. > > Naoto >