RFR: 8191808: Configurable read timeout for CRLs

[Seán Coffey]

Thanks. Looks good.

regards,
Sean.

On 09/05/2019 17:38, Sean Mullan wrote:
> On 5/9/19 11:29 AM, Seán Coffey wrote:
>> Nice feature to have Sean. Thanks.
>>
>> What do you think about adding a debug print if such a value is set ? 
>> (perhaps in initializeTimeout method)
>
> Good suggestion. I've added the following to the intializeTimeout method:
>
> +    private static int initializeTimeout(String prop, int def) {
> +        Integer tmp = GetIntegerAction.privilegedGetProperty(prop);
>          if (tmp == null || tmp < 0) {
> -            return DEFAULT_CRL_CONNECT_TIMEOUT;
> +            return def;
> +        }
> +        if (debug != null) {
> +            debug.println(prop + " set to " + tmp + " seconds");
>          }
>
> --Sean
>
>>
>> regards,
>> Sean.
>>
>> On 07/05/2019 17:16, Sean Mullan wrote:
>>> On 5/7/19 11:28 AM, Xuelei Fan wrote:
>>>> What do you think if com.sun.security.crl.readtimeout is not set, 
>>>> CRL_READ_TIMEOUT is set as the same value as CRL_CONNECT_TIMEOUT?
>>>
>>> There may be good reasons you would want different values for these 
>>> two properties, so if com.sun.security.crl.readtimeout is not set, I 
>>> think it is better to have a fixed default value and not have it be 
>>> the same as CRL_CONNECT_TIMEOUT.
>>>
>>>>
>>>> Otherwise, looks fine to me.
>>>
>>> Thanks. Could you also add your name as the CSR Reviewer?
>>>
>>> --Sean
>>>
>>>>
>>>> Xuelei
>>>>
>>>> On 5/7/2019 7:28 AM, Sean Mullan wrote:
>>>>> Please review this change to add a system property for configuring 
>>>>> the read timeout when downloading CRLs with a default value of 15 
>>>>> seconds. Currently there is no timeout so downloads of large CRLs 
>>>>> can block for a long time or indefinitely. Current workaround is 
>>>>> to use the sun.net.client.defaultReadTimeout system property but 
>>>>> that affects all connections.
>>>>>
>>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8191808
>>>>> CSR: https://bugs.openjdk.java.net/browse/JDK-8223310
>>>>> webrev: http://cr.openjdk.java.net/~mullan/webrevs/8191808/webrev.00/
>>>>>
>>>>> Thanks,
>>>>> Sean