Re: RFR: 8241306: Add SignatureMethodParameterSpec subclass for RSASSA-PSS params [v2]

2021-04-13 Thread Weijun Wang
On Tue, 13 Apr 2021 14:22:47 GMT, Sean Mullan wrote: >> You are right that the overridable methods are elsewhere >> (`XMLSignatureFactory::newSignatureMethod` and >> `SignatureMethod::getParameterSpec`), but I still feel it a little strange >> to move the default parameter of one particular

Re: RFR: 8241306: Add SignatureMethodParameterSpec subclass for RSASSA-PSS params [v2]

2021-04-13 Thread Sean Mullan
On Mon, 12 Apr 2021 20:53:21 GMT, Weijun Wang wrote: >> Ok, I understand now. I think `@implSpec` (and probably the `@implNote`) are >> in the wrong class. `@implSpec` means the implementation of this class. But >> this class is final and does not contain that logic. The logic of >>

Re: RFR: 8241306: Add SignatureMethodParameterSpec subclass for RSASSA-PSS params [v2]

2021-04-12 Thread Weijun Wang
On Mon, 12 Apr 2021 17:29:55 GMT, Sean Mullan wrote: >> I added the new lines as `@implNote` and kept the old `@implSpec` there >> (since it's still a requirement for implementations). New commit pushed. CSR >> updated as well. > > Ok, I understand now. I think `@implSpec` (and probably the

Re: RFR: 8241306: Add SignatureMethodParameterSpec subclass for RSASSA-PSS params [v2]

2021-04-12 Thread Sean Mullan
On Thu, 1 Apr 2021 13:29:42 GMT, Weijun Wang wrote: >> I'm not sure if it's appropriate to specify the default value in this >> method. As long as there is an `RSAPSSParameterSpec` object, there must be a >> non-null `PSSParameterSpec` inside and it is the one that was used to >> construct

Re: RFR: 8241306: Add SignatureMethodParameterSpec subclass for RSASSA-PSS params [v2]

2021-04-12 Thread Weijun Wang
On Mon, 12 Apr 2021 12:42:23 GMT, Sean Mullan wrote: >> src/java.xml.crypto/share/classes/javax/xml/crypto/dsig/spec/RSAPSSParameterSpec.java >> line 139: >> >>> 137: >>> 138: @Override >>> 139: public String toString() { >> >> Add specification. > > Actually, on second thought, I

Re: RFR: 8241306: Add SignatureMethodParameterSpec subclass for RSASSA-PSS params [v2]

2021-04-12 Thread Sean Mullan
On Tue, 30 Mar 2021 15:34:39 GMT, Sean Mullan wrote: >> Weijun Wang has updated the pull request incrementally with one additional >> commit since the last revision: >> >> update XMLUtils (not used by tests here) > >

Re: RFR: 8241306: Add SignatureMethodParameterSpec subclass for RSASSA-PSS params [v2]

2021-04-01 Thread Weijun Wang
On Tue, 30 Mar 2021 20:24:49 GMT, Weijun Wang wrote: >> I wonder if the @implSpec is clear enough that this will be returned. I >> might suggest adding a similar @implSpec in this method that basically >> states what you said above. > > I'm not sure if it's appropriate to specify the default

Re: RFR: 8241306: Add SignatureMethodParameterSpec subclass for RSASSA-PSS params [v2]

2021-03-30 Thread Weijun Wang
On Tue, 30 Mar 2021 18:41:45 GMT, Sean Mullan wrote: >> There are other fields in `RSASSAParams`, so if there is no DigestMethod, it >> will be SHA-256 but the other fields (like SaltLength or TrailerField) will >> still be read if they exist. >> >> If there is no `RSASSAParams` at all or if

Re: RFR: 8241306: Add SignatureMethodParameterSpec subclass for RSASSA-PSS params [v2]

2021-03-30 Thread Sean Mullan
On Tue, 30 Mar 2021 16:39:37 GMT, Weijun Wang wrote: >> src/java.xml.crypto/share/classes/javax/xml/crypto/dsig/spec/RSAPSSParameterSpec.java >> line 99: >> >>> 97: * @return the encapsulated {@code PSSParameterSpec} object >>> 98: */ >>> 99: public PSSParameterSpec

Re: RFR: 8241306: Add SignatureMethodParameterSpec subclass for RSASSA-PSS params [v2]

2021-03-30 Thread Sean Mullan
On Tue, 30 Mar 2021 16:56:22 GMT, Weijun Wang wrote: >> src/java.xml.crypto/share/classes/javax/xml/crypto/dsig/spec/RSAPSSParameterSpec.java >> line 103: >> >>> 101: } >>> 102: >>> 103: @Override >> >> Since you are overriding `Object.hashCode` and `equals`, I think you should >>

Re: RFR: 8241306: Add SignatureMethodParameterSpec subclass for RSASSA-PSS params [v2]

2021-03-30 Thread Sean Mullan
On Tue, 30 Mar 2021 16:34:45 GMT, Weijun Wang wrote: >> src/java.xml.crypto/share/classes/javax/xml/crypto/dsig/spec/RSAPSSParameterSpec.java >> line 89: >> >>> 87: * >>> 88: * @param spec the input {@code PSSParameterSpec} to be encapsulated >>> 89: */ >> >> Should this throw

Re: RFR: 8241306: Add SignatureMethodParameterSpec subclass for RSASSA-PSS params [v2]

2021-03-30 Thread Weijun Wang
On Tue, 30 Mar 2021 15:34:16 GMT, Sean Mullan wrote: >> Weijun Wang has updated the pull request incrementally with one additional >> commit since the last revision: >> >> update XMLUtils (not used by tests here) > >

Re: RFR: 8241306: Add SignatureMethodParameterSpec subclass for RSASSA-PSS params [v2]

2021-03-30 Thread Weijun Wang
On Tue, 30 Mar 2021 15:31:22 GMT, Sean Mullan wrote: >> Weijun Wang has updated the pull request incrementally with one additional >> commit since the last revision: >> >> update XMLUtils (not used by tests here) > >

Re: RFR: 8241306: Add SignatureMethodParameterSpec subclass for RSASSA-PSS params [v2]

2021-03-30 Thread Weijun Wang
On Tue, 30 Mar 2021 15:04:29 GMT, Sean Mullan wrote: >> Weijun Wang has updated the pull request incrementally with one additional >> commit since the last revision: >> >> update XMLUtils (not used by tests here) > >

Re: RFR: 8241306: Add SignatureMethodParameterSpec subclass for RSASSA-PSS params [v2]

2021-03-30 Thread Sean Mullan
On Tue, 30 Mar 2021 02:07:06 GMT, Weijun Wang wrote: >> This enhancement contains the following code changes: >> >> 1. Create a new public API `javax/xml/crypto/dsig/spec/RSAPSSParameterSpec` >> and remove the internal one. >> 2. Update marshaling and unmarshaling code inside

Re: RFR: 8241306: Add SignatureMethodParameterSpec subclass for RSASSA-PSS params [v2]

2021-03-29 Thread Weijun Wang
> This enhancement contains the following code changes: > > 1. Create a new public API `javax/xml/crypto/dsig/spec/RSAPSSParameterSpec` > and remove the internal one. > 2. Update marshaling and unmarshaling code inside `DOMRSAPSSSignatureMethod` > so it understands extra fields in