RFR: 8283577: SSLEngine.unwrap on read-only input ByteBuffer

2022-04-29 Thread Anthony Scarpino
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]

2022-04-29 Thread Daniel Fuchs
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]

2022-04-29 Thread Jaikiran Pai
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]

2022-04-29 Thread Aleksei Efimov
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]

2022-04-29 Thread Daniel Fuchs
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]

2022-04-29 Thread Michael Felt
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]

2022-04-29 Thread Michael Felt
> 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