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.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

Reply via email to