CLOUDSTACK-6053: While adding a primary or secondary of type smb the password wasn't

encoded. This cause createStoragePool or addImageStore command to fail if special
characters were present. Updated the code to pass user, password and domain as part
of details while adding primary or secondary. Also made changes on server side to
handle it.

engine/schema/src/org/apache/cloudstack/storage/datastore/db/ImageStoreVO.java

@@ -159,9 +159,6 @@ public class ImageStoreVO implements ImageStore {
 
     public void setUrl(String url) {
         this.url = url;
-        if ("cifs".equalsIgnoreCase(this.protocol)) {
-            this.url = UriUtils.getUpdateUri(url, true);
-        }
     }
 
     public Date getCreated() {

engine/schema/src/org/apache/cloudstack/storage/datastore/db/StoragePoolVO.java

@@ -298,9 +298,6 @@ public class StoragePoolVO implements StoragePool {
 
     public void setPath(String path) {
         this.path = path;
-        if (this.poolType == StoragePoolType.SMB) {
-            this.path = UriUtils.getUpdateUri(this.path, true);
-        }
     }
 
     public void setUserInfo(String userInfo) {

engine/storage/src/org/apache/cloudstack/storage/image/datastore/ImageStoreHelper.java

@@ -18,6 +18,8 @@
  */
 package org.apache.cloudstack.storage.image.datastore;
 
+import java.io.UnsupportedEncodingException;
+import java.net.URLEncoder;
 import java.util.Iterator;
 import java.util.Map;
 import java.util.UUID;
@@ -34,6 +36,7 @@ import org.apache.cloudstack.storage.datastore.db.ImageStoreDetailsDao;
 import org.apache.cloudstack.storage.datastore.db.ImageStoreVO;
 import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreDao;
 
+import com.cloud.exception.InvalidParameterValueException;
 import com.cloud.storage.DataStoreRole;
 import com.cloud.storage.ScopeType;
 import com.cloud.utils.crypt.DBEncryptionUtil;
@@ -95,7 +98,30 @@ public class ImageStoreHelper {
         if (store.getName() == null) {
             store.setName(store.getUuid());
         }
+
         store.setRole((DataStoreRole)params.get("role"));
+
+        if ("cifs".equalsIgnoreCase((String) params.get("protocol")) && details != null) {
+            String user = details.get("user");
+            String password = details.get("password");
+            String domain = details.get("domain");
+            String updatedPath = (String) params.get("url");
+
+            if (user == null || password == null) {
+                String errMsg = "Missing cifs user and password details. Add them as details parameter.";
+                throw new InvalidParameterValueException(errMsg);
+            } else {
+                try {
+                    password = DBEncryptionUtil.encrypt(URLEncoder.encode(password, "UTF-8"));
+                    details.put("password", password);
+                    updatedPath += "?user=" + user + "&password=" + password + "&domain=" + domain;
+                } catch (UnsupportedEncodingException e) {
+                    throw new CloudRuntimeException("Error while generating the cifs url. " + e.getMessage());
+                }
+                store.setUrl(updatedPath);
+            }
+        }
+
         store = imageStoreDao.persist(store);
 
         // persist details

engine/storage/src/org/apache/cloudstack/storage/volume/datastore/PrimaryDataStoreHelper.java

@@ -18,6 +18,8 @@
  */
 package org.apache.cloudstack.storage.volume.datastore;
 
+import java.io.UnsupportedEncodingException;
+import java.net.URLEncoder;
 import java.util.List;
 import java.util.Map;
 
@@ -38,12 +40,15 @@ import com.cloud.capacity.Capacity;
 import com.cloud.capacity.CapacityVO;
 import com.cloud.capacity.dao.CapacityDao;
 import com.cloud.hypervisor.Hypervisor.HypervisorType;
+import com.cloud.exception.InvalidParameterValueException;
 import com.cloud.storage.DataStoreRole;
 import com.cloud.storage.ScopeType;
 import com.cloud.storage.StorageManager;
 import com.cloud.storage.StoragePoolHostVO;
 import com.cloud.storage.StoragePoolStatus;
+import com.cloud.storage.Storage.StoragePoolType;
 import com.cloud.storage.dao.StoragePoolHostDao;
+import com.cloud.utils.crypt.DBEncryptionUtil;
 import com.cloud.utils.db.TransactionLegacy;
 import com.cloud.utils.exception.CloudRuntimeException;
 
@@ -87,6 +92,29 @@ public class PrimaryDataStoreHelper {
         dataStoreVO.setHypervisor(params.getHypervisorType());
 
         Map<String, String> details = params.getDetails();
+        if (params.getType() == StoragePoolType.SMB && details != null) {
+            String user = details.get("user");
+            String password = details.get("password");
+            String domain = details.get("domain");
+            String updatedPath = params.getPath();
+
+            if (user == null || password == null) {
+                String errMsg = "Missing cifs user and password details. Add them as details parameter.";
+                s_logger.warn(errMsg);
+                throw new InvalidParameterValueException(errMsg);
+            } else {
+                try {
+                    password = DBEncryptionUtil.encrypt(URLEncoder.encode(password, "UTF-8"));
+                    details.put("password", password);
+                    updatedPath += "?user=" + user + "&password=" + password + "&domain=" + domain;
+                } catch (UnsupportedEncodingException e) {
+                    throw new CloudRuntimeException("Error while generating the cifs url. " + e.getMessage());
+                }
+            }
+
+            dataStoreVO.setPath(updatedPath);
+        }
+
         String tags = params.getTags();
         if (tags != null) {
             String[] tokens = tags.split(",");

server/src/com/cloud/api/query/dao/ImageStoreJoinDaoImpl.java

@@ -81,7 +81,7 @@ public class ImageStoreJoinDaoImpl extends GenericDaoBase<ImageStoreJoinVO, Long
         osResponse.setZoneName(ids.getZoneName());
 
         String detailName = ids.getDetailName();
-        if ( detailName != null && detailName.length() > 0 ){
+        if ( detailName != null && detailName.length() > 0 && !detailName.equals(ApiConstants.PASSWORD)) {
             String detailValue = ids.getDetailValue();
             if (detailName.equals(ApiConstants.KEY) || detailName.equals(ApiConstants.S3_SECRET_KEY)) {
                 detailValue = DBEncryptionUtil.decrypt(detailValue);
@@ -96,7 +96,7 @@ public class ImageStoreJoinDaoImpl extends GenericDaoBase<ImageStoreJoinVO, Long
     @Override
     public ImageStoreResponse setImageStoreResponse(ImageStoreResponse response, ImageStoreJoinVO ids) {
         String detailName = ids.getDetailName();
-        if ( detailName != null && detailName.length() > 0 ){
+        if ( detailName != null && detailName.length() > 0 && !detailName.equals(ApiConstants.PASSWORD)) {
             String detailValue = ids.getDetailValue();
             if (detailName.equals(ApiConstants.KEY) || detailName.equals(ApiConstants.S3_SECRET_KEY)) {
                 detailValue = DBEncryptionUtil.decrypt(detailValue);

ui/scripts/sharedFunctions.js

@@ -1253,7 +1253,7 @@ var processPropertiesInImagestoreObject = function(jsonObj) {
         	url += 'cifs://';
         }
         
-        url += (server + path + '?user=' + smbUsername + '&password=' + smbPassword + '&domain=' + smbDomain);
+        url += (server + path);
                
         return url;
     }

ui/scripts/system.js

@@ -15662,7 +15662,10 @@
                                     var path = args.data.path;
                                     if (path.substring(0, 1) != "/")
                                         path = "/" + path;
-                                    url = smbURL(server, path, args.data.smbUsername, args.data.smbPassword, args.data.smbDomain);
+                                    url = smbURL(server, path);
+                                    array1.push("&details[0].user=" + args.data.smbUsername);
+                                    array1.push("&details[1].password=" + todb(args.data.smbPassword));
+                                    array1.push("&details[2].domain=" + args.data.smbDomain);
                                 } else if (args.data.protocol == "PreSetup") {                                    
                                     var path = args.data.path;
                                     if (path.substring(0, 1) != "/")
@@ -17065,12 +17068,17 @@
                                             var zoneid = args.data.zoneid;
                                             var nfs_server = args.data.nfsServer;
                                             var path = args.data.path;
-                                            var url = smbURL(nfs_server, path, args.data.smbUsername, args.data.smbPassword, args.data.smbDomain);
+                                            var url = smbURL(nfs_server, path);
-                                            
                                             $.extend(data, {
                                                 provider: args.data.provider,
                                                 zoneid: zoneid,
-                                                url: url
+                                                url: url,
+                                                'details[0].key': 'user',
+                                                'details[0].value': args.data.smbUsername,
+                                                'details[1].key': 'password',
+                                                'details[1].value': args.data.smbPassword,
+                                                'details[2].key': 'domain',
+                                                'details[2].value': args.data.smbDomain
                                             });
                                             
                                             $.ajax({

ui/scripts/zoneWizard.js

@@ -4420,7 +4420,10 @@
                     	var path = args.data.primaryStorage.path;
                         if (path.substring(0, 1) != "/")
                             path = "/" + path;
-                        url = smbURL(server, path, args.data.primaryStorage.smbUsername, args.data.primaryStorage.smbPassword, args.data.primaryStorage.smbDomain);
+                        url = smbURL(server, path);
+                        array1.push("&details[0].user=" + args.data.primaryStorage.smbUsername);
+                        array1.push("&details[1].password=" + todb(args.data.primaryStorage.smbPassword));
+                        array1.push("&details[2].domain=" + args.data.primaryStorage.smbDomain);
                     } else if (args.data.primaryStorage.protocol == "PreSetup") {                        
                         var path = args.data.primaryStorage.path;
                         if (path.substring(0, 1) != "/")
@@ -4529,12 +4532,18 @@
                     } else if (args.data.secondaryStorage.provider == 'SMB') {
                         var nfs_server = args.data.secondaryStorage.nfsServer;
                         var path = args.data.secondaryStorage.path;
-                        var url = smbURL(nfs_server, path, args.data.secondaryStorage.smbUsername, args.data.secondaryStorage.smbPassword, args.data.secondaryStorage.smbDomain);
+                        var url = smbURL(nfs_server, path);
 
                         $.extend(data, {
                             provider: args.data.secondaryStorage.provider,
                             zoneid: args.data.returnedZone.id,
-                            url: url
+                            url: url,
+                            'details[0].key': 'user',
+                            'details[0].value': args.data.secondaryStorage.smbUsername,
+                            'details[1].key': 'password',
+                            'details[1].value': args.data.secondaryStorage.smbPassword,
+                            'details[2].key': 'domain',
+                            'details[2].value': args.data.secondaryStorage.smbDomain
                         });
 
                         $.ajax({

utils/src/com/cloud/utils/UriUtils.java

@@ -108,13 +108,7 @@ public class UriUtils {
 
     public static String getCifsUriParametersProblems(URI uri) {
         if (!UriUtils.hostAndPathPresent(uri)) {
-            String errMsg = "cifs URI missing host and/or path.  " + " Make sure it's of the format " + "cifs://hostname/path?user=<username>&password=<password>";
+            String errMsg = "cifs URI missing host and/or path. Make sure it's of the format cifs://hostname/path";
-            s_logger.warn(errMsg);
-            return errMsg;
-        }
-        if (!UriUtils.cifsCredentialsPresent(uri)) {
-            String errMsg =
-                "cifs URI missing user and password details. " + "Add them as query parameters, e.g. " + "cifs://example.com/some_share?user=foo&password=bar";
             s_logger.warn(errMsg);
             return errMsg;
         }
@@ -185,12 +179,14 @@ public class UriUtils {
 
     private static List<NameValuePair> getUserDetails(String query) {
         List<NameValuePair> details = new ArrayList<NameValuePair>();
+        if (query != null && !query.isEmpty()) {
             StringTokenizer allParams = new StringTokenizer(query, "&");
             while (allParams.hasMoreTokens()) {
                 String param = allParams.nextToken();
                 details.add(new BasicNameValuePair(param.substring(0, param.indexOf("=")),
                         param.substring(param.indexOf("=") + 1)));
             }
+        }
 
         return details;
     }