Re: RFR: 8277976: Break up SEQUENCE in X509Certificate::getSubjectAlternativeNames and X509Certificate::getIssuerAlternativeNames in otherName [v8]

2022-02-18 Thread Michael StJohns

On 2/18/2022 3:05 PM, Weijun Wang wrote:

On Thu, 17 Feb 2022 16:00:46 GMT, Weijun Wang  wrote:


The enhancement adds two extra items in the `getSubjectAlternativeNames()` 
output for an OtherName.

It also fix several errors:
1. In `OtherName.java`, `nameValue` should be the value inside `CONTEXT [0]` 
without the tag and length bytes.
2. The argument in constructor `extClass.getConstructor(Object.class)` is 
suspicious. Maybe it meant `byte[]`.

Weijun Wang has updated the pull request incrementally with one additional 
commit since the last revision:

   s/can/may/
_Mailing list message from Michael StJohns...

I'll use your exception message on not a `[0]`.

For dealing with the error while the parsing of `nameValue`, we've discussed it 
and my last comment is at 
https://github.com/openjdk/jdk/pull/7167#discussion_r81016. I'd rather not 
fail.

-

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


Hi Weijun -

You're already adding an error condition for a badly encoded OtherName 
that wasn't there before.  Extending that error condition check to cover 
the wrapped ASN1 object doesn't seem to me to be much of a stretch.   
Looking at the OtherName specifications, there shouldn't be any 
ambiguity here - ANY is required to be a valid ASN1 encoded object.


Hmm... wouldn't these (both) errors be masked by the location in 
X509CertImpl you were discussing?  E.g. you're still going to have the 
raw bytes, but the IOException(s) will just be captured (and maybe 
logged per Sean's suggestion) and ignored.


Later, Mike







Re: RFR: 8277976: Break up SEQUENCE in X509Certificate::getSubjectAlternativeNames and X509Certificate::getIssuerAlternativeNames in otherName [v8]

2022-02-18 Thread Weijun Wang
On Thu, 17 Feb 2022 16:00:46 GMT, Weijun Wang  wrote:

>> The enhancement adds two extra items in the `getSubjectAlternativeNames()` 
>> output for an OtherName.
>> 
>> It also fix several errors:
>> 1. In `OtherName.java`, `nameValue` should be the value inside `CONTEXT [0]` 
>> without the tag and length bytes.
>> 2. The argument in constructor `extClass.getConstructor(Object.class)` is 
>> suspicious. Maybe it meant `byte[]`.
>
> Weijun Wang has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   s/can/may/

> _Mailing list message from Michael StJohns...

I'll use your exception message on not a `[0]`.

For dealing with the error while the parsing of `nameValue`, we've discussed it 
and my last comment is at 
https://github.com/openjdk/jdk/pull/7167#discussion_r81016. I'd rather not 
fail.

-

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


Re: RFR: 8277976: Break up SEQUENCE in X509Certificate::getSubjectAlternativeNames and X509Certificate::getIssuerAlternativeNames in otherName [v8]

2022-02-17 Thread Weijun Wang
> The enhancement adds two extra items in the `getSubjectAlternativeNames()` 
> output for an OtherName.
> 
> It also fix several errors:
> 1. In `OtherName.java`, `nameValue` should be the value inside `CONTEXT [0]` 
> without the tag and length bytes.
> 2. The argument in constructor `extClass.getConstructor(Object.class)` is 
> suspicious. Maybe it meant `byte[]`.

Weijun Wang has updated the pull request incrementally with one additional 
commit since the last revision:

  s/can/may/

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7167/files
  - new: https://git.openjdk.java.net/jdk/pull/7167/files/550f14ab..5043812c

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=7167=07
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7167=06-07

  Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7167.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7167/head:pull/7167

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