RFR 15: 8243010: Test support: Customizable Hex Printer

Fri Apr 17 15:36:57 UTC 2020

> On 17 Apr 2020, at 15:11, Roger Riggs <Roger.Riggs at oracle.com> wrote:
> 
> Hi Pavel,
> 
> On 4/17/20 8:27 AM, Pavel Rappo wrote:
>> Hi Roger,
>> 
>> First of all, many thanks for doing this. After skimming through that webrev I have two comments.
>> 
>> 1. It might make sense to beef this up with more retrofitted tests, so people could more clearly see how applicable this API is. If you'd like I could help you with looking for those across the OpenJDK codebase.
> There are fewer (tests) than you might think though I only grepped for HexDumpEncoder.
> I omitted a few that are dependent on parsing HexDumpEncoder formatted strings and there are golden files to be considered.
> A separate pass will be needed for those.

Here is what I have found so far:

    test/jdk/com/sun/crypto/provider/Cipher/AEAD/GCMLargeDataKAT.java:214
    test/jdk/com/sun/crypto/provider/Cipher/AES/TestGHASH.java:74
    test/jdk/com/sun/crypto/provider/Cipher/Blowfish/BlowfishTestVector.java:131
    test/jdk/com/sun/crypto/provider/Cipher/ChaCha20/ChaCha20KAT.java:473
    test/jdk/com/sun/crypto/provider/Cipher/ChaCha20/ChaCha20Poly1305ParamTest.java:395
    test/jdk/com/sun/crypto/provider/Cipher/PBE/PBMacDoFinalVsUpdate.java:202
    test/jdk/com/sun/crypto/provider/KeyAgreement/DHKeyAgreement2.java:268
    test/jdk/com/sun/crypto/provider/KeyAgreement/DHKeyAgreement3.java:173
    test/jdk/com/sun/crypto/provider/TLS/TestLeadingZeroes.java:125
    test/jdk/java/net/httpclient/http2/java.net.http/jdk/internal/net/http/hpack/SpecHelper.java:56
    test/jdk/java/security/testlibrary/SimpleOCSPServer.java:325
    test/jdk/java/text/Normalizer/ConformanceTest.java:448
    test/jdk/java/text/Normalizer/DataValidationTest.java:420
    test/jdk/java/text/testlib/IntlTest.java:230
    test/jdk/javax/net/ssl/compatibility/ClientHelloProcessing.java:763
    test/jdk/sun/net/idn/TestStringPrep.java:262
    test/jdk/sun/security/krb5/RFC396xTest.java:218
    test/jdk/sun/security/krb5/auto/ReplayCacheTestProc.java:425
    test/jdk/sun/security/pkcs11/tls/TestLeadingZeroesP11.java:125
    test/jdk/sun/security/provider/SecureRandom/DrbgCavp.java:407
    test/jdk/sun/security/rsa/SigRecord.java:40
    test/jdk/sun/security/rsa/pss/SigRecord.java:40
    test/jdk/sun/security/ssl/ClientHandshaker/LengthCheckTest.java:774
    test/jdk/sun/security/ssl/internal/java.base/sun/security/ssl/TestHkdf.java:235
    test/jdk/sun/security/x509/X500Name/DerValueConstructor.java:121
    test/langtools/tools/javac/sym/ElementStructureTest.java:244
    test/lib/jdk/test/lib/Utils.java:493

Those could be screened for being from upstream projects (as we should not fix
those), reviewed for relevancy and applicability for retrofitting.

Just to be clear. I am NOT saying that you (or anyone else) should do it, but it
could be beneficial. Maybe security folk could help us out with review since
that list consists of use cases which are predominantly from the security area.

There are many benefits of including more tests in this changeset. One would be
polishing the API on more use cases. Another would be spreading the knowledge
of how to do things like that. Next time someone tries to find how it is typically
done in OpenJDK they will likely see a use of HexPrinter.

-Pavel

