From affd010418cbe65cf87d17cbd80f0096ec9df387 Mon Sep 17 00:00:00 2001 From: Spaceman1984 <49917670+Spaceman1984@users.noreply.github.com> Date: Wed, 15 Jul 2020 12:29:18 +0200 Subject: [PATCH] templates: Fixed incorrect error message on invalid template type download (#4138) When a template is downloaded, the first 1MB of the template is validated to determine if the template is of correct file type. On failure, the download is aborted and the input stream is set to null. This leads to a second error when the try-with-resources block tries to auto-close the stream and throws an ioexception. The "stream closed" error message is then written to the db. This PR checks if an error has been stored before setting a new error message. Fixes: #4127 --- .../template/HttpTemplateDownloader.java | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) mode change 100644 => 100755 core/src/main/java/com/cloud/storage/template/HttpTemplateDownloader.java diff --git a/core/src/main/java/com/cloud/storage/template/HttpTemplateDownloader.java b/core/src/main/java/com/cloud/storage/template/HttpTemplateDownloader.java old mode 100644 new mode 100755 index d3c23a1fddb..35726274654 --- a/core/src/main/java/com/cloud/storage/template/HttpTemplateDownloader.java +++ b/core/src/main/java/com/cloud/storage/template/HttpTemplateDownloader.java @@ -27,6 +27,7 @@ import java.net.URI; import java.net.URISyntaxException; import java.util.Date; +import com.cloud.utils.exception.CloudRuntimeException; import org.apache.cloudstack.utils.imagestore.ImageStoreUtil; import org.apache.commons.httpclient.Credentials; import org.apache.commons.httpclient.Header; @@ -63,7 +64,7 @@ public class HttpTemplateDownloader extends ManagedContextRunnable implements Te private String downloadUrl; private String toFile; public TemplateDownloader.Status status; - public String errorString = " "; + private String errorString = null; private long remoteSize = 0; public long downloadTime = 0; public long totalBytes; @@ -218,7 +219,10 @@ public class HttpTemplateDownloader extends ManagedContextRunnable implements Te errorString = hte.getMessage(); } catch (IOException ioe) { status = TemplateDownloader.Status.UNRECOVERABLE_ERROR; //probably a file write error? - errorString = ioe.getMessage(); + // Let's not overwrite the original error message. + if (errorString == null) { + errorString = ioe.getMessage(); + } } finally { if (status == Status.UNRECOVERABLE_ERROR && file.exists() && !file.isDirectory()) { file.delete(); @@ -243,7 +247,6 @@ public class HttpTemplateDownloader extends ManagedContextRunnable implements Te 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; @@ -443,7 +446,7 @@ public class HttpTemplateDownloader extends ManagedContextRunnable implements Te @Override public String getDownloadError() { - return errorString; + return errorString == null ? " " : errorString; } @Override @@ -495,7 +498,6 @@ public class HttpTemplateDownloader extends ManagedContextRunnable implements Te } private class VerifyFormat { - private boolean invalidFormat; private File file; private boolean verifiedFormat; @@ -504,10 +506,6 @@ public class HttpTemplateDownloader extends ManagedContextRunnable implements Te this.verifiedFormat = false; } - boolean isInvalid() { - return invalidFormat; - } - public boolean isVerifiedFormat() { return verifiedFormat; } @@ -529,11 +527,10 @@ public class HttpTemplateDownloader extends ManagedContextRunnable implements Te } status = Status.UNRECOVERABLE_ERROR; errorString = "Template content is unsupported, or mismatch between selected format and template content. Found : " + unsupportedFormat; - invalidFormat = true; + throw new CloudRuntimeException(errorString); } else { s_logger.debug("Verified format of downloading file " + file.getAbsolutePath() + " is supported"); verifiedFormat = true; - invalidFormat = false; } return this; }