RFR: 8211227: Inconsistent TLS protocol version in debug output

[Evan Whelan-2]

Hi everyone,

Please review my fix for JDK-8211227

This supportability fix will result in a more consistent debug format when reading and writing TLS protocol versions.

Thanks,
Evan

-------------

Commit messages:
 - 8211227: Inconsistent TLS protocol version in debug output

Changes: https://git.openjdk.java.net/jdk/pull/2331/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=2331&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8211227
  Stats: 103 lines in 6 files changed: 92 ins; 0 del; 11 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2331.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2331/head:pull/2331

PR: https://git.openjdk.java.net/jdk/pull/2331

[Xue-Lei Andrew Fan]

On Mon, 1 Feb 2021 10:37:35 GMT, Evan Whelan <[hidden email]> wrote:

> Hi everyone,
>
> Please review my fix for JDK-8211227
>
> This supportability fix will result in a more consistent debug format when reading and writing TLS protocol versions.
>
> Thanks,
> Evan

Marked as reviewed by xuelei (Reviewer).

-------------

PR: https://git.openjdk.java.net/jdk/pull/2331

[Rajan Halade-2]

On Mon, 1 Feb 2021 10:37:35 GMT, Evan Whelan <[hidden email]> wrote:

> Hi everyone,
>
> Please review my fix for JDK-8211227
>
> This supportability fix will result in a more consistent debug format when reading and writing TLS protocol versions.
>
> Thanks,
> Evan

Changes requested by rhalade (Reviewer).

test/jdk/sun/security/ssl/SSLLogger/LoggingFormatConsistency.java line 36:

