Updated Branches: refs/heads/master 2f2918594 -> bc98d8ab4
test and cleanup for getBase64Keystore ConfigurationServerImpl.getBase64Keystore did not close the file input stream correctly. This patch adds test and replaces the file read with commons-io FileUtils call. Signed-off-by: Laszlo Hornyak <laszlo.horn...@gmail.com> Signed-off-by: Prasanna Santhanam <t...@apache.org> Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/48941879 Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/48941879 Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/48941879 Branch: refs/heads/master Commit: 4894187991d581b72807b4282b7a29a48a8031e5 Parents: 2f29185 Author: Laszlo Hornyak <laszlo.horn...@gmail.com> Authored: Fri May 31 02:53:11 2013 +0200 Committer: Prasanna Santhanam <t...@apache.org> Committed: Fri May 31 11:26:33 2013 +0530 ---------------------------------------------------------------------- server/pom.xml | 5 + .../com/cloud/server/ConfigurationServerImpl.java | 21 +--- .../cloud/server/ConfigurationServerImplTest.java | 78 +++++++++++++++ 3 files changed, 89 insertions(+), 15 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cloudstack/blob/48941879/server/pom.xml ---------------------------------------------------------------------- diff --git a/server/pom.xml b/server/pom.xml index 6385bf2..8fe1e2d 100644 --- a/server/pom.xml +++ b/server/pom.xml @@ -19,6 +19,11 @@ </parent> <dependencies> <dependency> + <groupId>commons-io</groupId> + <artifactId>commons-io</artifactId> + <version>${cs.commons-io.version}</version> + </dependency> + <dependency> <groupId>org.apache.cloudstack</groupId> <artifactId>cloud-core</artifactId> <version>${project.version}</version> http://git-wip-us.apache.org/repos/asf/cloudstack/blob/48941879/server/src/com/cloud/server/ConfigurationServerImpl.java ---------------------------------------------------------------------- diff --git a/server/src/com/cloud/server/ConfigurationServerImpl.java b/server/src/com/cloud/server/ConfigurationServerImpl.java index 35f977b..d334d7e 100755 --- a/server/src/com/cloud/server/ConfigurationServerImpl.java +++ b/server/src/com/cloud/server/ConfigurationServerImpl.java @@ -47,6 +47,7 @@ import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailVO; import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao; import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; import org.apache.commons.codec.binary.Base64; +import org.apache.commons.io.FileUtils; import org.apache.log4j.Logger; import org.springframework.stereotype.Component; @@ -437,23 +438,13 @@ public class ConfigurationServerImpl extends ManagerBase implements Configuratio } } - private String getBase64Keystore(String keystorePath) throws IOException { - byte[] storeBytes = new byte[4094]; - int len = 0; - try { - len = new FileInputStream(keystorePath).read(storeBytes); - } catch (EOFException e) { - } catch (Exception e) { - throw new IOException("Cannot read the generated keystore file"); + static String getBase64Keystore(String keystorePath) throws IOException { + byte[] storeBytes = FileUtils.readFileToByteArray(new File(keystorePath)); + if (storeBytes.length > 3000) { // Base64 codec would enlarge data by 1/3, and we have 4094 bytes in database entry at most + throw new IOException("KeyStore is too big for database! Length " + storeBytes.length); } - if (len > 3000) { // Base64 codec would enlarge data by 1/3, and we have 4094 bytes in database entry at most - throw new IOException("KeyStore is too big for database! Length " + len); - } - - byte[] encodeBytes = new byte[len]; - System.arraycopy(storeBytes, 0, encodeBytes, 0, len); - return new String(Base64.encodeBase64(encodeBytes)); + return new String(Base64.encodeBase64(storeBytes)); } private void generateDefaultKeystore(String keystorePath) throws IOException { http://git-wip-us.apache.org/repos/asf/cloudstack/blob/48941879/server/test/com/cloud/server/ConfigurationServerImplTest.java ---------------------------------------------------------------------- diff --git a/server/test/com/cloud/server/ConfigurationServerImplTest.java b/server/test/com/cloud/server/ConfigurationServerImplTest.java new file mode 100644 index 0000000..fd4bf03 --- /dev/null +++ b/server/test/com/cloud/server/ConfigurationServerImplTest.java @@ -0,0 +1,78 @@ +// 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.server; + +import java.io.File; +import java.io.IOException; + +import org.apache.commons.codec.binary.Base64; +import org.apache.commons.io.FileUtils; +import org.junit.Test; + +public class ConfigurationServerImplTest { + final static String TEST = "the quick brown fox jumped over the lazy dog"; + + @Test(expected = IOException.class) + public void testGetBase64KeystoreNoSuchFile() throws IOException { + ConfigurationServerImpl.getBase64Keystore("notexisting" + System.currentTimeMillis()); + } + + @Test(expected = IOException.class) + public void testGetBase64KeystoreTooBigFile() throws IOException { + File temp = File.createTempFile("keystore", ""); + StringBuilder builder = new StringBuilder(); + for (int i = 0; i < 1000; i++) { + builder.append("way too long...\n"); + } + FileUtils.writeStringToFile(temp, builder.toString()); + try { + ConfigurationServerImpl.getBase64Keystore(temp.getPath()); + } finally { + temp.delete(); + } + } + + @Test + public void testGetBase64Keystore() throws IOException { + File temp = File.createTempFile("keystore", ""); + try { + FileUtils.writeStringToFile(temp, Base64.encodeBase64String(TEST.getBytes())); + final String keystore = ConfigurationServerImpl.getBase64Keystore(temp.getPath()); + // let's decode it to make sure it makes sense + Base64.decodeBase64(keystore); + } finally { + temp.delete(); + } + } + + @Test + public void testGetBase64KeystoreZillionTimes() throws IOException { + File temp = File.createTempFile("keystore", ""); + try { + // may cause IOException with the original implementation because of too many open files + for (int i = 0; i < 100000; i++) { + FileUtils.writeStringToFile(temp, Base64.encodeBase64String(TEST.getBytes())); + final String keystore = ConfigurationServerImpl.getBase64Keystore(temp.getPath()); + // let's decode it to make sure it makes sense + Base64.decodeBase64(keystore); + } + } finally { + temp.delete(); + } + } + +}
