On Mon, 16 Nov 2020 18:20:02 GMT, Weijun Wang <wei...@openjdk.org> wrote:
>> src/java.base/share/classes/java/security/spec/MGF1ParameterSpec.java line >> 168: >> >>> 166: @Override >>> 167: public String toString() { >>> 168: return "MGF1:" + mdName; >> >> I would replace "MGF1" or perhaps add "DigestAlgorithm" which is the name of >> the attribute. Is it necessary to print that this is an MGF1? >> PSSParameterSpec does not print that it is an RSASSA-PSS-params, and also >> prints "MGF", so it seems there would be some duplication. It almost seems >> like we should have some rules regarding how these parameters are printed >> out so everything is consistent. >> >> Or perhaps it makes sense to have brackets around the fields. Otherwise when >> you chain several toStrings together, it makes it more difficult to discern >> when one field ends and the next begins. Hmm. > > "MGF1" is only one kind of MGF and we might see "MGF2" in the future so it's > worth printing out. Of course, `PSSParameterSpec` already has a > `getMGFAlgorithm()` so the name can either be printed in the `toString` of > the outer data type or the inner one. > > As for `DigestAlgorithm` I don't think it's necessary because it's the only > field for MGF1 so there's no ambiguity. > > That said, we can try to provide some consistency here. If the types had been > defined as records with > static record MGF1Params(String messageDigest) {} > static record PssParams(String messageDigest, String mgfAlgorithm, > MGF1Params mgfParams, int saltLength, int trailerField) {} > The automatically generated `toString` would output something like > PssParams[messageDigest=SHA-1, mgfAlgorithm=MGF1, > mgfParams=MGF1Params[messageDigest=SHA-1], saltLength=20, trailerField=1] > It's a little verbose. Do you like this style? I like the brackets surrounding the fields of each type. I think we should try to use brackets in the toString methods of security classes as there is often a deep hierarchy of objects and the brackets helps to see that. I also like seeing the actual type for non-primitives, and not an abbreviated form. As for the names of the fields, I'm ok with whatever seems most reasonable, the ASN.1 names, or the method or parameter names. ------------- PR: https://git.openjdk.java.net/jdk/pull/1208