RFR 15: 8243010: Test support: Customizable Hex Printer

Fri Apr 17 16:58:08 UTC 2020

Hi Pavel,

Thanks for the scan of potential replacements.
I'll take a look and file followup issues for candidates for cleanup.
Some are going to be sensitive to the specific formatting.

Roger

On 4/17/20 11:36 AM, Pavel Rappo wrote:
>> 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