RFR: 8256363: Define toString() for MGF1ParameterSpec [v2]

[Sean Mullan]

On Mon, 16 Nov 2020 18:20:02 GMT, Weijun Wang <weijun at 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