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

Author: Spaceman1984

core/src/main/java/com/cloud/storage/template/HttpTemplateDownloader.java

@@ -27,6 +27,7 @@
 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 long download(boolean resume, DownloadCompleteCallback callback) {
             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 @@ private boolean copyBytes(File file, InputStream in, RandomAccessFile out) throw
                 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 boolean isResume() {
 
     @Override
     public String getDownloadError() {
-        return errorString;
+        return errorString == null ? " " : errorString;
     }
 
     @Override
@@ -495,7 +498,6 @@ public ResourceType getResourceType() {
     }
 
     private class VerifyFormat {
-        private boolean invalidFormat;
         private File file;
         private boolean verifiedFormat;
 
@@ -504,10 +506,6 @@ public VerifyFormat(File file) {
             this.verifiedFormat = false;
         }
 
-        boolean isInvalid() {
-            return invalidFormat;
-        }
-
         public boolean isVerifiedFormat() {
             return verifiedFormat;
         }
@@ -529,11 +527,10 @@ public VerifyFormat invoke() {
                 }
                 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;
         }