> On 17 Apr 2020, at 15:11, Roger Riggs <Roger.Riggs at oracle.com> wrote:
> 
> Hi Pavel,
> 
> On 4/17/20 8:27 AM, Pavel Rappo wrote:
>> Hi Roger,
>> 
>> First of all, many thanks for doing this. After skimming through that webrev I have two comments.
>> 
>> 1. It might make sense to beef this up with more retrofitted tests, so people could more clearly see how applicable this API is. If you'd like I could help you with looking for those across the OpenJDK codebase.
> There are fewer (tests) than you might think though I only grepped for HexDumpEncoder.
> I omitted a few that are dependent on parsing HexDumpEncoder formatted strings and there are golden files to be considered.
> A separate pass will be needed for those.
> 
> I didn't consider any uses in the src/... tree since at the moment this is only considered for testing.  If it passes muster, it deserves further consideration for JDK.
>> 
>> 2. Correct me if I'm wrong, but I couldn't find the complementary functionality of parsing in a hexdump in that webrev. I'm not sure how prevalent this is, but I can attest that there is an occasional need for it. For instance, when implementing the HPACK format I coded the tests from that RFC which use a two-byte hexadecimal representation. For example,
>> 
>>    1008 7061 7373 776f 7264 0673 6563 7265 | ..password.secre
>>    74                                      | t
>> 
>> I'm sure that's not the only case where hex parsing might be needed.
> I developed a corresponding parser but it needs more work.
> 
> Thanks, Roger
>> 
>> Regardless of whether or not this functionality is already included in that webrev (I might've missed it), the hex printing in and of itself is valuable.
>> 
>> -Pavel
>> 
>>> On 16 Apr 2020, at 20:17, Roger Riggs <Roger.Riggs at oracle.com> wrote:
>>> 
>>> Please review[2] and comment on a new Hex printing utility to support OpenJDK tests.
>>> The usefulness of a hex printing has been discussed from time to time with
>>> many suggestions as to the API shape and features.
>>> 
>>> To get an idea of the API and features, take a look at the javadoc[1].
>>> It covers the basic formatting of offset, and values, and extends the
>>> descriptive part beyond simple ASCII or printable so that a custom formatter
>>> can interpret the byte stream in parallel to the bytes.
>>> 
>>> The webrev includes changes to existing tests that currently
>>> use HexDumpEncoder to use HexPrinter.
>>> 
>>> Comments appreciated, Roger
>>> 
>>> [1]Javadoc:
>>> http://cr.openjdk.java.net/~rriggs/webrev-hexprinter-8243010/javadoc
>>> 
>>> [2] Webrev:
>>>   http://cr.openjdk.java.net/~rriggs/webrev-hexprinter-8243010/
>>> 
>>> [3] Issue:
>>>   https://bugs.openjdk.java.net/browse/JDK-8243010
> 

> 
> I didn't consider any uses in the src/... tree since at the moment this is only considered for testing.  If it passes muster, it deserves further consideration for JDK.
>> 
>> 2. Correct me if I'm wrong, but I couldn't find the complementary functionality of parsing in a hexdump in that webrev. I'm not sure how prevalent this is, but I can attest that there is an occasional need for it. For instance, when implementing the HPACK format I coded the tests from that RFC which use a two-byte hexadecimal representation. For example,
>> 
>>    1008 7061 7373 776f 7264 0673 6563 7265 | ..password.secre
>>    74                                      | t
>> 
>> I'm sure that's not the only case where hex parsing might be needed.
> I developed a corresponding parser but it needs more work.
> 
> Thanks, Roger
>> 
>> Regardless of whether or not this functionality is already included in that webrev (I might've missed it), the hex printing in and of itself is valuable.
>> 
>> -Pavel
>> 
>>> On 16 Apr 2020, at 20:17, Roger Riggs <Roger.Riggs at oracle.com> wrote:
>>> 
>>> Please review[2] and comment on a new Hex printing utility to support OpenJDK tests.
>>> The usefulness of a hex printing has been discussed from time to time with
>>> many suggestions as to the API shape and features.
>>> 
>>> To get an idea of the API and features, take a look at the javadoc[1].
>>> It covers the basic formatting of offset, and values, and extends the
>>> descriptive part beyond simple ASCII or printable so that a custom formatter
>>> can interpret the byte stream in parallel to the bytes.
>>> 
>>> The webrev includes changes to existing tests that currently
>>> use HexDumpEncoder to use HexPrinter.
>>> 
>>> Comments appreciated, Roger
>>> 
>>> [1]Javadoc:
>>> http://cr.openjdk.java.net/~rriggs/webrev-hexprinter-8243010/javadoc
>>> 
>>> [2] Webrev:
>>>   http://cr.openjdk.java.net/~rriggs/webrev-hexprinter-8243010/
>>> 
>>> [3] Issue:
>>>   https://bugs.openjdk.java.net/browse/JDK-8243010
> 