You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by da...@apache.org on 2017/10/10 17:56:04 UTC
[cloudstack] branch master updated: CLOUDSTACK-9899 Url validation
disabling (#2074)
This is an automated email from the ASF dual-hosted git repository.
dahn pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
The following commit(s) were added to refs/heads/master by this push:
new 7ca5b53 CLOUDSTACK-9899 Url validation disabling (#2074)
7ca5b53 is described below
commit 7ca5b535a4a0ed573d9df60d28c95830fe13c119
Author: dahn <da...@gmail.com>
AuthorDate: Tue Oct 10 19:56:00 2017 +0200
CLOUDSTACK-9899 Url validation disabling (#2074)
* CLOUDSTACK-9899 adding a global setting for not checking URLs from the MS
* CLOUDSTACK-9899 refactor HttpTemplateDownloader contructor cleanup
* CLOUDSTACK-9899 refactor HttpTemplateDownloader.download() cleanup
* CLOUDSTACK-9899 add the new config key to configurable
* CLOUDSTACK-9899 refactor download method
* CLOUDSTACK-9899 less verbose setting comment
* CLOUDSTACK-9899 debug message to indicate checking happened
* CLOUDSTACK-9899 typi flase -> false
---
.../storage/template/HttpTemplateDownloader.java | 424 +++++++++++++--------
.../engine/orchestration/VolumeOrchestrator.java | 6 +-
.../com/cloud/storage/VolumeApiServiceImpl.java | 10 +-
3 files changed, 268 insertions(+), 172 deletions(-)
diff --git a/core/src/com/cloud/storage/template/HttpTemplateDownloader.java b/core/src/com/cloud/storage/template/HttpTemplateDownloader.java
index bdf4990..d3c23a1 100644
--- a/core/src/com/cloud/storage/template/HttpTemplateDownloader.java
+++ b/core/src/com/cloud/storage/template/HttpTemplateDownloader.java
@@ -62,7 +62,7 @@ public class HttpTemplateDownloader extends ManagedContextRunnable implements Te
private static final int CHUNK_SIZE = 1024 * 1024; //1M
private String downloadUrl;
private String toFile;
- public TemplateDownloader.Status status = TemplateDownloader.Status.NOT_STARTED;
+ public TemplateDownloader.Status status;
public String errorString = " ";
private long remoteSize = 0;
public long downloadTime = 0;
@@ -83,41 +83,38 @@ public class HttpTemplateDownloader extends ManagedContextRunnable implements Te
String user, String password, Proxy proxy, ResourceType resourceType) {
_storage = storageLayer;
this.downloadUrl = downloadUrl;
- setToDir(toDir);
- status = TemplateDownloader.Status.NOT_STARTED;
+ this.toDir = toDir;
this.resourceType = resourceType;
this.maxTemplateSizeInBytes = maxTemplateSizeInBytes;
+ completionCallback = callback;
+ status = TemplateDownloader.Status.NOT_STARTED;
totalBytes = 0;
client = new HttpClient(s_httpClientManager);
+ myretryhandler = createRetryTwiceHandler();
+ try {
+ request = createRequest(downloadUrl);
+ checkTemporaryDestination(toDir);
+ checkProxy(proxy);
+ checkCredentials(user, password);
+ } catch (Exception ex) {
+ errorString = "Unable to start download -- check url? ";
+ status = TemplateDownloader.Status.UNRECOVERABLE_ERROR;
+ s_logger.warn("Exception in constructor -- " + ex.toString());
+ } catch (Throwable th) {
+ s_logger.warn("throwable caught ", th);
+ }
+ }
- myretryhandler = new HttpMethodRetryHandler() {
- @Override
- public boolean retryMethod(final HttpMethod method, final IOException exception, int executionCount) {
- if (executionCount >= 2) {
- // Do not retry if over max retry count
- return false;
- }
- if (exception instanceof NoHttpResponseException) {
- // Retry if the server dropped connection on us
- return true;
- }
- if (!method.isRequestSent()) {
- // Retry if the request has not been sent fully or
- // if it's OK to retry methods that have been sent
- return true;
- }
- // otherwise do not retry
- return false;
- }
- };
+ private GetMethod createRequest(String downloadUrl) {
+ GetMethod request = new GetMethod(downloadUrl);
+ request.getParams().setParameter(HttpMethodParams.RETRY_HANDLER, myretryhandler);
+ request.setFollowRedirects(true);
+ return request;
+ }
+ private void checkTemporaryDestination(String toDir) {
try {
- request = new GetMethod(downloadUrl);
- request.getParams().setParameter(HttpMethodParams.RETRY_HANDLER, myretryhandler);
- completionCallback = callback;
- this.request.setFollowRedirects(true);
-
File f = File.createTempFile("dnld", "tmp_", new File(toDir));
if (_storage != null) {
@@ -125,15 +122,16 @@ public class HttpTemplateDownloader extends ManagedContextRunnable implements Te
}
toFile = f.getAbsolutePath();
- Pair<String, Integer> hostAndPort = UriUtils.validateUrl(downloadUrl);
+ } catch (IOException ex) {
+ errorString = "Unable to start download -- check url? ";
+ status = TemplateDownloader.Status.UNRECOVERABLE_ERROR;
+ s_logger.warn("Exception in constructor -- " + ex.toString());
+ }
+ }
- if (proxy != null) {
- client.getHostConfiguration().setProxy(proxy.getHost(), proxy.getPort());
- if (proxy.getUserName() != null) {
- Credentials proxyCreds = new UsernamePasswordCredentials(proxy.getUserName(), proxy.getPassword());
- client.getState().setProxyCredentials(AuthScope.ANY, proxyCreds);
- }
- }
+ private void checkCredentials(String user, String password) {
+ try {
+ Pair<String, Integer> hostAndPort = UriUtils.validateUrl(downloadUrl);
if ((user != null) && (password != null)) {
client.getParams().setAuthenticationPreemptive(true);
Credentials defaultcreds = new UsernamePasswordCredentials(user, password);
@@ -146,150 +144,74 @@ public class HttpTemplateDownloader extends ManagedContextRunnable implements Te
errorString = iae.getMessage();
status = TemplateDownloader.Status.UNRECOVERABLE_ERROR;
inited = false;
- } catch (Exception ex) {
- errorString = "Unable to start download -- check url? ";
- status = TemplateDownloader.Status.UNRECOVERABLE_ERROR;
- s_logger.warn("Exception in constructor -- " + ex.toString());
- } catch (Throwable th) {
- s_logger.warn("throwable caught ", th);
}
}
+ private void checkProxy(Proxy proxy) {
+ if (proxy != null) {
+ client.getHostConfiguration().setProxy(proxy.getHost(), proxy.getPort());
+ if (proxy.getUserName() != null) {
+ Credentials proxyCreds = new UsernamePasswordCredentials(proxy.getUserName(), proxy.getPassword());
+ client.getState().setProxyCredentials(AuthScope.ANY, proxyCreds);
+ }
+ }
+ }
+
+ private HttpMethodRetryHandler createRetryTwiceHandler() {
+ return new HttpMethodRetryHandler() {
+ @Override
+ public boolean retryMethod(final HttpMethod method, final IOException exception, int executionCount) {
+ if (executionCount >= 2) {
+ // Do not retry if over max retry count
+ return false;
+ }
+ if (exception instanceof NoHttpResponseException) {
+ // Retry if the server dropped connection on us
+ return true;
+ }
+ if (!method.isRequestSent()) {
+ // Retry if the request has not been sent fully or
+ // if it's OK to retry methods that have been sent
+ return true;
+ }
+ // otherwise do not retry
+ return false;
+ }
+ };
+ }
+
@Override
public long download(boolean resume, DownloadCompleteCallback callback) {
- switch (status) {
- case ABORTED:
- case UNRECOVERABLE_ERROR:
- case DOWNLOAD_FINISHED:
- return 0;
- default:
-
- }
+ if (skipDownloadOnStatus()) return 0;
int bytes = 0;
File file = new File(toFile);
try {
- long localFileSize = 0;
- if (file.exists() && resume) {
- localFileSize = file.length();
- s_logger.info("Resuming download to file (current size)=" + localFileSize);
- }
+ long localFileSize = checkLocalFileSizeForResume(resume, file);
Date start = new Date();
- int responseCode = 0;
+ if (checkServerResponse(localFileSize)) return 0;
- if (localFileSize > 0) {
- // require partial content support for resume
- request.addRequestHeader("Range", "bytes=" + localFileSize + "-");
- if (client.executeMethod(request) != HttpStatus.SC_PARTIAL_CONTENT) {
- errorString = "HTTP Server does not support partial get";
- status = TemplateDownloader.Status.UNRECOVERABLE_ERROR;
- return 0;
- }
- } else if ((responseCode = client.executeMethod(request)) != HttpStatus.SC_OK) {
- status = TemplateDownloader.Status.UNRECOVERABLE_ERROR;
- errorString = " HTTP Server returned " + responseCode + " (expected 200 OK) ";
- return 0; //FIXME: retry?
- }
+ if (!tryAndGetRemoteSize()) return 0;
- Header contentLengthHeader = request.getResponseHeader("Content-Length");
- boolean chunked = false;
- long remoteSize2 = 0;
- if (contentLengthHeader == null) {
- Header chunkedHeader = request.getResponseHeader("Transfer-Encoding");
- if (chunkedHeader == null || !"chunked".equalsIgnoreCase(chunkedHeader.getValue())) {
- status = TemplateDownloader.Status.UNRECOVERABLE_ERROR;
- errorString = " Failed to receive length of download ";
- return 0; //FIXME: what status do we put here? Do we retry?
- } else if ("chunked".equalsIgnoreCase(chunkedHeader.getValue())) {
- chunked = true;
- }
- } else {
- remoteSize2 = Long.parseLong(contentLengthHeader.getValue());
- if (remoteSize2 == 0) {
- status = TemplateDownloader.Status.DOWNLOAD_FINISHED;
- String downloaded = "(download complete remote=" + remoteSize + "bytes)";
- errorString = "Downloaded " + totalBytes + " bytes " + downloaded;
- downloadTime = 0;
- return 0;
- }
- }
+ if (!canHandleDownloadSize()) return 0;
- if (remoteSize == 0) {
- remoteSize = remoteSize2;
- }
+ checkAndSetDownloadSize();
- if (remoteSize > maxTemplateSizeInBytes) {
- s_logger.info("Remote size is too large: " + remoteSize + " , max=" + maxTemplateSizeInBytes);
- status = Status.UNRECOVERABLE_ERROR;
- errorString = "Download file size is too large";
- return 0;
- }
+ try (InputStream in = request.getResponseBodyAsStream();
+ RandomAccessFile out = new RandomAccessFile(file, "rw");
+ ) {
+ out.seek(localFileSize);
- if (remoteSize == 0) {
- remoteSize = maxTemplateSizeInBytes;
- }
+ s_logger.info("Starting download from " + downloadUrl + " to " + toFile + " remoteSize=" + remoteSize + " , max size=" + maxTemplateSizeInBytes);
- InputStream in = request.getResponseBodyAsStream();
-
- RandomAccessFile out = new RandomAccessFile(file, "rw");
- out.seek(localFileSize);
-
- s_logger.info("Starting download from " + getDownloadUrl() + " to " + toFile + " remoteSize=" + remoteSize + " , max size=" + maxTemplateSizeInBytes);
-
- byte[] block = new byte[CHUNK_SIZE];
- long offset = 0;
- boolean done = false;
- boolean verifiedFormat=false;
- status = TemplateDownloader.Status.IN_PROGRESS;
- while (!done && status != Status.ABORTED && offset <= remoteSize) {
- if ((bytes = in.read(block, 0, CHUNK_SIZE)) > -1) {
- out.write(block, 0, bytes);
- offset += bytes;
- out.seek(offset);
- totalBytes += bytes;
- if (!verifiedFormat && (offset >= 1048576 || offset >= remoteSize)) { //let's check format after we get 1MB or full file
- String uripath = null;
- try {
- URI str = new URI(getDownloadUrl());
- uripath = str.getPath();
- } catch (URISyntaxException e) {
- s_logger.warn("Invalid download url: " + getDownloadUrl() + ", This should not happen since we have validated the url before!!");
- }
- String unsupportedFormat = ImageStoreUtil.checkTemplateFormat(file.getAbsolutePath(), uripath);
- if (unsupportedFormat == null || !unsupportedFormat.isEmpty()) {
- try {
- request.abort();
- out.close();
- in.close();
- } catch (Exception ex) {
- s_logger.debug("Error on http connection : " + ex.getMessage());
- }
- status = Status.UNRECOVERABLE_ERROR;
- errorString = "Template content is unsupported, or mismatch between selected format and template content. Found : " + unsupportedFormat;
- return 0;
- }
- s_logger.debug("Verified format of downloading file " + file.getAbsolutePath() + " is supported");
- verifiedFormat = true;
- }
- } else {
- done = true;
- }
- }
- out.getFD().sync();
-
- Date finish = new Date();
- String downloaded = "(incomplete download)";
- if (totalBytes >= remoteSize) {
- status = TemplateDownloader.Status.DOWNLOAD_FINISHED;
- downloaded = "(download complete remote=" + remoteSize + "bytes)";
- }
- errorString = "Downloaded " + totalBytes + " bytes " + downloaded;
- downloadTime += finish.getTime() - start.getTime();
- in.close();
- out.close();
+ if (copyBytes(file, in, out)) return 0;
+ Date finish = new Date();
+ checkDowloadCompletion();
+ downloadTime += finish.getTime() - start.getTime();
+ } finally { /* in.close() and out.close() */ }
return totalBytes;
} catch (HttpException hte) {
status = TemplateDownloader.Status.UNRECOVERABLE_ERROR;
@@ -309,6 +231,133 @@ public class HttpTemplateDownloader extends ManagedContextRunnable implements Te
return 0;
}
+ private boolean copyBytes(File file, InputStream in, RandomAccessFile out) throws IOException {
+ int bytes;
+ byte[] block = new byte[CHUNK_SIZE];
+ long offset = 0;
+ boolean done = false;
+ VerifyFormat verifyFormat = new VerifyFormat(file);
+ status = Status.IN_PROGRESS;
+ while (!done && status != Status.ABORTED && offset <= remoteSize) {
+ if ((bytes = in.read(block, 0, CHUNK_SIZE)) > -1) {
+ offset = writeBlock(bytes, out, block, offset);
+ if (!verifyFormat.isVerifiedFormat() && (offset >= 1048576 || offset >= remoteSize)) { //let's check format after we get 1MB or full file
+ verifyFormat.invoke();
+ if (verifyFormat.isInvalid()) return true;
+ }
+ } else {
+ done = true;
+ }
+ }
+ out.getFD().sync();
+ return false;
+ }
+
+ private long writeBlock(int bytes, RandomAccessFile out, byte[] block, long offset) throws IOException {
+ out.write(block, 0, bytes);
+ offset += bytes;
+ out.seek(offset);
+ totalBytes += bytes;
+ return offset;
+ }
+
+ private void checkDowloadCompletion() {
+ String downloaded = "(incomplete download)";
+ if (totalBytes >= remoteSize) {
+ status = Status.DOWNLOAD_FINISHED;
+ downloaded = "(download complete remote=" + remoteSize + "bytes)";
+ }
+ errorString = "Downloaded " + totalBytes + " bytes " + downloaded;
+ }
+
+ private boolean canHandleDownloadSize() {
+ if (remoteSize > maxTemplateSizeInBytes) {
+ s_logger.info("Remote size is too large: " + remoteSize + " , max=" + maxTemplateSizeInBytes);
+ status = Status.UNRECOVERABLE_ERROR;
+ errorString = "Download file size is too large";
+ return false;
+ }
+
+ return true;
+ }
+
+ private void checkAndSetDownloadSize() {
+ if (remoteSize == 0) {
+ remoteSize = maxTemplateSizeInBytes;
+ }
+ }
+
+ private boolean tryAndGetRemoteSize() {
+ Header contentLengthHeader = request.getResponseHeader("Content-Length");
+ boolean chunked = false;
+ long reportedRemoteSize = 0;
+ if (contentLengthHeader == null) {
+ Header chunkedHeader = request.getResponseHeader("Transfer-Encoding");
+ if (chunkedHeader == null || !"chunked".equalsIgnoreCase(chunkedHeader.getValue())) {
+ status = Status.UNRECOVERABLE_ERROR;
+ errorString = " Failed to receive length of download ";
+ return false;
+ } else if ("chunked".equalsIgnoreCase(chunkedHeader.getValue())) {
+ chunked = true;
+ }
+ } else {
+ reportedRemoteSize = Long.parseLong(contentLengthHeader.getValue());
+ if (reportedRemoteSize == 0) {
+ status = Status.DOWNLOAD_FINISHED;
+ String downloaded = "(download complete remote=" + remoteSize + "bytes)";
+ errorString = "Downloaded " + totalBytes + " bytes " + downloaded;
+ downloadTime = 0;
+ return false;
+ }
+ }
+
+ if (remoteSize == 0) {
+ remoteSize = reportedRemoteSize;
+ }
+ return true;
+ }
+
+ private boolean checkServerResponse(long localFileSize) throws IOException {
+ int responseCode = 0;
+
+ if (localFileSize > 0) {
+ // require partial content support for resume
+ request.addRequestHeader("Range", "bytes=" + localFileSize + "-");
+ if (client.executeMethod(request) != HttpStatus.SC_PARTIAL_CONTENT) {
+ errorString = "HTTP Server does not support partial get";
+ status = Status.UNRECOVERABLE_ERROR;
+ return true;
+ }
+ } else if ((responseCode = client.executeMethod(request)) != HttpStatus.SC_OK) {
+ status = Status.UNRECOVERABLE_ERROR;
+ errorString = " HTTP Server returned " + responseCode + " (expected 200 OK) ";
+ return true; //FIXME: retry?
+ }
+ return false;
+ }
+
+ private long checkLocalFileSizeForResume(boolean resume, File file) {
+ // TODO check the status of this downloader as well?
+ long localFileSize = 0;
+ if (file.exists() && resume) {
+ localFileSize = file.length();
+ s_logger.info("Resuming download to file (current size)=" + localFileSize);
+ }
+ return localFileSize;
+ }
+
+ private boolean skipDownloadOnStatus() {
+ switch (status) {
+ case ABORTED:
+ case UNRECOVERABLE_ERROR:
+ case DOWNLOAD_FINISHED:
+ return true;
+ default:
+
+ }
+ return false;
+ }
+
public String getDownloadUrl() {
return downloadUrl;
}
@@ -407,19 +456,12 @@ public class HttpTemplateDownloader extends ManagedContextRunnable implements Te
this.resume = resume;
}
- public void setToDir(String toDir) {
- this.toDir = toDir;
- }
-
- public String getToDir() {
- return toDir;
- }
-
@Override
public long getMaxTemplateSizeInBytes() {
return maxTemplateSizeInBytes;
}
+ // TODO move this test code to unit tests or integration tests
public static void main(String[] args) {
String url = "http:// dev.mysql.com/get/Downloads/MySQL-5.0/mysql-noinstall-5.0.77-win32.zip/from/http://mirror.services.wisc.edu/mysql/";
try {
@@ -452,4 +494,48 @@ public class HttpTemplateDownloader extends ManagedContextRunnable implements Te
return resourceType;
}
+ private class VerifyFormat {
+ private boolean invalidFormat;
+ private File file;
+ private boolean verifiedFormat;
+
+ public VerifyFormat(File file) {
+ this.file = file;
+ this.verifiedFormat = false;
+ }
+
+ boolean isInvalid() {
+ return invalidFormat;
+ }
+
+ public boolean isVerifiedFormat() {
+ return verifiedFormat;
+ }
+
+ public VerifyFormat invoke() {
+ String uripath = null;
+ try {
+ URI str = new URI(downloadUrl);
+ uripath = str.getPath();
+ } catch (URISyntaxException e) {
+ s_logger.warn("Invalid download url: " + downloadUrl + ", This should not happen since we have validated the url before!!");
+ }
+ String unsupportedFormat = ImageStoreUtil.checkTemplateFormat(file.getAbsolutePath(), uripath);
+ if (unsupportedFormat == null || !unsupportedFormat.isEmpty()) {
+ try {
+ request.abort();
+ } catch (Exception ex) {
+ s_logger.debug("Error on http connection : " + ex.getMessage());
+ }
+ status = Status.UNRECOVERABLE_ERROR;
+ errorString = "Template content is unsupported, or mismatch between selected format and template content. Found : " + unsupportedFormat;
+ invalidFormat = true;
+ } else {
+ s_logger.debug("Verified format of downloading file " + file.getAbsolutePath() + " is supported");
+ verifiedFormat = true;
+ invalidFormat = false;
+ }
+ return this;
+ }
+ }
}
diff --git a/engine/orchestration/src/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java b/engine/orchestration/src/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java
index c039a2f..66928ba 100644
--- a/engine/orchestration/src/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java
+++ b/engine/orchestration/src/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java
@@ -1458,9 +1458,13 @@ public class VolumeOrchestrator extends ManagerBase implements VolumeOrchestrati
public static final ConfigKey<Boolean> StorageMigrationEnabled = new ConfigKey<Boolean>(Boolean.class, "enable.storage.migration", "Storage", "true",
"Enable/disable storage migration across primary storage", true);
+ static final ConfigKey<Boolean> VolumeUrlCheck = new ConfigKey<Boolean>("Advanced", Boolean.class, "volume.url.check", "true",
+ "Check the url for a volume before downloading it from the management server. Set to flase when you managment has no internet access.",
+ true);
+
@Override
public ConfigKey<?>[] getConfigKeys() {
- return new ConfigKey<?>[] {RecreatableSystemVmEnabled, MaxVolumeSize, StorageHAMigrationEnabled, StorageMigrationEnabled, CustomDiskOfferingMaxSize, CustomDiskOfferingMinSize};
+ return new ConfigKey<?>[] {RecreatableSystemVmEnabled, MaxVolumeSize, StorageHAMigrationEnabled, StorageMigrationEnabled, CustomDiskOfferingMaxSize, CustomDiskOfferingMinSize, VolumeUrlCheck};
}
@Override
diff --git a/server/src/com/cloud/storage/VolumeApiServiceImpl.java b/server/src/com/cloud/storage/VolumeApiServiceImpl.java
index 3330cc7..5461f31 100644
--- a/server/src/com/cloud/storage/VolumeApiServiceImpl.java
+++ b/server/src/com/cloud/storage/VolumeApiServiceImpl.java
@@ -260,6 +260,10 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
static final ConfigKey<Long> VmJobCheckInterval = new ConfigKey<Long>("Advanced", Long.class, "vm.job.check.interval", "3000",
"Interval in milliseconds to check if the job is complete", false);
+ static final ConfigKey<Boolean> VolumeUrlCheck = new ConfigKey<Boolean>("Advanced", Boolean.class, "volume.url.check", "true",
+ "Check the url for a volume before downloading it from the management server. Set to false when you managment has no internet access.",
+ true);
+
private long _maxVolumeSizeInGb;
private final StateMachine2<Volume.State, Volume.Event, Volume> _volStateMachine;
@@ -403,8 +407,10 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
throw new InvalidParameterValueException("File:// type urls are currently unsupported");
}
UriUtils.validateUrl(format, url);
- // check URL existence
- UriUtils.checkUrlExistence(url);
+ if (VolumeUrlCheck.value()) { // global setting that can be set when their MS does not have internet access
+ s_logger.debug("Checking url: " + url);
+ UriUtils.checkUrlExistence(url);
+ }
// Check that the resource limit for secondary storage won't be exceeded
_resourceLimitMgr.checkResourceLimit(_accountMgr.getAccount(ownerId), ResourceType.secondary_storage, UriUtils.getRemoteSize(url));
} else {
--
To stop receiving notification emails like this one, please contact
['"commits@cloudstack.apache.org" <co...@cloudstack.apache.org>'].