RFR: 8251989: Hex formatting and parsing utility [v8]

Fri Oct 23 16:24:44 UTC 2020

On Thu, 22 Oct 2020 15:55:30 GMT, Roger Riggs <rriggs at openjdk.org> wrote:

>> java.util.HexFormat utility:
>> 
>>  - Format and parse hexadecimal strings, with parameters for delimiter, prefix, suffix and upper/lowercase
>>  - Static factories and builder methods to create HexFormat copies with modified parameters.
>>  - Consistent naming of methods for conversion of byte arrays to formatted strings and back: formatHex and parseHex
>>  - Consistent naming of methods for conversion of primitive types: toHexDigits... and fromHexDigits...
>>  - Prefix and suffixes now apply to each formatted value, not the string as a whole
>>  - Using java.util.Appendable as a target for buffered conversions so output to Writers and PrintStreams
>>    like System.out are supported in addition to StringBuilder. (IOExceptions are converted to unchecked exceptions)
>>  - Immutable and thread safe, a "value-based" class
>> 
>> See the [HexFormat javadoc](http://cr.openjdk.java.net/~rriggs/8251989-hex-formatter/java.base/java/util/HexFormat.html) for details.
>> 
>> Review comments and suggestions welcome.
>
> Roger Riggs has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Review comment updates to class javadoc

Do you want to refactor some of the classes where `HexFormat` would be a well-suited replacement for the current code, and what do you think about my other previous review comments (mostly formatting related)?
The bridgekeeper bot was so kind and censored my [previous comment](https://github.com/openjdk/jdk/pull/482#issuecomment-710709003), but apparently it can't handle review comments yet ...

And no, I am not going to sign the [OpenJDK Terms of Use](https://openjdk.java.net/legal/tou/terms) for a simple pull request comment, especially when the Terms of Use document is quite long and contains the following:
> You hereby grant to Oracle and all Users a royalty-free, perpetual, irrevocable, worldwide, non-exclusive and fully sub-licensable right and license under Your intellectual property rights to reproduce, modify, adapt, publish, translate, create derivative works from, distribute, perform, display and use Your Submissions (in whole or part) and to incorporate or implement them in other works in any form, media, or technology now known or later developed. This includes, without limitation, the right to incorporate or implement the Submission into any product or service, and to display, market, sublicense and distribute the Submissions as incorporated or embedded in any product or service distributed or offered by Oracle without compensation to you.

(Might be good to rework that to actually encourage contributions. I am kind of surprised that any company is willing to contribute to the OpenJDK under these terms.)

Sorry for driving this pull request off-topic, I will stop all interaction here and will not bother you any further here if that is desired. Below is the original comment:

-----

It might also be good to clarify in the documentation that `parseHex(...)` is case insensitive. This is currently not obvious and one might expect that it respects `isUpperCase()`.

Additionally there are quite a lot places within the JDK code where this new class could be used. I have picked some where it would definitely be an improvement (but omitted test classes):
<details>
<summary>Classes</summary>
<ul>
<li><a href="https://github.com/openjdk/jdk/blob/83ea863122e1d58527c8bf3cd178acf4d0c54f30/src/java.base/share/classes/java/net/HostPortrange.java#L94"><code>java.base</code> <code>java.net.HostPortrange</code></li>
<li><a href="https://github.com/openjdk/jdk/blob/83ea863122e1d58527c8bf3cd178acf4d0c54f30/src/java.base/share/classes/jdk/internal/module/ModuleHashes.java#L215"><code>java.base</code> <code>jdk.internal.module.ModuleHashes</code></li>
<li><a href="https://github.com/openjdk/jdk/blob/83ea863122e1d58527c8bf3cd178acf4d0c54f30/src/java.base/share/classes/sun/net/www/protocol/http/DigestAuthentication.java#L544"><code>java.base</code> <code>sun.net.www.protocol.http.DigestAuthentication</code></li>
<li><a href="https://github.com/openjdk/jdk/blob/83ea863122e1d58527c8bf3cd178acf4d0c54f30/src/java.base/share/classes/sun/security/provider/AbstractDrbg.java#L738"><code>java.base</code> <code>sun.security.provider.AbstractDrbg</code></li>
<li><a href="https://github.com/openjdk/jdk/blob/83ea863122e1d58527c8bf3cd178acf4d0c54f30/src/java.base/share/classes/sun/security/provider/certpath/ResponderId.java#L269"><code>java.base</code> <code>sun.security.provider.certpath.ResponderId</code></li>
<li><a href="https://github.com/openjdk/jdk/blob/83ea863122e1d58527c8bf3cd178acf4d0c54f30/src/java.base/share/classes/sun/security/ssl/Utilities.java#L39"><code>java.base</code> <code>sun.security.ssl.Utilities</code></li>
<li><a href="https://github.com/openjdk/jdk/blob/83ea863122e1d58527c8bf3cd178acf4d0c54f30/src/java.base/share/classes/sun/security/util/Debug.java#L323"><code>java.base</code> <code>sun.security.util.Debug</code></li>
<li><a href="https://github.com/openjdk/jdk/blob/83ea863122e1d58527c8bf3cd178acf4d0c54f30/src/java.base/share/classes/sun/security/util/HexDumpEncoder.java"><code>java.base</code> <code>sun.security.util.HexDumpEncoder</code></li>
<li><a href="https://github.com/openjdk/jdk/blob/83ea863122e1d58527c8bf3cd178acf4d0c54f30/src/java.base/share/classes/sun/security/util/ManifestEntryVerifier.java#L244"><code>java.base</code> <code>sun.security.util.ManifestEntryVerifier</code></li>
<li><a href="https://github.com/openjdk/jdk/blob/83ea863122e1d58527c8bf3cd178acf4d0c54f30/src/java.base/share/classes/sun/security/util/SignatureFileVerifier.java#L753"><code>java.base</code> <code>sun.security.util.SignatureFileVerifier</code></li>
<li><a href="https://github.com/openjdk/jdk/blob/83ea863122e1d58527c8bf3cd178acf4d0c54f30/src/java.base/share/classes/sun/security/x509/AVA.java#L742"><code>java.base</code> <code>sun.security.x509.AVA</code></li>
<li><a href="https://github.com/openjdk/jdk/blob/83ea863122e1d58527c8bf3cd178acf4d0c54f30/src/java.base/share/classes/sun/security/x509/AVA.java#L811"><code>java.base</code> <code>sun.security.x509.AVA</code></li>
<li><a href="https://github.com/openjdk/jdk/blob/83ea863122e1d58527c8bf3cd178acf4d0c54f30/src/java.base/share/classes/sun/security/x509/AVA.java#L889"><code>java.base</code> <code>sun.security.x509.AVA</code></li>
<li><a href="https://github.com/openjdk/jdk/blob/83ea863122e1d58527c8bf3cd178acf4d0c54f30/src/java.base/share/classes/sun/security/x509/AVA.java#L967"><code>java.base</code> <code>sun.security.x509.AVA</code></li>
<li><a href="https://github.com/openjdk/jdk/blob/83ea863122e1d58527c8bf3cd178acf4d0c54f30/src/java.base/share/classes/sun/security/x509/AVA.java#L1111"><code>java.base</code> <code>sun.security.x509.AVA</code></li>
<li><a href="https://github.com/openjdk/jdk/blob/83ea863122e1d58527c8bf3cd178acf4d0c54f30/src/java.base/share/classes/sun/security/x509/X509CertImpl.java#L1982"><code>java.base</code> <code>sun.security.x509.X509CertImpl</code></li>
<li><a href="https://github.com/openjdk/jdk/blob/83ea863122e1d58527c8bf3cd178acf4d0c54f30/src/java.naming/share/classes/com/sun/jndi/ldap/LdapName.java#L837"><code>java.naming</code> <code>com.sun.jndi.ldap.LdapName</code></li>
<li><a href="https://github.com/openjdk/jdk/blob/83ea863122e1d58527c8bf3cd178acf4d0c54f30/src/java.naming/share/classes/javax/naming/ldap/Rdn.java#L568"><code>java.naming</code> <code>javax.naming.ldap.Rdn</code></li>
<li><a href="https://github.com/openjdk/jdk/blob/83ea863122e1d58527c8bf3cd178acf4d0c54f30/src/java.security.jgss/share/classes/sun/security/krb5/KrbApReq.java#L314"><code>java.security.jgss</code> <code>sun.security.krb5.KrbApReq</code></li>
<li><a href="https://github.com/openjdk/jdk/blob/83ea863122e1d58527c8bf3cd178acf4d0c54f30/src/java.security.jgss/share/classes/sun/security/krb5/internal/ktab/KeyTabEntry.java#L80"><code>java.security.jgss</code> <code>sun.security.krb5.internal.ktab.KeyTabEntry</code></li>
<li><a href="https://github.com/openjdk/jdk/blob/83ea863122e1d58527c8bf3cd178acf4d0c54f30/src/java.smartcardio/share/classes/sun/security/smartcardio/PCSC.java#L174"><code>java.smartcardio</code> <code>sun.security.smartcardio.PCSC</code></li>
<li><a href="https://github.com/openjdk/jdk/blob/83ea863122e1d58527c8bf3cd178acf4d0c54f30/src/jdk.compiler/share/classes/com/sun/tools/javac/file/BaseFileManager.java#L355"><code>jdk.compiler</code> <code>com.sun.tools.javac.file.BaseFileManager</code></li>
<li><a href="https://github.com/openjdk/jdk/blob/83ea863122e1d58527c8bf3cd178acf4d0c54f30/src/jdk.compiler/share/classes/com/sun/tools/javac/main/Main.java#L490"><code>jdk.compiler</code> <code>com.sun.tools.javac.main.Main</code></li>
<li><a href="https://github.com/openjdk/jdk/blob/83ea863122e1d58527c8bf3cd178acf4d0c54f30/src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11Util.java#L158"><code>jdk.crypto.cryptoki</code> <code>sun.security.pkcs11.P11Util</code></li>
<li><a href="https://github.com/openjdk/jdk/blob/83ea863122e1d58527c8bf3cd178acf4d0c54f30/src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/wrapper/Functions.java#L168"><code>jdk.crypto.cryptoki</code> <code>sun.security.pkcs11.wrapper.Functions</code></li>
<li><a href="https://github.com/openjdk/jdk/blob/83ea863122e1d58527c8bf3cd178acf4d0c54f30/src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.java/src/org/graalvm/compiler/java/LambdaUtils.java#L129"><code>jdk.internal.vm.compiler</code> <code>org.graalvm.compiler</code></li>
<li><a href="https://github.com/openjdk/jdk/blob/83ea863122e1d58527c8bf3cd178acf4d0c54f30/src/jdk.jartool/share/classes/sun/tools/jar/Main.java#L2011"><code>jdk.jartool</code> <code>sun.tools.jar.Main</code></li>
<li><a href="https://github.com/openjdk/jdk/blob/83ea863122e1d58527c8bf3cd178acf4d0c54f30/src/jdk.jconsole/share/classes/sun/tools/jconsole/AboutDialog.java#L83"><code>jdk.jconsole</code> <code>sun.tools.jconsole.AboutDialog</code></li>
<li><a href="https://github.com/openjdk/jdk/blob/83ea863122e1d58527c8bf3cd178acf4d0c54f30/src/jdk.jconsole/share/classes/sun/tools/jconsole/ConnectDialog.java#L287"><code>jdk.jconsole</code> <code>sun.tools.jconsole.ConnectDialog</code></li>
<li><a href="https://github.com/openjdk/jdk/blob/83ea863122e1d58527c8bf3cd178acf4d0c54f30/src/jdk.jconsole/share/classes/sun/tools/jconsole/ConnectDialog.java#L595"><code>jdk.jconsole</code> <code>sun.tools.jconsole.ConnectDialog</code></li>
<li><a href="https://github.com/openjdk/jdk/blob/83ea863122e1d58527c8bf3cd178acf4d0c54f30/src/jdk.jconsole/share/classes/sun/tools/jconsole/HTMLPane.java#L61"><code>jdk.jconsole</code> <code>sun.tools.jconsole.HTMLPane</code></li>
<li><a href="https://github.com/openjdk/jdk/blob/83ea863122e1d58527c8bf3cd178acf4d0c54f30/src/jdk.jconsole/share/classes/sun/tools/jconsole/inspector/XArrayDataViewer.java#L79"><code>jdk.jconsole</code> <code>sun.tools.jconsole.inspector.XArrayDataViewer</code></li>
<li><a href="https://github.com/openjdk/jdk/blob/83ea863122e1d58527c8bf3cd178acf4d0c54f30/src/jdk.jdeps/share/classes/com/sun/tools/javap/AttributeWriter.java#L1137"><code>jdk.jdeps</code> <code>com.sun.tools.javap.AttributeWriter</code></li>
<li><a href="https://github.com/openjdk/jdk/blob/83ea863122e1d58527c8bf3cd178acf4d0c54f30/src/jdk.jdeps/share/classes/com/sun/tools/javap/ClassWriter.java#L149"><code>jdk.jdeps</code> <code>com.sun.tools.javap.ClassWriter</code></li>
<li><a href="https://github.com/openjdk/jdk/blob/83ea863122e1d58527c8bf3cd178acf4d0c54f30/src/jdk.jlink/share/classes/jdk/tools/jmod/JmodTask.java#L417"><code>jdk.jlink</code> <code>jdk.tools.jmod.JmodTask</code></li>
</ul>

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

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