updated code review request: 7099399: cannot deal with CRL file larger than 16MB

Weijun Wang weijun.wang at oracle.com
Wed Oct 12 15:08:25 UTC 2011


On Oct 12, 2011, at 7:19 AM, Sean Mullan <sean.mullan at oracle.com> wrote:

> On 10/11/11 7:32 PM, Weijun Wang wrote:
>> Hi All
>> 
>> I've updated the webrev at
>> 
>>   http://cr.openjdk.java.net/~weijun/7099399/webrev.01/
>> 
>> I would like at least 2 reviewers. It might need to go to 7u2.
>> 
>> The src file is identical to the last version, and I made several 
>> changes to the test:
>> 
>> 1. Now run in othervm. Though hasn't made any changes to the VM, the 
>> huge memory footprint might prevent other tests to be running smoothly, 
>> at least before a full gc.
>> 
>> 2. It only runs on "Server" VMs now. My last JPRT test shows it failing 
>> on all c1 VMs and succeeding on all c2 VMs.
> 
> Is that the only way to determine if a server VM is being used? The name and
> whether it includes "Server" seems to be implementation specific.

Yes it does not look reliable.

In fact, this requirement of a server VM is only related to the max heap size. I'm about to do some measurement today. Hopefully I can add -Xmx??m in the @run line and remove the server VM check.

That will need another webrev.

-Max

> 
> Otherwise looks fine to me.
> 
> --Sean
> 
>> 
>> I just finished a new JPRT test, all builds and tests pass on supported 
>> platforms.
>> 
>> Thanks
>> Max
>> 
>> 
>> On 10/10/2011 10:59 PM, Weijun Wang wrote:
>>> I'm now running JPRT tests on this fix.
>>> 
>>> Unfortunately, for the 6 finished tests, I've already seen linux-i586
>>> and solaris-i586 (both with c1 VM) failed on the new test with
>>> 
>>> java.lang.OutOfMemoryError: Java heap space
>>> 
>>> I guess our DER parsing codes are using too many memory. Maybe multiple
>>> copies of huge byte array here and there. In the short term, I'll see if
>>> I can rewrite the test with othervm and a proper -Xmx option.
>>> 
>>> Hopefully customer dealing with these huge CRLs always use c2 VM with
>>> lots of memory.
>>> 
>>> -Max
>>> 
>>> On 10/10/2011 8:19 PM, Xuelei Fan wrote:
>>>> Right! Then fine to me.
>>>> 
>>>> Thanks,
>>>> Xuelei
>>>> 
>>>> On 10/11/2011 11:13 AM, Weijun Wang wrote:
>>>>> 0xff will be 255, -1 means no byte to read, EOF.
>>>>> 
>>>>> 
>>>>> On Oct 10, 2011, at 7:15 PM, Xuelei Fan<xuelei.fan at oracle.com> wrote:
>>>>> 
>>>>>> I'm not sure why the latest byte cannot be 0xFF? What about if my
>>>>>> content length is 256? For example:
>>>>>> 
>>>>>> 677 if (lowByte == -1) {
>>>>>> 678 throw new IOException("Incomplete BER/DER length info");
>>>>>> 679 }
>>>>>> 
>>>>>> Otherwise, looks fine to me.
>>>>>> 
>>>>>> Xuelei
>>>>>> 
>>>>>> On 10/11/2011 9:05 AM, Weijun Wang wrote:
>>>>>>> Webrev at http://cr.openjdk.java.net/~weijun/7099399/webrev.00/
>>>>>>> 
>>>>>>> Basically, we're now accepting X.509 block of 4-octets length. For
>>>>>>> simplicity, the highest byte must be<= 127, so that the length can be
>>>>>>> expressed with a 32-bit int.
>>>>>>> 
>>>>>>> Thanks
>>>>>>> Max
>>>>>>> 
>>>>>>> 
>>>>>>> -------- Original Message --------
>>>>>>> *Change Request ID*: 7099399
>>>>>>> *Synopsis*: cannot deal with CRL file larger than 16MB
>>>>>>> 
>>>>>>> Product: java
>>>>>>> Category: java
>>>>>>> Subcategory: classes_security
>>>>>>> Type: Defect
>>>>>>> 
>>>>>>> === *Description*
>>>>>>> ============================================================
>>>>>>> The X.509 impl of CertificateFactory only parses X.509 blocks smaller
>>>>>>> than 16MB, i.e. when the length can be encoded in 3 octets. Now we
>>>>>>> have
>>>>>>> a customer whose CRL file is as big as 30MB.
>>>>>>> 
>>>>>> 
>>>> 



More information about the security-dev mailing list