Re: RFR: 8191808: Configurable read timeout for CRLs
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
Re: RFR: 8191808: Configurable read timeout for CRLs
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
Re: RFR: 8191808: Configurable read timeout for CRLs
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) 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
Re: RFR: 8191808: Configurable read timeout for CRLs
On 5/7/2019 9:16 AM, 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. It's okay to me. Otherwise, looks fine to me. Thanks. Could you also add your name as the CSR Reviewer? I added myself as the reviewer. Xuelei --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
Re: RFR: 8191808: Configurable read timeout for CRLs
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
Re: RFR: 8191808: Configurable read timeout for CRLs
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? Otherwise, looks fine to me. 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
RFR: 8191808: Configurable read timeout for CRLs
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
