Skip to content

Commit b1642bc

Browse files
weizhouapacheweizhouapache
authored
engine-storage: control download redirection
Add a global setting to control whether redirection is allowed while downloading templates and volumes core: some changes on SimpleHttpMultiFileDownloader similar as HttpTemplateDownloader Signed-off-by: Abhishek Kumar <[email protected]>
1 parent b1e0bf9 commit b1642bc

File tree

46 files changed

+458
-124
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

46 files changed

+458
-124
lines changed

Diff for: ‎core/src/main/java/com/cloud/storage/template/HttpTemplateDownloader.java

+17-1
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import java.net.URI;
2929
import java.net.URISyntaxException;
3030
import java.util.Date;
31+
import java.util.List;
3132

3233
import org.apache.cloudstack.managed.context.ManagedContextRunnable;
3334
import org.apache.cloudstack.storage.command.DownloadCommand.ResourceType;
@@ -80,6 +81,7 @@ public class HttpTemplateDownloader extends ManagedContextRunnable implements Te
8081
private long maxTemplateSizeInBytes;
8182
private ResourceType resourceType = ResourceType.TEMPLATE;
8283
private final HttpMethodRetryHandler myretryhandler;
84+
private boolean followRedirects = false;
8385

8486
public HttpTemplateDownloader(StorageLayer storageLayer, String downloadUrl, String toDir, DownloadCompleteCallback callback, long maxTemplateSizeInBytes,
8587
String user, String password, Proxy proxy, ResourceType resourceType) {
@@ -111,7 +113,7 @@ public HttpTemplateDownloader(StorageLayer storageLayer, String downloadUrl, Str
111113
private GetMethod createRequest(String downloadUrl) {
112114
GetMethod request = new GetMethod(downloadUrl);
113115
request.getParams().setParameter(HttpMethodParams.RETRY_HANDLER, myretryhandler);
114-
request.setFollowRedirects(true);
116+
request.setFollowRedirects(followRedirects);
115117
return request;
116118
}
117119

@@ -337,6 +339,12 @@ private boolean checkServerResponse(long localFileSize) throws IOException {
337339
} else if ((responseCode = client.executeMethod(request)) != HttpStatus.SC_OK) {
338340
status = Status.UNRECOVERABLE_ERROR;
339341
errorString = " HTTP Server returned " + responseCode + " (expected 200 OK) ";
342+
if (List.of(HttpStatus.SC_MOVED_PERMANENTLY, HttpStatus.SC_MOVED_TEMPORARILY).contains(responseCode)
343+
&& !followRedirects) {
344+
errorString = String.format("Failed to download %s due to redirection, response code: %d",
345+
downloadUrl, responseCode);
346+
s_logger.error(errorString);
347+
}
340348
return true; //FIXME: retry?
341349
}
342350
return false;
@@ -538,4 +546,12 @@ public VerifyFormat invoke() {
538546
return this;
539547
}
540548
}
549+
550+
@Override
551+
public void setFollowRedirects(boolean followRedirects) {
552+
this.followRedirects = followRedirects;
553+
if (this.request != null) {
554+
this.request.setFollowRedirects(followRedirects);
555+
}
556+
}
541557
}

Diff for: ‎core/src/main/java/com/cloud/storage/template/MetalinkTemplateDownloader.java

