From d4071f9eb63cf6114580ec13036f8f0bf39c6e32 Mon Sep 17 00:00:00 2001 From: Jacobus Martinus Kruithof Date: Fri, 10 Jul 2009 17:37:04 +0000 Subject: [PATCH] Also allow redirect from http to https a subset of what was requested in pr 47433. Tests from https to http v.v. have also been executed I couldn't find a https space for public testcases though. git-svn-id: https://svn.apache.org/repos/asf/ant/core/trunk@793048 13f79535-47bb-0310-9956-ffa450edef68 --- WHATSNEW | 3 + docs/webtest/gettest/.htaccess | 10 + docs/webtest/gettest/testother.txt | 1 + docs/webtest/gettest/testperm.txt | 1 + docs/webtest/gettest/testredir5.txt | 1 + docs/webtest/gettest/testtemp.txt | 1 + .../org/apache/tools/ant/taskdefs/Get.java | 191 +++++++++++++----- src/tests/antunit/taskdefs/get-test.xml | 90 +++++++++ 8 files changed, 245 insertions(+), 53 deletions(-) create mode 100644 docs/webtest/gettest/.htaccess create mode 100644 docs/webtest/gettest/testother.txt create mode 100644 docs/webtest/gettest/testperm.txt create mode 100644 docs/webtest/gettest/testredir5.txt create mode 100644 docs/webtest/gettest/testtemp.txt create mode 100644 src/tests/antunit/taskdefs/get-test.xml diff --git a/WHATSNEW b/WHATSNEW index 4c4d57b7f..7e87a3c4b 100644 --- a/WHATSNEW +++ b/WHATSNEW @@ -394,6 +394,9 @@ Fixed bugs: Other changes: -------------- + * The get task now also follows redirects from http to https + Bugzilla report 47433 + * A HostInfo task was added performing information on hosts, including info on the host ant is running on. Bugzilla reports 45861 and 31164. diff --git a/docs/webtest/gettest/.htaccess b/docs/webtest/gettest/.htaccess new file mode 100644 index 000000000..262439ff9 --- /dev/null +++ b/docs/webtest/gettest/.htaccess @@ -0,0 +1,10 @@ +Redirect seeother /webtest/gettest/other.txt /webtest/gettest/testother.txt +Redirect temp /webtest/gettest/temp.txt /webtest/gettest/testtemp.txt +Redirect permanent /webtest/gettest/permanent.txt /webtest/gettest/testperm.txt +Redirect /webtest/gettest/infinite.txt /webtest/gettest/infinite2.txt +Redirect /webtest/gettest/infinite2.txt /webtest/gettest/infinite.txt +Redirect /webtest/gettest/redir5.txt /webtest/gettest/redir5-1.txt +Redirect /webtest/gettest/redir5-1.txt /webtest/gettest/redir5-2.txt +Redirect /webtest/gettest/redir5-2.txt /webtest/gettest/redir5-3.txt +Redirect /webtest/gettest/redir5-3.txt /webtest/gettest/redir5-4.txt +Redirect /webtest/gettest/redir5-4.txt /webtest/gettest/testredir5.txt diff --git a/docs/webtest/gettest/testother.txt b/docs/webtest/gettest/testother.txt new file mode 100644 index 000000000..0387eb0a4 --- /dev/null +++ b/docs/webtest/gettest/testother.txt @@ -0,0 +1 @@ +seeother redirect succeeded diff --git a/docs/webtest/gettest/testperm.txt b/docs/webtest/gettest/testperm.txt new file mode 100644 index 000000000..d16174652 --- /dev/null +++ b/docs/webtest/gettest/testperm.txt @@ -0,0 +1 @@ +permanent redirect succeeded diff --git a/docs/webtest/gettest/testredir5.txt b/docs/webtest/gettest/testredir5.txt new file mode 100644 index 000000000..507b4390d --- /dev/null +++ b/docs/webtest/gettest/testredir5.txt @@ -0,0 +1 @@ +5 levels of redirect succeeded diff --git a/docs/webtest/gettest/testtemp.txt b/docs/webtest/gettest/testtemp.txt new file mode 100644 index 000000000..d036b86fe --- /dev/null +++ b/docs/webtest/gettest/testtemp.txt @@ -0,0 +1 @@ +temporary redirect succeeded diff --git a/src/main/org/apache/tools/ant/taskdefs/Get.java b/src/main/org/apache/tools/ant/taskdefs/Get.java index 71f393843..ba71cf3e4 100644 --- a/src/main/org/apache/tools/ant/taskdefs/Get.java +++ b/src/main/org/apache/tools/ant/taskdefs/Get.java @@ -18,12 +18,8 @@ package org.apache.tools.ant.taskdefs; -import org.apache.tools.ant.BuildException; -import org.apache.tools.ant.Project; -import org.apache.tools.ant.Task; -import org.apache.tools.ant.util.FileUtils; - import java.io.File; +import java.io.FileNotFoundException; import java.io.FileOutputStream; import java.io.IOException; import java.io.InputStream; @@ -33,6 +29,13 @@ import java.net.HttpURLConnection; import java.net.URL; import java.net.URLConnection; import java.util.Date; +import java.util.HashSet; +import java.util.Set; + +import org.apache.tools.ant.BuildException; +import org.apache.tools.ant.Project; +import org.apache.tools.ant.Task; +import org.apache.tools.ant.util.FileUtils; /** * Gets a particular file from a URL source. @@ -49,6 +52,10 @@ public class Get extends Task { private static final int DOTS_PER_LINE = 50; private static final int BIG_BUFFER_SIZE = 100 * 1024; private static final FileUtils FILE_UTILS = FileUtils.getFileUtils(); + private static final int REDIRECT_LIMIT = 25; + + private static final String HTTP = "http"; + private static final String HTTPS = "https"; private URL source; // required private File dest; // required @@ -361,6 +368,7 @@ public class Get extends Task { } private class GetThread extends Thread { + private final boolean hasTimestamp; private final long timestamp; private final DownloadProgress progress; @@ -371,6 +379,8 @@ public class Get extends Task { private BuildException exception = null; private InputStream is = null; private OutputStream os = null; + private URLConnection connection; + private int redirections = 0; GetThread(boolean h, long t, DownloadProgress p, int l) { hasTimestamp = h; @@ -390,10 +400,64 @@ public class Get extends Task { } private boolean get() throws IOException, BuildException { - //set up the URL connection - URLConnection connection = source.openConnection(); - //modify the headers - //NB: things like user authentication could go in here too. + + connection = openConnection(source); + + if (connection == null) + { + return false; + } + + boolean downloadSucceeded = downloadFile(); + + //if (and only if) the use file time option is set, then + //the saved file now has its timestamp set to that of the + //downloaded file + if (downloadSucceeded && useTimestamp) { + updateTimeStamp(); + } + + return downloadSucceeded; + } + + + private boolean redirectionAllowed(URL aSource, URL aDest) { + if (!(aSource.getProtocol().equals(aDest.getProtocol()) || (HTTP + .equals(aSource.getProtocol()) && HTTPS.equals(aDest + .getProtocol())))) { + String message = "Redirection detected from " + + aSource.getProtocol() + " to " + aDest.getProtocol() + + ". Protocol switch unsafe, not allowed."; + if (ignoreErrors) { + log(message, logLevel); + return false; + } else { + throw new BuildException(message); + } + } + + redirections++; + if (redirections > REDIRECT_LIMIT) { + String message = "More than " + REDIRECT_LIMIT + + " times redirected, giving up"; + if (ignoreErrors) { + log(message, logLevel); + return false; + } else { + throw new BuildException(message); + } + } + + + return true; + } + + private URLConnection openConnection(URL aSource) throws IOException { + + // set up the URL connection + URLConnection connection = aSource.openConnection(); + // modify the headers + // NB: things like user authentication could go in here too. if (hasTimestamp) { connection.setIfModifiedSince(timestamp); } @@ -401,45 +465,65 @@ public class Get extends Task { if (uname != null || pword != null) { String up = uname + ":" + pword; String encoding; - //we do not use the sun impl for portability, - //and always use our own implementation for consistent - //testing + // we do not use the sun impl for portability, + // and always use our own implementation for consistent + // testing Base64Converter encoder = new Base64Converter(); encoding = encoder.encode(up.getBytes()); - connection.setRequestProperty ("Authorization", - "Basic " + encoding); + connection.setRequestProperty("Authorization", "Basic " + + encoding); } - //connect to the remote site (may take some time) + if (connection instanceof HttpURLConnection) { + ((HttpURLConnection) connection) + .setInstanceFollowRedirects(false); + } + // connect to the remote site (may take some time) connection.connect(); - //next test for a 304 result (HTTP only) + + // First check on a 301 / 302 (moved) response (HTTP only) if (connection instanceof HttpURLConnection) { - HttpURLConnection httpConnection - = (HttpURLConnection) connection; + HttpURLConnection httpConnection = (HttpURLConnection) connection; + // httpConnection.setInstanceFollowRedirects(false); + // httpConnection.setUseCaches(false); + int responseCode = httpConnection.getResponseCode(); + if (responseCode == HttpURLConnection.HTTP_MOVED_PERM || + responseCode == HttpURLConnection.HTTP_MOVED_TEMP || + responseCode == HttpURLConnection.HTTP_SEE_OTHER) + { + String newLocation = httpConnection.getHeaderField("Location"); + String message = aSource + + (responseCode == HttpURLConnection.HTTP_MOVED_PERM ? " permanently" + : "") + " moved to " + newLocation; + log(message, logLevel); + URL newURL = new URL(newLocation); + if (!redirectionAllowed(aSource, newURL)) + { + return null; + } + return openConnection(newURL); + } + // next test for a 304 result (HTTP only) long lastModified = httpConnection.getLastModified(); - if (httpConnection.getResponseCode() - == HttpURLConnection.HTTP_NOT_MODIFIED - || (lastModified != 0 && hasTimestamp - && timestamp >= lastModified)) { - //not modified so no file download. just return - //instead and trace out something so the user - //doesn't think that the download happened when it - //didn't + if (responseCode == HttpURLConnection.HTTP_NOT_MODIFIED + || (lastModified != 0 && hasTimestamp && timestamp >= lastModified)) { + // not modified so no file download. just return + // instead and trace out something so the user + // doesn't think that the download happened when it + // didn't log("Not modified - so not downloaded", logLevel); - return false; + return null; } // test for 401 result (HTTP only) - if (httpConnection.getResponseCode() - == HttpURLConnection.HTTP_UNAUTHORIZED) { + if (responseCode == HttpURLConnection.HTTP_UNAUTHORIZED) { String message = "HTTP Authorization failure"; if (ignoreErrors) { log(message, logLevel); - return false; + return null; } else { throw new BuildException(message); } } - } //REVISIT: at this point even non HTTP connections may @@ -447,11 +531,15 @@ public class Get extends Task { //the date of the content and skip the write if it is not //newer. Some protocols (FTP) don't include dates, of //course. + return connection; + } + private boolean downloadFile() + throws FileNotFoundException, IOException { for (int i = 0; i < NUMBER_RETRIES; i++) { - //this three attempt trick is to get round quirks in different - //Java implementations. Some of them take a few goes to bind - //property; we ignore the first couple of such failures. + // this three attempt trick is to get round quirks in different + // Java implementations. Some of them take a few goes to bind + // property; we ignore the first couple of such failures. try { is = connection.getInputStream(); break; @@ -464,8 +552,8 @@ public class Get extends Task { if (ignoreErrors) { return false; } - throw new BuildException("Can't get " + source + " to " - + dest, getLocation()); + throw new BuildException("Can't get " + source + " to " + dest, + getLocation()); } os = new FileOutputStream(dest); @@ -491,26 +579,23 @@ public class Get extends Task { } } progress.endDownload(); - - //if (and only if) the use file time option is set, then - //the saved file now has its timestamp set to that of the - //downloaded file - if (useTimestamp) { - long remoteTimestamp = connection.getLastModified(); - if (verbose) { - Date t = new Date(remoteTimestamp); - log("last modified = " + t.toString() - + ((remoteTimestamp == 0) - ? " - using current time instead" - : ""), logLevel); - } - if (remoteTimestamp != 0) { - FILE_UTILS.setFileLastModified(dest, remoteTimestamp); - } - } return true; } + private void updateTimeStamp() { + long remoteTimestamp = connection.getLastModified(); + if (verbose) { + Date t = new Date(remoteTimestamp); + log("last modified = " + t.toString() + + ((remoteTimestamp == 0) + ? " - using current time instead" + : ""), logLevel); + } + if (remoteTimestamp != 0) { + FILE_UTILS.setFileLastModified(dest, remoteTimestamp); + } + } + /** * Has the download completed successfully? * diff --git a/src/tests/antunit/taskdefs/get-test.xml b/src/tests/antunit/taskdefs/get-test.xml new file mode 100644 index 000000000..216eb55e8 --- /dev/null +++ b/src/tests/antunit/taskdefs/get-test.xml @@ -0,0 +1,90 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +