> On 17 Apr 2020, at 15:11, Roger Riggs <roger.ri...@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.ri...@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.ri...@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.ri...@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
>
