You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by ra...@apache.org on 2015/03/20 11:57:27 UTC
git commit: updated refs/heads/volume-upload to 018023c
Repository: cloudstack
Updated Branches:
refs/heads/volume-upload d5dffb5dc -> 018023c1e
volume upload: added validation for file formats
merged TemplateUtils and ImageStoreUtil to a singe ImageStoreUtil
also added a unittest for ImageStoreUtil
Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo
Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/018023c1
Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/018023c1
Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/018023c1
Branch: refs/heads/volume-upload
Commit: 018023c1ef7fc5216a607e40811ff20f185d295c
Parents: d5dffb5
Author: Rajani Karuturi <ra...@gmail.com>
Authored: Thu Mar 19 11:54:51 2015 +0530
Committer: Rajani Karuturi <ra...@gmail.com>
Committed: Fri Mar 20 16:25:13 2015 +0530
----------------------------------------------------------------------
.../template/HttpTemplateDownloader.java | 4 +-
.../com/cloud/storage/VolumeApiServiceImpl.java | 2 +-
.../com/cloud/template/TemplateManagerImpl.java | 2 +-
.../resource/HttpUploadServerHandler.java | 10 ++
.../resource/NfsSecondaryStorageResource.java | 14 +++
utils/src/com/cloud/utils/ImageStoreUtil.java | 39 -------
.../utils/imagestore/ImageStoreUtil.java | 110 +++++++++++++++++++
.../utils/template/TemplateUtils.java | 97 ----------------
.../utils/imagestore/ImageStoreUtilTest.java | 38 +++++++
9 files changed, 176 insertions(+), 140 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/cloudstack/blob/018023c1/core/src/com/cloud/storage/template/HttpTemplateDownloader.java
----------------------------------------------------------------------
diff --git a/core/src/com/cloud/storage/template/HttpTemplateDownloader.java b/core/src/com/cloud/storage/template/HttpTemplateDownloader.java
index 5644af4..632a809 100644
--- a/core/src/com/cloud/storage/template/HttpTemplateDownloader.java
+++ b/core/src/com/cloud/storage/template/HttpTemplateDownloader.java
@@ -28,6 +28,7 @@ import java.net.URISyntaxException;
import java.net.URL;
import java.util.Date;
+import org.apache.cloudstack.utils.imagestore.ImageStoreUtil;
import org.apache.commons.httpclient.Credentials;
import org.apache.commons.httpclient.Header;
import org.apache.commons.httpclient.HttpClient;
@@ -45,7 +46,6 @@ import org.apache.log4j.Logger;
import org.apache.cloudstack.managed.context.ManagedContextRunnable;
import org.apache.cloudstack.storage.command.DownloadCommand.ResourceType;
-import org.apache.cloudstack.utils.template.TemplateUtils;
import com.cloud.agent.api.storage.Proxy;
import com.cloud.storage.StorageLayer;
@@ -259,7 +259,7 @@ public class HttpTemplateDownloader extends ManagedContextRunnable implements Te
} catch (URISyntaxException e) {
s_logger.warn("Invalid download url: " + getDownloadUrl() + ", This should not happen since we have validated the url before!!");
}
- String unsupportedFormat = TemplateUtils.checkTemplateFormat(file.getAbsolutePath(), uripath);
+ String unsupportedFormat = ImageStoreUtil.checkTemplateFormat(file.getAbsolutePath(), uripath);
if (unsupportedFormat == null || !unsupportedFormat.isEmpty()) {
try {
request.abort();
http://git-wip-us.apache.org/repos/asf/cloudstack/blob/018023c1/server/src/com/cloud/storage/VolumeApiServiceImpl.java
----------------------------------------------------------------------
diff --git a/server/src/com/cloud/storage/VolumeApiServiceImpl.java b/server/src/com/cloud/storage/VolumeApiServiceImpl.java
index 567a742..4ff7686 100644
--- a/server/src/com/cloud/storage/VolumeApiServiceImpl.java
+++ b/server/src/com/cloud/storage/VolumeApiServiceImpl.java
@@ -29,7 +29,6 @@ import java.util.concurrent.ExecutionException;
import javax.inject.Inject;
import com.cloud.utils.EncryptionUtil;
-import com.cloud.utils.ImageStoreUtil;
import com.cloud.utils.db.TransactionCallbackWithException;
import com.google.gson.Gson;
import com.google.gson.GsonBuilder;
@@ -39,6 +38,7 @@ import org.apache.cloudstack.api.response.GetUploadParamsResponse;
import org.apache.cloudstack.engine.subsystem.api.storage.DataObject;
import org.apache.cloudstack.engine.subsystem.api.storage.EndPoint;
import org.apache.cloudstack.storage.command.TemplateOrVolumePostUploadCommand;
+import org.apache.cloudstack.utils.imagestore.ImageStoreUtil;
import org.apache.log4j.Logger;
import org.apache.cloudstack.api.command.user.volume.AttachVolumeCmd;
import org.apache.cloudstack.api.command.user.volume.CreateVolumeCmd;
http://git-wip-us.apache.org/repos/asf/cloudstack/blob/018023c1/server/src/com/cloud/template/TemplateManagerImpl.java
----------------------------------------------------------------------
diff --git a/server/src/com/cloud/template/TemplateManagerImpl.java b/server/src/com/cloud/template/TemplateManagerImpl.java
index 9747ad0..10ff77f 100755
--- a/server/src/com/cloud/template/TemplateManagerImpl.java
+++ b/server/src/com/cloud/template/TemplateManagerImpl.java
@@ -35,13 +35,13 @@ import javax.naming.ConfigurationException;
import com.cloud.storage.ImageStoreUploadMonitorImpl;
import com.cloud.utils.EncryptionUtil;
-import com.cloud.utils.ImageStoreUtil;
import com.google.gson.Gson;
import com.google.gson.GsonBuilder;
import org.apache.cloudstack.api.command.user.template.GetUploadParamsForTemplateCmd;
import org.apache.cloudstack.api.response.GetUploadParamsResponse;
import org.apache.cloudstack.storage.command.TemplateOrVolumePostUploadCommand;
+import org.apache.cloudstack.utils.imagestore.ImageStoreUtil;
import org.apache.commons.collections.CollectionUtils;
import org.apache.log4j.Logger;
import org.apache.cloudstack.acl.SecurityChecker.AccessType;
http://git-wip-us.apache.org/repos/asf/cloudstack/blob/018023c1/services/secondary-storage/server/src/org/apache/cloudstack/storage/resource/HttpUploadServerHandler.java
----------------------------------------------------------------------
diff --git a/services/secondary-storage/server/src/org/apache/cloudstack/storage/resource/HttpUploadServerHandler.java b/services/secondary-storage/server/src/org/apache/cloudstack/storage/resource/HttpUploadServerHandler.java
index 44a2b60..4a3fa86 100644
--- a/services/secondary-storage/server/src/org/apache/cloudstack/storage/resource/HttpUploadServerHandler.java
+++ b/services/secondary-storage/server/src/org/apache/cloudstack/storage/resource/HttpUploadServerHandler.java
@@ -28,6 +28,8 @@ import java.util.Map;
import java.util.Map.Entry;
import org.apache.cloudstack.storage.template.UploadEntity;
+import org.apache.cloudstack.utils.imagestore.ImageStoreUtil;
+import org.apache.commons.lang.StringUtils;
import org.apache.log4j.Logger;
import com.cloud.exception.InvalidParameterValueException;
@@ -228,6 +230,14 @@ public class HttpUploadServerHandler extends SimpleChannelInboundHandler<HttpObj
FileUpload fileUpload = (FileUpload) data;
if (fileUpload.isCompleted()) {
fileReceived = true;
+ String format = ImageStoreUtil.checkTemplateFormat(fileUpload.getFile().getAbsolutePath(), fileUpload.getFilename());
+ if(StringUtils.isNotBlank(format)) {
+ String errorString = "File type mismatch between the sent file and the actual content. Received: " + format;
+ logger.error(errorString);
+ responseContent.append(errorString);
+ storageResource.updateStateMapWithError(uuid, errorString);
+ return HttpResponseStatus.BAD_REQUEST;
+ }
String status = storageResource.postUpload(uuid, fileUpload.getFile().getName());
if (status != null) {
responseContent.append(status);
http://git-wip-us.apache.org/repos/asf/cloudstack/blob/018023c1/services/secondary-storage/server/src/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java
----------------------------------------------------------------------
diff --git a/services/secondary-storage/server/src/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java b/services/secondary-storage/server/src/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java
index f67015c..8ccbcb6 100755
--- a/services/secondary-storage/server/src/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java
+++ b/services/secondary-storage/server/src/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java
@@ -66,8 +66,10 @@ import io.netty.handler.logging.LogLevel;
import io.netty.handler.logging.LoggingHandler;
import org.apache.cloudstack.storage.command.TemplateOrVolumePostUploadCommand;
import org.apache.cloudstack.storage.template.UploadEntity;
+import org.apache.cloudstack.utils.imagestore.ImageStoreUtil;
import org.apache.commons.codec.digest.DigestUtils;
import org.apache.commons.io.FileUtils;
+import org.apache.commons.io.FilenameUtils;
import org.apache.commons.lang.StringUtils;
import org.apache.http.HttpEntity;
import org.apache.http.HttpResponse;
@@ -2663,6 +2665,18 @@ public class NfsSecondaryStorageResource extends ServerResourceBase implements S
String fileSavedTempLocation = uploadEntity.getInstallPathPrefix() + "/" + filename;
+ String uploadedFileExtension = FilenameUtils.getExtension(filename);
+ String userSelectedFormat= "vhd";
+ if(uploadedFileExtension.equals("zip") || uploadedFileExtension.equals("bz2") || uploadedFileExtension.equals("gz")) {
+ userSelectedFormat += "." + uploadedFileExtension;
+ }
+ String formatError = ImageStoreUtil.checkTemplateFormat(fileSavedTempLocation, userSelectedFormat);
+ if(StringUtils.isNotBlank(formatError)) {
+ String errorString = "File type mismatch between uploaded file and selected format. Selected file format: " + uploadEntity.getFormat() + ". Received: " + formatError;
+ s_logger.error(errorString);
+ return errorString;
+ }
+
int imgSizeGigs = getSizeInGB(_storage.getSize(fileSavedTempLocation));
int maxSize = uploadEntity.getMaxSizeInGB();
if(imgSizeGigs > maxSize) {
http://git-wip-us.apache.org/repos/asf/cloudstack/blob/018023c1/utils/src/com/cloud/utils/ImageStoreUtil.java
----------------------------------------------------------------------
diff --git a/utils/src/com/cloud/utils/ImageStoreUtil.java b/utils/src/com/cloud/utils/ImageStoreUtil.java
deleted file mode 100644
index 52f1324..0000000
--- a/utils/src/com/cloud/utils/ImageStoreUtil.java
+++ /dev/null
@@ -1,39 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements. See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership. The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied. See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-package com.cloud.utils;
-
-import org.apache.log4j.Logger;
-
-public class ImageStoreUtil {
- public static final Logger s_logger = Logger.getLogger(ImageStoreUtil.class.getName());
-
- public static String generatePostUploadUrl(String ssvmUrlDomain, String ipAddress, String uuid) {
- String hostname = ipAddress;
-
- //if ssvm url domain is present, use it to construct hostname in the format 1-2-3-4.domain
- // if the domain name is not present, ssl validation fails and has to be ignored
- if(StringUtils.isNotBlank(ssvmUrlDomain)) {
- hostname = ipAddress.replace(".", "-");
- hostname = hostname + ssvmUrlDomain.substring(1);
- }
-
- //only https works with postupload and url format is fixed
- return "https://" + hostname + "/upload/" + uuid;
- }
-}
http://git-wip-us.apache.org/repos/asf/cloudstack/blob/018023c1/utils/src/org/apache/cloudstack/utils/imagestore/ImageStoreUtil.java
----------------------------------------------------------------------
diff --git a/utils/src/org/apache/cloudstack/utils/imagestore/ImageStoreUtil.java b/utils/src/org/apache/cloudstack/utils/imagestore/ImageStoreUtil.java
new file mode 100644
index 0000000..ed13360
--- /dev/null
+++ b/utils/src/org/apache/cloudstack/utils/imagestore/ImageStoreUtil.java
@@ -0,0 +1,110 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.cloudstack.utils.imagestore;
+
+import com.cloud.utils.script.Script;
+import org.apache.commons.lang.StringUtils;
+import org.apache.log4j.Logger;
+
+public class ImageStoreUtil {
+ public static final Logger s_logger = Logger.getLogger(ImageStoreUtil.class.getName());
+
+ public static String generatePostUploadUrl(String ssvmUrlDomain, String ipAddress, String uuid) {
+ String hostname = ipAddress;
+
+ //if ssvm url domain is present, use it to construct hostname in the format 1-2-3-4.domain
+ // if the domain name is not present, ssl validation fails and has to be ignored
+ if(StringUtils.isNotBlank(ssvmUrlDomain)) {
+ hostname = ipAddress.replace(".", "-");
+ hostname = hostname + ssvmUrlDomain.substring(1);
+ }
+
+ //only https works with postupload and url format is fixed
+ return "https://" + hostname + "/upload/" + uuid;
+ }
+
+ // given a path, returns empty if path is supported image, and the file type if unsupported
+ // this is meant to catch things like accidental upload of ASCII text .vmdk descriptor
+ public static String checkTemplateFormat(String path, String uripath) {
+ // note 'path' was generated by us so it should be safe on the cmdline, be wary of 'url'
+ String command = "file ";
+ if (isCompressedExtension(uripath)) {
+ command = "file -z ";
+ }
+ String output = Script.runSimpleBashScript(command + path + " | cut -d: -f2", 60000);
+
+ // vmdk
+ if ((output.contains("VMware") || output.contains("data")) && isCorrectExtension(uripath, "vmdk")) {
+ s_logger.debug("File at path " + path + " looks like a vmware image :" + output);
+ return "";
+ }
+ // raw
+ if ((output.contains("x86 boot") || output.contains("data")) && (isCorrectExtension(uripath, "raw") || isCorrectExtension(uripath, "img"))) {
+ s_logger.debug("File at path " + path + " looks like a raw image :" + output);
+ return "";
+ }
+ // qcow2
+ if (output.contains("QEMU QCOW") && isCorrectExtension(uripath, "qcow2")) {
+ s_logger.debug("File at path " + path + " looks like QCOW2 : " + output);
+ return "";
+ }
+ // vhd
+ if (output.contains("Microsoft Disk Image") && (isCorrectExtension(uripath, "vhd") || isCorrectExtension(uripath, "vhdx"))) {
+ s_logger.debug("File at path " + path + " looks like vhd : " + output);
+ return "";
+ }
+ // ova
+ if (output.contains("POSIX tar") && isCorrectExtension(uripath, "ova")) {
+ s_logger.debug("File at path " + path + " looks like ova : " + output);
+ return "";
+ }
+
+ //lxc
+ if (output.contains("POSIX tar") && isCorrectExtension(uripath, "tar")) {
+ s_logger.debug("File at path " + path + " looks like just tar : " + output);
+ return "";
+ }
+
+ if (output.contains("ISO 9660") && isCorrectExtension(uripath, "iso")) {
+ s_logger.debug("File at path " + path + " looks like an iso : " + output);
+ return "";
+ }
+ return output;
+ }
+
+ private static boolean isCorrectExtension(String path, String ext) {
+ if (path.toLowerCase().endsWith(ext)
+ || path.toLowerCase().endsWith(ext + ".gz")
+ || path.toLowerCase().endsWith(ext + ".bz2")
+ || path.toLowerCase().endsWith(ext + ".zip")) {
+ return true;
+ }
+ return false;
+ }
+
+ private static boolean isCompressedExtension(String path) {
+ if (path.toLowerCase().endsWith(".gz")
+ || path.toLowerCase().endsWith(".bz2")
+ || path.toLowerCase().endsWith(".zip")) {
+ return true;
+ }
+ return false;
+ }
+}
+
http://git-wip-us.apache.org/repos/asf/cloudstack/blob/018023c1/utils/src/org/apache/cloudstack/utils/template/TemplateUtils.java
----------------------------------------------------------------------
diff --git a/utils/src/org/apache/cloudstack/utils/template/TemplateUtils.java b/utils/src/org/apache/cloudstack/utils/template/TemplateUtils.java
deleted file mode 100644
index 53aa911..0000000
--- a/utils/src/org/apache/cloudstack/utils/template/TemplateUtils.java
+++ /dev/null
@@ -1,97 +0,0 @@
-//
-// Licensed to the Apache Software Foundation (ASF) under one
-// or more contributor license agreements. See the NOTICE file
-// distributed with this work for additional information
-// regarding copyright ownership. The ASF licenses this file
-// to you under the Apache License, Version 2.0 (the
-// "License"); you may not use this file except in compliance
-// with the License. You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing,
-// software distributed under the License is distributed on an
-// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
-// KIND, either express or implied. See the License for the
-// specific language governing permissions and limitations
-// under the License.
-//
-
-package org.apache.cloudstack.utils.template;
-
-import org.apache.log4j.Logger;
-
-import com.cloud.utils.script.Script;
-
-public class TemplateUtils {
- public static final Logger s_logger = Logger.getLogger(TemplateUtils.class.getName());
-
- // given a path, returns empty if path is supported image, and the file type if unsupported
- // this is meant to catch things like accidental upload of ASCII text .vmdk descriptor
- public static String checkTemplateFormat(String path, String uripath) {
- // note 'path' was generated by us so it should be safe on the cmdline, be wary of 'url'
- String command = "file ";
- if (isCompressedExtension(uripath)) {
- command = "file -z ";
- }
- String output = Script.runSimpleBashScript(command + path + " | cut -d: -f2", 60000);
-
- // vmdk
- if ((output.contains("VMware") || output.contains("data")) && isCorrectExtension(uripath, "vmdk")) {
- s_logger.debug("File at path " + path + " looks like a vmware image :" + output);
- return "";
- }
- // raw
- if ((output.contains("x86 boot") || output.contains("data")) && (isCorrectExtension(uripath, "raw") || isCorrectExtension(uripath, "img"))) {
- s_logger.debug("File at path " + path + " looks like a raw image :" + output);
- return "";
- }
- // qcow2
- if (output.contains("QEMU QCOW") && isCorrectExtension(uripath, "qcow2")) {
- s_logger.debug("File at path " + path + " looks like QCOW2 : " + output);
- return "";
- }
- // vhd
- if (output.contains("Microsoft Disk Image") && (isCorrectExtension(uripath, "vhd") || isCorrectExtension(uripath, "vhdx"))) {
- s_logger.debug("File at path " + path + " looks like vhd : " + output);
- return "";
- }
- // ova
- if (output.contains("POSIX tar") && isCorrectExtension(uripath, "ova")) {
- s_logger.debug("File at path " + path + " looks like ova : " + output);
- return "";
- }
-
- //lxc
- if (output.contains("POSIX tar") && isCorrectExtension(uripath, "tar")) {
- s_logger.debug("File at path " + path + " looks like just tar : " + output);
- return "";
- }
-
- if (output.contains("ISO 9660") && isCorrectExtension(uripath, "iso")) {
- s_logger.debug("File at path " + path + " looks like an iso : " + output);
- return "";
- }
- return output;
- }
-
- public static boolean isCorrectExtension(String path, String ext) {
- if (path.toLowerCase().endsWith(ext)
- || path.toLowerCase().endsWith(ext + ".gz")
- || path.toLowerCase().endsWith(ext + ".bz2")
- || path.toLowerCase().endsWith(ext + ".zip")) {
- return true;
- }
- return false;
- }
-
- public static boolean isCompressedExtension(String path) {
- if (path.toLowerCase().endsWith(".gz")
- || path.toLowerCase().endsWith(".bz2")
- || path.toLowerCase().endsWith(".zip")) {
- return true;
- }
- return false;
- }
-}
-
http://git-wip-us.apache.org/repos/asf/cloudstack/blob/018023c1/utils/test/org/apache/cloudstack/utils/imagestore/ImageStoreUtilTest.java
----------------------------------------------------------------------
diff --git a/utils/test/org/apache/cloudstack/utils/imagestore/ImageStoreUtilTest.java b/utils/test/org/apache/cloudstack/utils/imagestore/ImageStoreUtilTest.java
new file mode 100644
index 0000000..e1b5578
--- /dev/null
+++ b/utils/test/org/apache/cloudstack/utils/imagestore/ImageStoreUtilTest.java
@@ -0,0 +1,38 @@
+package org.apache.cloudstack.utils.imagestore;
+
+import java.net.MalformedURLException;
+import java.net.URL;
+import java.util.UUID;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+public class ImageStoreUtilTest {
+
+ @Test
+ public void testgeneratePostUploadUrl() throws MalformedURLException {
+ String ssvmdomain = "*.realhostip.com";
+ String ipAddress = "10.147.28.14";
+ String uuid = UUID.randomUUID().toString();
+
+ //ssvm domain is not set
+ String url = ImageStoreUtil.generatePostUploadUrl(null, ipAddress, uuid);
+ assertPostUploadUrl(url, ipAddress, uuid);
+
+ //ssvm domain is set to empty value
+ url = ImageStoreUtil.generatePostUploadUrl("", ipAddress, uuid);
+ assertPostUploadUrl(url, ipAddress, uuid);
+
+ //ssvm domain is set to a valid value
+ url = ImageStoreUtil.generatePostUploadUrl(ssvmdomain, ipAddress, uuid);
+ assertPostUploadUrl(url, ipAddress.replace(".", "-") + ssvmdomain.substring(1), uuid);
+ }
+
+ private void assertPostUploadUrl(String urlStr, String domain, String uuid) throws MalformedURLException {
+ URL url = new URL(urlStr);
+ Assert.assertNotNull(url);
+ Assert.assertEquals(url.getHost(), domain);
+ Assert.assertEquals(url.getPath(), "/upload/" + uuid);
+ }
+
+}
\ No newline at end of file