From 9e95ef3ea2ac6ed69994cdad1a4134e094bee4c8 Mon Sep 17 00:00:00 2001 From: Michael Bayne Date: Mon, 23 Oct 2017 18:12:48 -0700 Subject: [PATCH] Apply connect and read timeouts uniformly. Also use a 5 second connect/read timeout when detecting a proxy, to avoid lengthy stalls. --- .../threerings/getdown/data/Application.java | 9 ++---- .../threerings/getdown/launcher/Getdown.java | 6 ++-- .../getdown/net/HTTPDownloader.java | 8 ++--- .../getdown/util/ConnectionUtil.java | 32 ++++++++++++++++--- 4 files changed, 34 insertions(+), 21 deletions(-) diff --git a/src/main/java/com/threerings/getdown/data/Application.java b/src/main/java/com/threerings/getdown/data/Application.java index 0a725dc..0b65b06 100644 --- a/src/main/java/com/threerings/getdown/data/Application.java +++ b/src/main/java/com/threerings/getdown/data/Application.java @@ -1241,7 +1241,7 @@ public class Application InputStream in = null; PrintStream out = null; try { - in = ConnectionUtil.open(_latest).getInputStream(); + in = ConnectionUtil.open(_latest, 0, 0).getInputStream(); BufferedReader bin = new BufferedReader(new InputStreamReader(in)); for (String[] pair : ConfigUtil.parsePairs(bin, ConfigUtil.createOpts(false))) { if (pair[0].equals("version")) { @@ -1648,18 +1648,13 @@ public class Application InputStream fin = null; FileOutputStream fout = null; try { - URLConnection uconn = ConnectionUtil.open(targetURL); + URLConnection uconn = ConnectionUtil.open(targetURL, 0, 0); // we have to tell Java not to use caches here, otherwise it will cache any request for // same URL for the lifetime of this JVM (based on the URL string, not the URL object); // if the getdown.txt file, for example, changes in the meanwhile, we would never hear // about it; turning off caches is not a performance concern, because when Getdown asks // to download a file, it expects it to come over the wire, not from a cache uconn.setUseCaches(false); - // configure a connect timeout if requested - int ctimeout = SysProps.connectTimeout(); - if (ctimeout > 0) { - uconn.setConnectTimeout(ctimeout * 1000); - } fin = uconn.getInputStream(); fout = new FileOutputStream(target); StreamUtil.copy(fin, fout); diff --git a/src/main/java/com/threerings/getdown/launcher/Getdown.java b/src/main/java/com/threerings/getdown/launcher/Getdown.java index 8999843..3d2bef6 100644 --- a/src/main/java/com/threerings/getdown/launcher/Getdown.java +++ b/src/main/java/com/threerings/getdown/launcher/Getdown.java @@ -324,8 +324,8 @@ public abstract class Getdown extends Thread URL rurl = _app.getConfigResource().getRemote(); try { - // try to make a HEAD request for this URL - URLConnection conn = ConnectionUtil.open(rurl); + // try to make a HEAD request for this URL (use short connect and read timeouts) + URLConnection conn = ConnectionUtil.open(rurl, 5, 5); if (conn instanceof HttpURLConnection) { HttpURLConnection hcon = (HttpURLConnection)conn; try { @@ -1215,7 +1215,7 @@ public abstract class Getdown extends Thread @Override public void run () { try { - HttpURLConnection ucon = ConnectionUtil.openHttp(_url); + HttpURLConnection ucon = ConnectionUtil.openHttp(_url, 0, 0); // if we have a tracking cookie configured, configure the request with it if (_app.getTrackingCookieName() != null && diff --git a/src/main/java/com/threerings/getdown/net/HTTPDownloader.java b/src/main/java/com/threerings/getdown/net/HTTPDownloader.java index 95c5272..6317016 100644 --- a/src/main/java/com/threerings/getdown/net/HTTPDownloader.java +++ b/src/main/java/com/threerings/getdown/net/HTTPDownloader.java @@ -34,7 +34,7 @@ public class HTTPDownloader extends Downloader protected long checkSize (Resource rsrc) throws IOException { - URLConnection conn = ConnectionUtil.open(rsrc.getRemote()); + URLConnection conn = ConnectionUtil.open(rsrc.getRemote(), 0, 0); try { // if we're accessing our data via HTTP, we only need a HEAD request if (conn instanceof HttpURLConnection) { @@ -60,11 +60,7 @@ public class HTTPDownloader extends Downloader throws IOException { // download the resource from the specified URL - URLConnection conn = ConnectionUtil.open(rsrc.getRemote()); - int rtimeout = SysProps.readTimeout(); - if (rtimeout > 0) { - conn.setReadTimeout(rtimeout * 1000); - } + URLConnection conn = ConnectionUtil.open(rsrc.getRemote(), 0, 0); conn.connect(); // make sure we got a satisfactory response code diff --git a/src/main/java/com/threerings/getdown/util/ConnectionUtil.java b/src/main/java/com/threerings/getdown/util/ConnectionUtil.java index 97edc94..9948414 100644 --- a/src/main/java/com/threerings/getdown/util/ConnectionUtil.java +++ b/src/main/java/com/threerings/getdown/util/ConnectionUtil.java @@ -13,16 +13,37 @@ import java.net.URLDecoder; import org.apache.commons.codec.binary.Base64; +import com.threerings.getdown.data.SysProps; + public class ConnectionUtil { /** * Opens a connection to a URL, setting the authentication header if user info is present. + * @param url the URL to which to open a connection. + * @param connectTimeout if {@code > 0} then a timeout, in seconds, to use when opening the + * connection. If {@code 0} is supplied, the connection timeout specified via system properties + * will be used instead. + * @param readTimeout if {@code > 0} then a timeout, in seconds, to use while reading data from + * the connection. If {@code 0} is supplied, the read timeout specified via system properties + * will be used instead. */ - public static URLConnection open (URL url) + public static URLConnection open (URL url, int connectTimeout, int readTimeout) throws IOException { URLConnection conn = url.openConnection(); + // configure a connect timeout, if requested + int ctimeout = connectTimeout > 0 ? connectTimeout : SysProps.connectTimeout(); + if (ctimeout > 0) { + conn.setConnectTimeout(ctimeout * 1000); + } + + // configure a read timeout, if requested + int rtimeout = readTimeout > 0 ? readTimeout : SysProps.readTimeout(); + if (rtimeout > 0) { + conn.setReadTimeout(rtimeout * 1000); + } + // If URL has a username:password@ before hostname, use HTTP basic auth String userInfo = url.getUserInfo(); if (userInfo != null) { @@ -38,12 +59,13 @@ public class ConnectionUtil } /** - * Opens a connection to a http or https URL, setting the authentication header if user info - * is present. Throws a class cast exception if the connection returned is not the right type. + * Opens a connection to a http or https URL, setting the authentication header if user info is + * present. Throws a class cast exception if the connection returned is not the right type. See + * {@link #open} for parameter documentation. */ - public static HttpURLConnection openHttp (URL url) + public static HttpURLConnection openHttp (URL url, int connectTimeout, int readTimeout) throws IOException { - return (HttpURLConnection)open(url); + return (HttpURLConnection)open(url, connectTimeout, readTimeout); } }