Webrev updated at

   https://cr.openjdk.java.net/~weijun/8157404/webrev.01/

> On Apr 1, 2019, at 10:16 PM, Xuelei Fan <[email protected]> wrote:
> 
> On 3/30/2019 6:32 AM, Weijun Wang wrote:
>>> On Mar 27, 2019, at 11:48 PM, Xuelei Fan <[email protected]> wrote:
>>> 
>>> DerIndefLenConverter.convertStream().
>>> 
>>> Is it a concern that this method read too much? For example, the DER bytes 
>>> of the target object is 256 bytes, but read 1024 bytes from the input 
>>> stream.  And then the next DER or other object in the inputstream may not 
>>> be able to properly parsed.
>> Yes, this is possible, but I am not going to deal with it here.
> Okay.  Do you plan to file a new bug?

Cannot access JBS. I'll remember to file one.

Thanks,
Max

> 
>>> 
>>> BTW, if the input stream is a slow traffic, there might be a few 
>>> DataNotEnoughException get thrown.  Throwing and catching of exceptions are 
>>> expensive.  I may divide the convertBytes() into two parts, one looking for 
>>> the DER ending position, the other one converting to DER encoding.  Then 
>>> the DataNotEnoughException in convertStream() could be avoided.
>> You mean still using the same logic but just not with an exception? Can I 
>> just return null?
> Yes. Returning null should be fine as well.
> 
> Xuelei
> 
>> Thanks,
>> Max
>>> 
>>> Thanks,
>>> Xuelei
>>> 
>>> On 3/24/2019 7:42 PM, Weijun Wang wrote:
>>>> Ping again.
>>>> No new test added.
>>>> Thanks,
>>>> Max
>>>>> On Mar 5, 2019, at 11:06 AM, Weijun Wang <[email protected]> wrote:
>>>>> 
>>>>> Please take a review at
>>>>> 
>>>>>   https://cr.openjdk.java.net/~weijun/8157404/webrev.00/
>>>>> 
>>>>> When Java finds out data is not enough while resolving a BER, it reads in 
>>>>> more data and try converting again. Please note that calling available() 
>>>>> again after readNBytes is not reliable because it might return zero even 
>>>>> if there are more bytes.
>>>>> 
>>>>> A more efficient fix could be rewriting the convert logic to use the 
>>>>> stream directly (parsing while reading), and thus avoid the need to call 
>>>>> the whole convertBytes method again, but that's a big change and there is 
>>>>> a risk getting wrong somewhere. This fix is likely to be backported to 
>>>>> older LTS releases.
>>>>> 
>>>>> Note this could block but it should only happen when data is not enough, 
>>>>> and it only reads one byte.
>>>>> 
>>>>> The test included in the bug report passed, but I'll see if I can write a 
>>>>> new test not depending on any existing binary data.
>>>>> 
>>>>> And I'm running a mach5 test job now.
>>>>> 
>>>>> Thanks,
>>>>> Max
>>>>> 

Reply via email to