Skip to content

Commit 6dee370

Browse files
bodewigbodewig
committed
make http condition follow redirects from http to https
Bugzilla Report 65105
1 parent 5f8c637 commit 6dee370

File tree

4 files changed

+125
-12
lines changed

4 files changed

+125
-12
lines changed

‎WHATSNEW

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,9 @@ Fixed bugs:
2727
central repository didn't contain any source files.
2828
Bugzilla Report 65110
2929

30+
* The <http> condition didn't follow redirects from http to https.
31+
Bugzilla Report 65105
32+
3033
Other changes:
3134
--------------
3235

‎src/main/org/apache/tools/ant/taskdefs/Get.java

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -533,6 +533,18 @@ protected static class Base64Converter
533533
extends org.apache.tools.ant.util.Base64Converter {
534534
}
535535

536+
/**
537+
* Does the response code represent a redirection?
538+
*
539+
* @since 1.10.10
540+
*/
541+
public static boolean isMoved(final int responseCode) {
542+
return responseCode == HttpURLConnection.HTTP_MOVED_PERM
543+
|| responseCode == HttpURLConnection.HTTP_MOVED_TEMP
544+
|| responseCode == HttpURLConnection.HTTP_SEE_OTHER
545+
|| responseCode == HTTP_MOVED_TEMP;
546+
}
547+
536548
/**
537549
* Interface implemented for reporting
538550
* progress of downloading.
@@ -815,13 +827,6 @@ private URLConnection openConnection(final URL aSource) throws IOException {
815827
return connection;
816828
}
817829

818-
private boolean isMoved(final int responseCode) {
819-
return responseCode == HttpURLConnection.HTTP_MOVED_PERM
820-
|| responseCode == HttpURLConnection.HTTP_MOVED_TEMP
821-
|| responseCode == HttpURLConnection.HTTP_SEE_OTHER
822-
|| responseCode == HTTP_MOVED_TEMP;
823-
}
824-
825830
private boolean downloadFile() throws IOException {
826831
for (int i = 0; i < numberRetries; i++) {
827832
// this three attempt trick is to get round quirks in different

‎src/main/org/apache/tools/ant/taskdefs/condition/Http.java

Lines changed: 39 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
import org.apache.tools.ant.BuildException;
3131
import org.apache.tools.ant.Project;
3232
import org.apache.tools.ant.ProjectComponent;
33+
import org.apache.tools.ant.taskdefs.Get;
3334

3435
/**
3536
* Condition to wait for a HTTP request to succeed. Its attribute(s) are:
@@ -42,6 +43,8 @@
4243
public class Http extends ProjectComponent implements Condition {
4344
private static final int ERROR_BEGINS = 400;
4445
private static final String DEFAULT_REQUEST_METHOD = "GET";
46+
private static final String HTTP = "http";
47+
private static final String HTTPS = "https";
4548

4649
private String spec = null;
4750
private String requestMethod = DEFAULT_REQUEST_METHOD;
@@ -124,11 +127,7 @@ public boolean eval() throws BuildException {
124127
try {
125128
URLConnection conn = url.openConnection();
126129
if (conn instanceof HttpURLConnection) {
127-
HttpURLConnection http = (HttpURLConnection) conn;
128-
http.setRequestMethod(requestMethod);
129-
http.setInstanceFollowRedirects(followRedirects);
130-
http.setReadTimeout(readTimeout);
131-
int code = http.getResponseCode();
130+
int code = request((HttpURLConnection) conn, url);
132131
log("Result code for " + spec + " was " + code,
133132
Project.MSG_VERBOSE);
134133
return code > 0 && code < errorsBeginAt;
@@ -144,4 +143,39 @@ public boolean eval() throws BuildException {
144143
}
145144
return true;
146145
}
146+
147+
private int request(final HttpURLConnection http, final URL url) throws IOException {
148+
http.setRequestMethod(requestMethod);
149+
http.setInstanceFollowRedirects(followRedirects);
150+
http.setReadTimeout(readTimeout);
151+
final int firstStatusCode = http.getResponseCode();
152+
if (Get.isMoved(firstStatusCode)) {
153+
final String newLocation = http.getHeaderField("Location");
154+
final URL newURL = new URL(newLocation);
155+
if (redirectionAllowed(url, newURL)) {
156+
final URLConnection newConn = newURL.openConnection();
157+
if (newConn instanceof HttpURLConnection) {
158+
log("Following redirect from " + url + " to " + newURL);
159+
return request((HttpURLConnection) newConn, newURL);
160+
}
161+
}
162+
}
163+
return firstStatusCode;
164+
}
165+
166+
private boolean redirectionAllowed(final URL from, final URL to) {
167+
if (from.equals(to)) {
168+
// most simple case of an infinite redirect loop
169+
return false;
170+
}
171+
if (!(from.getProtocol().equals(to.getProtocol())
172+
|| (HTTP.equals(from.getProtocol())
173+
&& HTTPS.equals(to.getProtocol())))) {
174+
log("Redirection detected from "
175+
+ from.getProtocol() + " to " + to.getProtocol()
176+
+ ". Protocol switch unsafe, not allowed.");
177+
return false;
178+
}
179+
return true;
180+
}
147181
}

‎src/tests/antunit/taskdefs/condition/http-test.xml

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
<?xml version="1.0"?>
2+
<!--
3+
Licensed to the Apache Software Foundation (ASF) under one or more
4+
contributor license agreements. See the NOTICE file distributed with
5+
this work for additional information regarding copyright ownership.
6+
The ASF licenses this file to You under the Apache License, Version 2.0
7+
(the "License"); you may not use this file except in compliance with
8+
the License. You may obtain a copy of the License at
9+
10+
https://www.apache.org/licenses/LICENSE-2.0
11+
12+
Unless required by applicable law or agreed to in writing, software
13+
distributed under the License is distributed on an "AS IS" BASIS,
14+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
See the License for the specific language governing permissions and
16+
limitations under the License.
17+
-->
18+
19+
<project name="http-test" default="antunit" xmlns:au="antlib:org.apache.ant.antunit">
20+
<import file="../../antunit-base.xml" />
21+
22+
<property name="location" value="https://ant.apache.org/webtest/gettest" />
23+
<property name="unsecurelocation" value="http://ant.apache.org/webtest/gettest/http-to-https.txt" />
24+
25+
<target name="testSeeOtherRedirect">
26+
<sleep milliseconds="250"/>
27+
<au:assertTrue>
28+
<http url="${location}/other.txt"/>
29+
</au:assertTrue>
30+
<au:assertLogContains level="verbose"
31+
text="Result code for ${location}/other.txt was 200" />
32+
</target>
33+
34+
<target name="testPermanentRedirect">
35+
<sleep milliseconds="250"/>
36+
<au:assertTrue>
37+
<http url="${location}/permanent.txt"/>
38+
</au:assertTrue>
39+
<au:assertLogContains level="verbose"
40+
text="Result code for ${location}/permanent.txt was 200" />
41+
</target>
42+
43+
<target name="testTemporaryRedirect">
44+
<sleep milliseconds="250"/>
45+
<au:assertTrue>
46+
<http url="${location}/temp.txt"/>
47+
</au:assertTrue>
48+
<au:assertLogContains level="verbose"
49+
text="Result code for ${location}/temp.txt was 200" />
50+
</target>
51+
52+
<target name="testStatusCode307Redirect">
53+
<sleep milliseconds="250"/>
54+
<au:assertTrue>
55+
<http url="${location}/307.txt"/>
56+
</au:assertTrue>
57+
<au:assertLogContains level="verbose"
58+
text="Result code for ${location}/307.txt was 200" />
59+
</target>
60+
61+
<target name="testHttpToHttpsRedirect" description="Tests that a resource that's redirected
62+
from HTTP to HTTPS works without an error. See bugzilla-65105 for details">
63+
<sleep milliseconds="250"/>
64+
<au:assertTrue>
65+
<http url="${unsecurelocation}"/>
66+
</au:assertTrue>
67+
<au:assertLogContains level="verbose"
68+
text="Result code for ${unsecurelocation} was 200" />
69+
</target>
70+
71+
</project>

0 commit comments

Comments
 (0)