+9-1
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ public MetalinkTemplateDownloader(StorageLayer storageLayer, String downloadUrl,
6060
protected GetMethod createRequest(String downloadUrl) {
6161
GetMethod request = new GetMethod(downloadUrl);
6262
request.getParams().setParameter(HttpMethodParams.RETRY_HANDLER, myretryhandler);
63-
request.setFollowRedirects(true);
63+
request.setFollowRedirects(followRedirects);
6464
if (!toFileSet) {
6565
String[] parts = downloadUrl.split("/");
6666
String filename = parts[parts.length - 1];
@@ -173,4 +173,12 @@ public Status getStatus() {
173173
public void setStatus(Status status) {
174174
this.status = status;
175175
}
176+
177+
@Override
178+
public void setFollowRedirects(boolean followRedirects) {
179+
super.setFollowRedirects(followRedirects);
180+
if (this.request != null) {
181+
this.request.setFollowRedirects(followRedirects);
182+
}
183+
}
176184
}

Diff for: ‎core/src/main/java/com/cloud/storage/template/S3TemplateDownloader.java

+15-4
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
import org.apache.cloudstack.storage.command.DownloadCommand.ResourceType;
3535
import org.apache.commons.httpclient.Header;
3636
import org.apache.commons.httpclient.HttpClient;
37+
import org.apache.commons.httpclient.HttpStatus;
3738
import org.apache.commons.httpclient.URIException;
3839
import org.apache.commons.httpclient.methods.GetMethod;
3940
import org.apache.commons.httpclient.params.HttpMethodParams;
@@ -44,6 +45,7 @@
4445
import java.io.IOException;
4546
import java.io.InputStream;
4647
import java.util.Date;
48+
import java.util.List;
4749

4850
import static com.cloud.utils.NumbersUtil.toHumanReadableSize;
4951
import static java.util.Arrays.asList;
@@ -72,8 +74,8 @@ public class S3TemplateDownloader extends ManagedContextRunnable implements Temp
7274
private long downloadTime;
7375
private long totalBytes;
7476
private long maxTemplateSizeInByte;
75-
7677
private boolean resume = false;
78+
private boolean followRedirects = false;
7779

7880
public S3TemplateDownloader(S3TO s3TO, String downloadUrl, String installPath, DownloadCompleteCallback downloadCompleteCallback,
7981
long maxTemplateSizeInBytes, String username, String password, Proxy proxy, ResourceType resourceType) {
@@ -91,7 +93,7 @@ public S3TemplateDownloader(S3TO s3TO, String downloadUrl, String installPath, D
9193
this.getMethod.getParams().setParameter(HttpMethodParams.RETRY_HANDLER, HTTPUtils.getHttpMethodRetryHandler(5));
9294

9395
// Follow redirects
94-
this.getMethod.setFollowRedirects(true);
96+
this.getMethod.setFollowRedirects(followRedirects);
9597

9698
// Set file extension.
9799
this.fileExtension = StringUtils.substringAfterLast(StringUtils.substringAfterLast(downloadUrl, "/"), ".");
@@ -124,10 +126,11 @@ public long download(boolean resume, DownloadCompleteCallback callback) {
124126
return 0;
125127
}
126128

127-
if (!HTTPUtils.verifyResponseCode(responseCode)) {
129+
boolean failedDueToRedirection = List.of(HttpStatus.SC_MOVED_PERMANENTLY,
130+
HttpStatus.SC_MOVED_TEMPORARILY).contains(responseCode) && !followRedirects;
131+
if (!HTTPUtils.verifyResponseCode(responseCode) || failedDueToRedirection) {
128132
errorString = "Response code for GetMethod of " + downloadUrl + " is incorrect, responseCode: " + responseCode;
129133
LOGGER.warn(errorString);
130-
131134
status = Status.UNRECOVERABLE_ERROR;
132135
return 0;
133136
}
@@ -373,4 +376,12 @@ public long getTotalBytes() {
373376
public String getFileExtension() {
374377
return fileExtension;
375378
}
379+
380+
@Override
381+
public void setFollowRedirects(boolean followRedirects) {
382+
this.followRedirects = followRedirects;
383+
if (this.getMethod != null) {
384+
this.getMethod.setFollowRedirects(followRedirects);
385+
}
386+
}
376387
}

Diff for: ‎core/src/main/java/com/cloud/storage/template/SimpleHttpMultiFileDownloader.java

+19-3
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import java.io.RandomAccessFile;
2626
import java.util.Date;
2727
import java.util.HashMap;
28+
import java.util.List;
2829
import java.util.Map;
2930

3031
import org.apache.cloudstack.managed.context.ManagedContextRunnable;
@@ -73,6 +74,7 @@ public class SimpleHttpMultiFileDownloader extends ManagedContextRunnable implem
7374
private final HttpMethodRetryHandler retryHandler;
7475

7576
private HashMap<String, String> urlFileMap;
77+
private boolean followRedirects = false;
7678

7779
public SimpleHttpMultiFileDownloader(StorageLayer storageLayer, String[] downloadUrls, String toDir,
7880
DownloadCompleteCallback callback, long maxTemplateSizeInBytes,
@@ -94,7 +96,7 @@ public SimpleHttpMultiFileDownloader(StorageLayer storageLayer, String[] downloa
9496
private GetMethod createRequest(String downloadUrl) {
9597
GetMethod request = new GetMethod(downloadUrl);
9698
request.getParams().setParameter(HttpMethodParams.RETRY_HANDLER, retryHandler);
97-
request.setFollowRedirects(true);
99+
request.setFollowRedirects(followRedirects);
98100
return request;
99101
}
100102

@@ -170,7 +172,7 @@ private long downloadFile(String downloadUrl) {
170172
urlFileMap.put(downloadUrl, currentToFile);
171173
file = new File(currentToFile);
172174
long localFileSize = checkLocalFileSizeForResume(resume, file);
173-
if (checkServerResponse(localFileSize)) return 0;
175+
if (checkServerResponse(localFileSize, downloadUrl)) return 0;
174176
if (!tryAndGetRemoteSize()) return 0;
175177
if (!canHandleDownloadSize()) return 0;
176178
checkAndSetDownloadSize();
@@ -317,7 +319,7 @@ private boolean tryAndGetRemoteSize() {
317319
return true;
318320
}
319321

320-
private boolean checkServerResponse(long localFileSize) throws IOException {
322+
private boolean checkServerResponse(long localFileSize, String downloadUrl) throws IOException {
321323
int responseCode = 0;
322324

323325
if (localFileSize > 0) {
@@ -331,6 +333,12 @@ private boolean checkServerResponse(long localFileSize) throws IOException {
331333
} else if ((responseCode = client.executeMethod(request)) != HttpStatus.SC_OK) {
332334
currentStatus = Status.UNRECOVERABLE_ERROR;
333335
errorString = " HTTP Server returned " + responseCode + " (expected 200 OK) ";
336+
if (List.of(HttpStatus.SC_MOVED_PERMANENTLY, HttpStatus.SC_MOVED_TEMPORARILY).contains(responseCode)
337+
&& !followRedirects) {
338+
errorString = String.format("Failed to download %s due to redirection, response code: %d",
339+
downloadUrl, responseCode);
340+
s_logger.error(errorString);
341+
}
334342
return true; //FIXME: retry?
335343
}
336344
return false;
@@ -478,4 +486,12 @@ public DownloadCommand.ResourceType getResourceType() {
478486
public Map<String, String> getDownloadedFilesMap() {
479487
return urlFileMap;
480488
}
489+
490+
@Override
491+
public void setFollowRedirects(boolean followRedirects) {
492+
this.followRedirects = followRedirects;
493+
if (this.request != null) {
494+
this.request.setFollowRedirects(followRedirects);
495+
}
496+
}
481497
}

Diff for: ‎core/src/main/java/com/cloud/storage/template/TemplateDownloader.java

+2
Original file line numberDiff line numberDiff line change
@@ -92,4 +92,6 @@ enum Status {
9292
boolean isInited();
9393

9494
long getMaxTemplateSizeInBytes();
95+
96+
void setFollowRedirects(boolean followRedirects);
9597
}

Diff for: ‎core/src/main/java/com/cloud/storage/template/TemplateDownloaderBase.java

+6
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ public abstract class TemplateDownloaderBase extends ManagedContextRunnable impl
4343
protected long _start;
4444
protected StorageLayer _storage;
4545
protected boolean _inited = false;
46+
protected boolean followRedirects = false;
4647
private long maxTemplateSizeInBytes;
4748

4849
public TemplateDownloaderBase(StorageLayer storage, String downloadUrl, String toDir, long maxTemplateSizeInBytes, DownloadCompleteCallback callback) {
@@ -149,4 +150,9 @@ public void setResume(boolean resume) {
149150
public boolean isInited() {
150151
return _inited;
151152
}
153+
154+
@Override
155+
public void setFollowRedirects(boolean followRedirects) {
156+
this.followRedirects = followRedirects;
157+
}
152158
}

Diff for: ‎core/src/main/java/org/apache/cloudstack/agent/directdownload/CheckUrlCommand.java

+9-2
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ public class CheckUrlCommand extends Command {
2828
private Integer connectTimeout;
2929
private Integer connectionRequestTimeout;
3030
private Integer socketTimeout;
31+
private boolean followRedirects;
3132

3233
public String getFormat() {
3334
return format;
@@ -43,19 +44,25 @@ public String getUrl() {
4344

4445
public Integer getSocketTimeout() { return socketTimeout; }
4546

46-
public CheckUrlCommand(final String format,final String url) {
47+
public boolean isFollowRedirects() {
48+
return followRedirects;
49+
}
50+
51+
public CheckUrlCommand(final String format, final String url, final boolean followRedirects) {
4752
super();
4853
this.format = format;
4954
this.url = url;
55+
this.followRedirects = followRedirects;
5056
}
5157

52-
public CheckUrlCommand(final String format,final String url, Integer connectTimeout, Integer connectionRequestTimeout, Integer socketTimeout) {
58+
public CheckUrlCommand(final String format,final String url, Integer connectTimeout, Integer connectionRequestTimeout, Integer socketTimeout, final boolean followRedirects) {
5359
super();
5460
this.format = format;
5561
this.url = url;
5662
this.connectTimeout = connectTimeout;
5763
this.socketTimeout = socketTimeout;
5864
this.connectionRequestTimeout = connectionRequestTimeout;
65+
this.followRedirects = followRedirects;
5966
}
6067

6168
@Override

Diff for: ‎core/src/main/java/org/apache/cloudstack/agent/directdownload/DirectDownloadCommand.java

+14-1
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,11 @@ public enum DownloadProtocol {
4545
private Long templateSize;
4646
private Storage.ImageFormat format;
4747

48-
protected DirectDownloadCommand (final String url, final Long templateId, final PrimaryDataStoreTO destPool, final String checksum, final Map<String, String> headers, final Integer connectTimeout, final Integer soTimeout, final Integer connectionRequestTimeout) {
48+
private boolean followRedirects;
49+
50+
protected DirectDownloadCommand (final String url, final Long templateId, final PrimaryDataStoreTO destPool,
51+
final String checksum, final Map<String, String> headers, final Integer connectTimeout,
52+
final Integer soTimeout, final Integer connectionRequestTimeout, final boolean followRedirects) {
4953
this.url = url;
5054
this.templateId = templateId;
5155
this.destData = destData;
@@ -55,6 +59,7 @@ protected DirectDownloadCommand (final String url, final Long templateId, final
5559
this.connectTimeout = connectTimeout;
5660
this.soTimeout = soTimeout;
5761
this.connectionRequestTimeout = connectionRequestTimeout;
62+
this.followRedirects = followRedirects;
5863
}
5964

6065
public String getUrl() {
@@ -137,4 +142,12 @@ public boolean executeInSequence() {
137142
public int getWaitInMillSeconds() {
138143
return getWait() * 1000;
139144
}
145+
146+
public boolean isFollowRedirects() {
147+
return followRedirects;
148+
}
149+
150+
public void setFollowRedirects(boolean followRedirects) {
151+
this.followRedirects = followRedirects;
152+
}
140153
}

Diff for: ‎core/src/main/java/org/apache/cloudstack/agent/directdownload/HttpDirectDownloadCommand.java

+4-2
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,10 @@
2424

2525
public class HttpDirectDownloadCommand extends DirectDownloadCommand {
2626

27-
public HttpDirectDownloadCommand(String url, Long templateId, PrimaryDataStoreTO destPool, String checksum, Map<String, String> headers, int connectTimeout, int soTimeout) {
28-
super(url, templateId, destPool, checksum, headers, connectTimeout, soTimeout, null);
27+
public HttpDirectDownloadCommand(String url, Long templateId, PrimaryDataStoreTO destPool, String checksum,
28+
Map<String, String> headers, int connectTimeout, int soTimeout, boolean followRedirects) {
29+
super(url, templateId, destPool, checksum, headers, connectTimeout, soTimeout,
30+
null, followRedirects);
2931
}
3032

3133
}

Diff for: ‎core/src/main/java/org/apache/cloudstack/agent/directdownload/HttpsDirectDownloadCommand.java

+5-2
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,10 @@
2525

2626
public class HttpsDirectDownloadCommand extends DirectDownloadCommand {
2727

28-
public HttpsDirectDownloadCommand(String url, Long templateId, PrimaryDataStoreTO destPool, String checksum, Map<String, String> headers, int connectTimeout, int soTimeout, int connectionRequestTimeout) {
29-
super(url, templateId, destPool, checksum, headers, connectTimeout, soTimeout, connectionRequestTimeout);
28+
public HttpsDirectDownloadCommand(String url, Long templateId, PrimaryDataStoreTO destPool, String checksum,
29+
Map<String, String> headers, int connectTimeout, int soTimeout, int connectionRequestTimeout,
30+
boolean followRedirects) {
31+
super(url, templateId, destPool, checksum, headers, connectTimeout, soTimeout,
32+
connectionRequestTimeout, followRedirects);
3033
}
3134
}

Diff for: ‎core/src/main/java/org/apache/cloudstack/agent/directdownload/MetalinkDirectDownloadCommand.java

+3-2
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,9 @@
2424

2525
public class MetalinkDirectDownloadCommand extends DirectDownloadCommand {
2626

27-
public MetalinkDirectDownloadCommand(String url, Long templateId, PrimaryDataStoreTO destPool, String checksum, Map<String, String> headers, int connectTimeout, int soTimeout) {
28-
super(url, templateId, destPool, checksum, headers, connectTimeout, soTimeout, null);
27+
public MetalinkDirectDownloadCommand(String url, Long templateId, PrimaryDataStoreTO destPool, String checksum,
28+
Map<String, String> headers, int connectTimeout, int soTimeout, boolean followRedirects) {
29+
super(url, templateId, destPool, checksum, headers, connectTimeout, soTimeout, null, followRedirects);
2930
}
3031

3132
}

Diff for: ‎core/src/main/java/org/apache/cloudstack/agent/directdownload/NfsDirectDownloadCommand.java

+3-2
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,9 @@
2424

2525
public class NfsDirectDownloadCommand extends DirectDownloadCommand {
2626

27-
public NfsDirectDownloadCommand(final String url, final Long templateId, final PrimaryDataStoreTO destPool, final String checksum, final Map<String, String> headers) {
28-
super(url, templateId, destPool, checksum, headers, null, null, null);
27+
public NfsDirectDownloadCommand(final String url, final Long templateId, final PrimaryDataStoreTO destPool,
28+
final String checksum, final Map<String, String> headers) {
29+
super(url, templateId, destPool, checksum, headers, null, null, null, false);
2930
}
3031

3132
}

0 commit comments

Comments
 (0)