> 34: /*
> 35:  * This test runs in another process so we can monitor the debug
> 36:  * results.  The OutputAnalyzer must see correct debug output to return a

Suggestion:

 * results. The OutputAnalyzer must see correct debug output to return a

test/jdk/sun/security/ssl/SSLLogger/LoggingFormatConsistency.java line 83:

> 81:         // Re-enabling as test depends on these algorithms
> 82:         SecurityUtils.removeFromDisabledTlsAlgs("TLSv1", "TLSv1.1");
> 83:         var url = new URL("https://jpg-data.us.oracle.com/");

This URL is not accessible outside.

test/jdk/sun/security/ssl/SSLLogger/LoggingFormatConsistency.java line 50:

> 48: public class LoggingFormatConsistency {
> 49:     public static void main(String[] args) throws Exception {
> 50:         if (args.length == 0){

Please add a comment to explain when the test should be run with parameter.

-------------

PR: https://git.openjdk.java.net/jdk/pull/2331

[Evan Whelan-2]

On Mon, 1 Feb 2021 19:40:07 GMT, Rajan Halade <[hidden email]> wrote:

>> Hi everyone,
>>
>> Please review my fix for JDK-8211227
>>
>> This supportability fix will result in a more consistent debug format when reading and writing TLS protocol versions.
>>
>> Thanks,
>> Evan
>
> test/jdk/sun/security/ssl/SSLLogger/LoggingFormatConsistency.java line 83:
>
>> 81:         // Re-enabling as test depends on these algorithms
>> 82:         SecurityUtils.removeFromDisabledTlsAlgs("TLSv1", "TLSv1.1");
>> 83:         var url = new URL("https://jpg-data.us.oracle.com/");
>
> This URL is not accessible outside.

Thanks Rajan,

I'm going to change my approach for this test, I will set up a dummy server on localhost and create my URL based off that, rather than URLs pointing to an existing site

-------------

PR: https://git.openjdk.java.net/jdk/pull/2331

[Evan Whelan-2]

> Hi everyone,
>
> Please review my fix for JDK-8211227
>
> This supportability fix will result in a more consistent debug format when reading and writing TLS protocol versions.
>
> Thanks,
> Evan

Evan Whelan has updated the pull request incrementally with one additional commit since the last revision:

  8211227: Re-wrote LoggingFormatConsistency to use local SSL server rather than an existing URL

-------------

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/2331/files
  - new: https://git.openjdk.java.net/jdk/pull/2331/files/e8c262f2..ff7e2eb4

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2331&range=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2331&range=00-01

  Stats: 232 lines in 1 file changed: 213 ins; 5 del; 14 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2331.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2331/head:pull/2331

PR: https://git.openjdk.java.net/jdk/pull/2331

[Evan Whelan-2]

On Mon, 1 Feb 2021 19:41:09 GMT, Rajan Halade <[hidden email]> wrote:

>> Evan Whelan has updated the pull request incrementally with one additional commit since the last revision:
>>
>>   8211227: Re-wrote LoggingFormatConsistency to use local SSL server rather than an existing URL
>
> test/jdk/sun/security/ssl/SSLLogger/LoggingFormatConsistency.java line 50:
>
>> 48: public class LoggingFormatConsistency {
>> 49:     public static void main(String[] args) throws Exception {
>> 50:         if (args.length == 0){
>
> Please add a comment to explain when the test should be run with parameter.

Done! Thanks

> test/jdk/sun/security/ssl/SSLLogger/LoggingFormatConsistency.java line 36:
>
>> 34: /*
>> 35:  * This test runs in another process so we can monitor the debug
>> 36:  * results.  The OutputAnalyzer must see correct debug output to return a
>
> Suggestion:
>
>  * results. The OutputAnalyzer must see correct debug output to return a

Blank space removed

-------------

PR: https://git.openjdk.java.net/jdk/pull/2331

[Rajan Halade-2]

On Thu, 18 Feb 2021 20:37:58 GMT, Evan Whelan <[hidden email]> wrote:

>> Hi everyone,
>>
>> Please review my fix for JDK-8211227
>>
>> This supportability fix will result in a more consistent debug format when reading and writing TLS protocol versions.
>>
>> Thanks,
>> Evan
>
> Evan Whelan has updated the pull request incrementally with one additional commit since the last revision:
>
>   8211227: Re-wrote LoggingFormatConsistency to use local SSL server rather than an existing URL

Changes requested by rhalade (Reviewer).

test/jdk/sun/security/ssl/SSLLogger/LoggingFormatConsistency.java line 145:

> 143:      * Fork off the other side, then do your work.
> 144:      */
> 145:     LoggingFormatConsistency() throws Exception {

Have you looked at the templates available at test/jdk/javax/net/ssl/templates? Check SSLSocketTemplate.java which is using CountDownLatch.

-------------

PR: https://git.openjdk.java.net/jdk/pull/2331

[Evan Whelan-2]

On Thu, 18 Feb 2021 22:24:28 GMT, Rajan Halade <[hidden email]> wrote:

>> Evan Whelan has updated the pull request incrementally with one additional commit since the last revision:
>>
>>   8211227: Re-wrote LoggingFormatConsistency to use local SSL server rather than an existing URL
>
> test/jdk/sun/security/ssl/SSLLogger/LoggingFormatConsistency.java line 145:
>
>> 143:      * Fork off the other side, then do your work.
>> 144:      */
>> 145:     LoggingFormatConsistency() throws Exception {
>
> Have you looked at the templates available at test/jdk/javax/net/ssl/templates? Check SSLSocketTemplate.java which is using CountDownLatch.

I have not! I'll take a look, thanks

-------------

PR: https://git.openjdk.java.net/jdk/pull/2331

[Evan Whelan-2]

> Hi everyone,
>
> Please review my fix for JDK-8211227
>
> This supportability fix will result in a more consistent debug format when reading and writing TLS protocol versions.
>
> Thanks,
> Evan

Evan Whelan has updated the pull request incrementally with one additional commit since the last revision:

  8211227: Re-wrote LoggingFormatConsistency to leverage SSLSocketTemplate

-------------

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/2331/files
  - new: https://git.openjdk.java.net/jdk/pull/2331/files/ff7e2eb4..bc9d87d6

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2331&range=02
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2331&range=01-02

  Stats: 229 lines in 1 file changed: 17 ins; 166 del; 46 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2331.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2331/head:pull/2331

PR: https://git.openjdk.java.net/jdk/pull/2331

[Rajan Halade-2]

On Mon, 22 Feb 2021 09:47:16 GMT, Evan Whelan <[hidden email]> wrote:

>> Hi everyone,
>>
>> Please review my fix for JDK-8211227
>>
>> This supportability fix will result in a more consistent debug format when reading and writing TLS protocol versions.
>>
>> Thanks,
>> Evan
>
> Evan Whelan has updated the pull request incrementally with one additional commit since the last revision:
>
>   8211227: Re-wrote LoggingFormatConsistency to leverage SSLSocketTemplate

Thanks for developing test for this change and updating it to use template.

-------------

Marked as reviewed by rhalade (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/2331

[Evan Whelan-2]

On Mon, 22 Feb 2021 18:44:15 GMT, Rajan Halade <[hidden email]> wrote:

>> Evan Whelan has updated the pull request incrementally with one additional commit since the last revision:
>>
>>   8211227: Re-wrote LoggingFormatConsistency to leverage SSLSocketTemplate
>
> Thanks for developing test for this change and updating it to use template.

Thanks Rajan, I appreciate the guidance.

-------------

PR: https://git.openjdk.java.net/jdk/pull/2331

[Evan Whelan-2]

On Mon, 1 Feb 2021 10:37:35 GMT, Evan Whelan <[hidden email]> wrote:

> Hi everyone,
>
> Please review my fix for JDK-8211227
>
> This supportability fix will result in a more consistent debug format when reading and writing TLS protocol versions.
>
> Thanks,
> Evan

This pull request has now been integrated.

Changeset: a8672885
Author:    Evan Whelan <[hidden email]>
Committer: Rajan Halade <[hidden email]>
URL:       https://git.openjdk.java.net/jdk/commit/a8672885
Stats:     162 lines in 6 files changed: 151 ins; 0 del; 11 mod

8211227: Inconsistent TLS protocol version in debug output

Reviewed-by: xuelei, rhalade

-------------

PR: https://git.openjdk.java.net/jdk/pull/2331