RFR: 8283577: SSLEngine.unwrap on read-only input ByteBuffer
Hi, I need a review of this fix to allow a read-only 'src' buffer to be used with SSLEngine.unwrap(). A temporary read-write buffer is created in the SSLCipher operation when a read-only buffer is passed. If the 'src' is read-write, there is no effect on the current operation The PR also includes a CSR for an API implementation note to the SSLEngine.unwrap. The 'src' buffer may be modified during the decryption operation. 'unwrap()' has had this behavior forever, so there is no compatibility issue with this note. Using the 'src' buffer for in-place decryption was a performance decision. Tony - Commit messages: - update some nits - PR ready - Initial Changes: https://git.openjdk.java.net/jdk/pull/8462/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=8462&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8283577 Stats: 401 lines in 3 files changed: 301 ins; 20 del; 80 mod Patch: https://git.openjdk.java.net/jdk/pull/8462.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8462/head:pull/8462 PR: https://git.openjdk.java.net/jdk/pull/8462
Re: RFR: JDK-8280498: jdk/java/net/Inet4Address/PingThis.java fails on AIX [v4]
On Fri, 29 Apr 2022 15:06:28 GMT, Jaikiran Pai wrote: >> too much Python (no semi-colons) - great catch. >> >> Looking into how to verify proposed changes using jenkins (adoptium). When >> not in aqa-tests, more difficult (for me) too get it tested. (aka Better >> next time). > > @aixtools, if you enable GitHub Actions on your forked repo, it should then > automatically trigger a GitHub Actions job for your changes in your PR and > those results/run will be visible in this PR. The openjdk/jdk repo has been > configured to run `tier1` build/test on PR submission, so errors like this > are easily caught. Of course, if you build this locally, that's much quicker > to catch too. This is a test in tier2 though - so I don't expect it will be caught by github actions - PR: https://git.openjdk.java.net/jdk/pull/7013
Re: RFR: JDK-8280498: jdk/java/net/Inet4Address/PingThis.java fails on AIX [v4]
On Fri, 29 Apr 2022 12:11:27 GMT, Michael Felt wrote: >> Good catch! > > too much Python (no semi-colons) - great catch. > > Looking into how to verify proposed changes using jenkins (adoptium). When > not in aqa-tests, more difficult (for me) too get it tested. (aka Better next > time). @aixtools, if you enable GitHub Actions on your forked repo, it should then automatically trigger a GitHub Actions job for your changes in your PR and those results/run will be visible in this PR. The openjdk/jdk repo has been configured to run `tier1` build/test on PR submission, so errors like this are easily caught. Of course, if you build this locally, that's much quicker to catch too. - PR: https://git.openjdk.java.net/jdk/pull/7013
Re: RFR: JDK-8280498: jdk/java/net/Inet4Address/PingThis.java fails on AIX [v5]
On Fri, 29 Apr 2022 12:16:36 GMT, Michael Felt wrote: >> with IP "0.0.0.0" >> >> - it either does nothing and ping fails, or, in some virtual environments >> is treated as the default route address. >> - IPv6 support for ::1 is available since 1977; however, ::0 is not accepted >> as a vaild psuedo IPv6 address. '::1' must be used instead. >> >> ping: bind: The socket name is not available on this system. >> ping: bind: The socket name is not available on this system. >> PING ::1: (::1): 56 data bytes >> 64 bytes from ::1: icmp_seq=0 ttl=255 time=0.037 ms >> 64 bytes from ::1: icmp_seq=1 ttl=255 time=0.045 ms >> >> --- ::1 ping statistics --- >> 2 packets transmitted, 2 packets received, 0% packet loss >> round-trip min/avg/max = 0/0/0 ms >> PING ::1: (::1): 56 data bytes >> 64 bytes from ::1: icmp_seq=0 ttl=255 time=0.052 ms >> 64 bytes from ::1: icmp_seq=1 ttl=255 time=0.047 ms >> >> --- ::1 ping statistics --- >> 2 packets transmitted, 2 packets received, 0% packet loss >> >> >> A long commit message. >> >> This came to light because some systems failed with IPv4 (those that passed >> replaced 0.0.0.0 with the default router. but most just fail - not >> substituting >> 0.0.0.0 with 127.0.0.1. However, InetAddress.getByName("") returns 127.0.0.1 >> which compares well with other platform's behavior. > > Michael Felt has updated the pull request incrementally with one additional > commit since the last revision: > > missing semi-colon The changes looks ok to me given that the test can be compiled and executed with no failures. The following question being asked before ,but is there a reason why we check platform in the test code, instead of using [requires jtreg tag](https://openjdk.java.net/jtreg/tag-spec.html#requires_names) ? The following requires tag could be used as a replacement for the proposed change: * @requires (os.family != "windows" & os.family != "aix") - Marked as reviewed by aefimov (Committer). PR: https://git.openjdk.java.net/jdk/pull/7013
Re: RFR: JDK-8280498: jdk/java/net/Inet4Address/PingThis.java fails on AIX [v4]
On Fri, 29 Apr 2022 12:11:27 GMT, Michael Felt wrote: >> Good catch! > > too much Python (no semi-colons) - great catch. > > Looking into how to verify proposed changes using jenkins (adoptium). When > not in aqa-tests, more difficult (for me) too get it tested. (aka Better next > time). Still won't compile ;-) - osname should also be declared. You could do that with adding `var` before `osname` at line 48. Could you please also run the test and verify that it compiles? - PR: https://git.openjdk.java.net/jdk/pull/7013
Re: RFR: JDK-8280498: jdk/java/net/Inet4Address/PingThis.java fails on AIX [v4]
On Thu, 28 Apr 2022 15:56:00 GMT, Daniel Fuchs wrote: >> test/jdk/java/net/Inet4Address/PingThis.java line 49: >> >>> 47: public static void main(String args[]) throws Exception { >>> 48: osname = System.getProperty("os.name") >>> 49: /* >> >> Doesn't look like the line above will compile. > > Good catch! too much Python (no semi-colons) - great catch. Looking into how to verify proposed changes using jenkins (adoptium). When not in aqa-tests, more difficult (for me) too get it tested. (aka Better next time). - PR: https://git.openjdk.java.net/jdk/pull/7013
Re: RFR: JDK-8280498: jdk/java/net/Inet4Address/PingThis.java fails on AIX [v5]
> with IP "0.0.0.0" > > - it either does nothing and ping fails, or, in some virtual environments > is treated as the default route address. > - IPv6 support for ::1 is available since 1977; however, ::0 is not accepted > as a vaild psuedo IPv6 address. '::1' must be used instead. > > ping: bind: The socket name is not available on this system. > ping: bind: The socket name is not available on this system. > PING ::1: (::1): 56 data bytes > 64 bytes from ::1: icmp_seq=0 ttl=255 time=0.037 ms > 64 bytes from ::1: icmp_seq=1 ttl=255 time=0.045 ms > > --- ::1 ping statistics --- > 2 packets transmitted, 2 packets received, 0% packet loss > round-trip min/avg/max = 0/0/0 ms > PING ::1: (::1): 56 data bytes > 64 bytes from ::1: icmp_seq=0 ttl=255 time=0.052 ms > 64 bytes from ::1: icmp_seq=1 ttl=255 time=0.047 ms > > --- ::1 ping statistics --- > 2 packets transmitted, 2 packets received, 0% packet loss > > > A long commit message. > > This came to light because some systems failed with IPv4 (those that passed > replaced 0.0.0.0 with the default router. but most just fail - not > substituting > 0.0.0.0 with 127.0.0.1. However, InetAddress.getByName("") returns 127.0.0.1 > which compares well with other platform's behavior. Michael Felt has updated the pull request incrementally with one additional commit since the last revision: missing semi-colon - Changes: - all: https://git.openjdk.java.net/jdk/pull/7013/files - new: https://git.openjdk.java.net/jdk/pull/7013/files/10103c56..f5619ebd Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=7013&range=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=7013&range=03-04 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/7013.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7013/head:pull/7013 PR: https://git.openjdk.java.net/jdk/pull/7013