From 6dee370878e7aad16882a12c6f47e2141ce023f8 Mon Sep 17 00:00:00 2001
From: Stefan Bodewig <bodewig@apache.org>
Date: Sun, 28 Feb 2021 17:45:52 +0100
Subject: [PATCH] make http condition follow redirects from http to https

Bugzilla Report 65105
---
 WHATSNEW                                      |  3 +
 .../org/apache/tools/ant/taskdefs/Get.java    | 19 +++--
 .../tools/ant/taskdefs/condition/Http.java    | 44 ++++++++++--
 .../antunit/taskdefs/condition/http-test.xml  | 71 +++++++++++++++++++
 4 files changed, 125 insertions(+), 12 deletions(-)
 create mode 100644 src/tests/antunit/taskdefs/condition/http-test.xml

diff --git a/WHATSNEW b/WHATSNEW
index be98daad1..35f8ec82a 100644
--- a/WHATSNEW
+++ b/WHATSNEW
@@ -27,6 +27,9 @@ Fixed bugs:
    central repository didn't contain any source files.
    Bugzilla Report 65110
 
+ * The <http> condition didn't follow redirects from http to https.
+   Bugzilla Report 65105
+
 Other changes:
 --------------
 
diff --git a/src/main/org/apache/tools/ant/taskdefs/Get.java b/src/main/org/apache/tools/ant/taskdefs/Get.java
index e7d56d71d..5ee3fb6e9 100644
--- a/src/main/org/apache/tools/ant/taskdefs/Get.java
+++ b/src/main/org/apache/tools/ant/taskdefs/Get.java
@@ -533,6 +533,18 @@ public class Get extends Task {
         extends org.apache.tools.ant.util.Base64Converter {
     }
 
+    /**
+     * Does the response code represent a redirection?
+     *
+     * @since 1.10.10
+     */
+    public static boolean isMoved(final int responseCode) {
+        return responseCode == HttpURLConnection.HTTP_MOVED_PERM
+            || responseCode == HttpURLConnection.HTTP_MOVED_TEMP
+            || responseCode == HttpURLConnection.HTTP_SEE_OTHER
+            || responseCode == HTTP_MOVED_TEMP;
+    }
+
     /**
      * Interface implemented for reporting
      * progress of downloading.
@@ -815,13 +827,6 @@ public class Get extends Task {
             return connection;
         }
 
-        private boolean isMoved(final int responseCode) {
-            return responseCode == HttpURLConnection.HTTP_MOVED_PERM
-                    || responseCode == HttpURLConnection.HTTP_MOVED_TEMP
-                    || responseCode == HttpURLConnection.HTTP_SEE_OTHER
-                    || responseCode == HTTP_MOVED_TEMP;
-        }
-
         private boolean downloadFile() throws IOException {
             for (int i = 0; i < numberRetries; i++) {
                 // this three attempt trick is to get round quirks in different
diff --git a/src/main/org/apache/tools/ant/taskdefs/condition/Http.java b/src/main/org/apache/tools/ant/taskdefs/condition/Http.java
index 1219fabd7..a047decc5 100644
--- a/src/main/org/apache/tools/ant/taskdefs/condition/Http.java
+++ b/src/main/org/apache/tools/ant/taskdefs/condition/Http.java
@@ -30,6 +30,7 @@ import java.util.Locale;
 import org.apache.tools.ant.BuildException;
 import org.apache.tools.ant.Project;
 import org.apache.tools.ant.ProjectComponent;
+import org.apache.tools.ant.taskdefs.Get;
 
 /**
  * Condition to wait for a HTTP request to succeed. Its attribute(s) are:
@@ -42,6 +43,8 @@ import org.apache.tools.ant.ProjectComponent;
 public class Http extends ProjectComponent implements Condition {
     private static final int ERROR_BEGINS = 400;
     private static final String DEFAULT_REQUEST_METHOD = "GET";
+    private static final String HTTP = "http";
+    private static final String HTTPS = "https";
 
     private String spec = null;
     private String requestMethod = DEFAULT_REQUEST_METHOD;
@@ -124,11 +127,7 @@ public class Http extends ProjectComponent implements Condition {
             try {
                 URLConnection conn = url.openConnection();
                 if (conn instanceof HttpURLConnection) {
-                    HttpURLConnection http = (HttpURLConnection) conn;
-                    http.setRequestMethod(requestMethod);
-                    http.setInstanceFollowRedirects(followRedirects);
-                    http.setReadTimeout(readTimeout);
-                    int code = http.getResponseCode();
+                    int code = request((HttpURLConnection) conn, url);
                     log("Result code for " + spec + " was " + code,
                         Project.MSG_VERBOSE);
                     return code > 0 && code < errorsBeginAt;
@@ -144,4 +143,39 @@ public class Http extends ProjectComponent implements Condition {
         }
         return true;
     }
+
+    private int request(final HttpURLConnection http, final URL url) throws IOException {
+        http.setRequestMethod(requestMethod);
+        http.setInstanceFollowRedirects(followRedirects);
+        http.setReadTimeout(readTimeout);
+        final int firstStatusCode = http.getResponseCode();
+        if (Get.isMoved(firstStatusCode)) {
+            final String newLocation = http.getHeaderField("Location");
+            final URL newURL = new URL(newLocation);
+            if (redirectionAllowed(url, newURL)) {
+                final URLConnection newConn = newURL.openConnection();
+                if (newConn instanceof HttpURLConnection) {
+                    log("Following redirect from " + url + " to " + newURL);
+                    return request((HttpURLConnection) newConn, newURL);
+                }
+            }
+        }
+        return firstStatusCode;
+    }
+
+    private boolean redirectionAllowed(final URL from, final URL to) {
+        if (from.equals(to)) {
+            // most simple case of an infinite redirect loop
+            return false;
+        }
+        if (!(from.getProtocol().equals(to.getProtocol())
+              || (HTTP.equals(from.getProtocol())
+                  && HTTPS.equals(to.getProtocol())))) {
+            log("Redirection detected from "
+                + from.getProtocol() + " to " + to.getProtocol()
+                + ". Protocol switch unsafe, not allowed.");
+            return false;
+        }
+        return true;
+    }
 }
diff --git a/src/tests/antunit/taskdefs/condition/http-test.xml b/src/tests/antunit/taskdefs/condition/http-test.xml
new file mode 100644
index 000000000..91ec866bd
--- /dev/null
+++ b/src/tests/antunit/taskdefs/condition/http-test.xml
@@ -0,0 +1,71 @@
+<?xml version="1.0"?>
+<!--
+  Licensed to the Apache Software Foundation (ASF) under one or more
+  contributor license agreements.  See the NOTICE file distributed with
+  this work for additional information regarding copyright ownership.
+  The ASF licenses this file to You under the Apache License, Version 2.0
+  (the "License"); you may not use this file except in compliance with
+  the License.  You may obtain a copy of the License at
+
+      https://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS,
+  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  See the License for the specific language governing permissions and
+  limitations under the License.
+-->
+
+<project name="http-test" default="antunit" xmlns:au="antlib:org.apache.ant.antunit">
+  <import file="../../antunit-base.xml" />
+
+  <property name="location" value="https://ant.apache.org/webtest/gettest" />
+  <property name="unsecurelocation" value="http://ant.apache.org/webtest/gettest/http-to-https.txt" />
+
+  <target name="testSeeOtherRedirect">
+    <sleep milliseconds="250"/>
+    <au:assertTrue>
+      <http url="${location}/other.txt"/>
+    </au:assertTrue>
+    <au:assertLogContains level="verbose"
+        text="Result code for ${location}/other.txt was 200" />
+  </target>
+
+  <target name="testPermanentRedirect">
+    <sleep milliseconds="250"/>
+    <au:assertTrue>
+      <http url="${location}/permanent.txt"/>
+    </au:assertTrue>
+    <au:assertLogContains level="verbose"
+        text="Result code for ${location}/permanent.txt was 200" />
+  </target>
+
+  <target name="testTemporaryRedirect">
+    <sleep milliseconds="250"/>
+    <au:assertTrue>
+      <http url="${location}/temp.txt"/>
+    </au:assertTrue>
+    <au:assertLogContains level="verbose"
+        text="Result code for ${location}/temp.txt was 200" />
+  </target>
+
+  <target name="testStatusCode307Redirect">
+    <sleep milliseconds="250"/>
+    <au:assertTrue>
+      <http url="${location}/307.txt"/>
+    </au:assertTrue>
+    <au:assertLogContains level="verbose"
+        text="Result code for ${location}/307.txt was 200" />
+  </target>
+
+  <target name="testHttpToHttpsRedirect" description="Tests that a resource that's redirected
+        from HTTP to HTTPS works without an error. See bugzilla-65105 for details">
+    <sleep milliseconds="250"/>
+    <au:assertTrue>
+      <http url="${unsecurelocation}"/>
+    </au:assertTrue>
+    <au:assertLogContains level="verbose"
+        text="Result code for ${unsecurelocation} was 200" />
+  </target>
+
+</project>