modules/core/util/src/main/java/org/rhq/core/util/file/FileUtil.java | 182 +++++++++-
modules/core/util/src/main/java/org/rhq/core/util/updater/DeploymentData.java | 54 +-
modules/core/util/src/test/java/org/rhq/core/util/file/FileUtilTest.java | 141 +++++--
modules/core/util/src/test/java/org/rhq/core/util/updater/DeployerCanonicalPathTest.java | 180 +++++++--
modules/plugins/jboss-as-5/src/main/java/org/rhq/plugins/jbossas5/ManagedComponentComponent.java | 34 +
5 files changed, 475 insertions(+), 116 deletions(-)
New commits:
commit a8472635e28dcdc053e5d3d4a700d5cd8c60e752
Author: Lukas Krejci <lkrejci(a)redhat.com>
Date: Thu Jun 27 23:46:34 2013 +0200
[BZ 917765] - Symlinks in deploy dir no longer confuse relative file path
if deployDir was:
/opt/depls/target -> /opt/realdepls/realtarget
(i.e. target was a symlink to realtarget)
and a file was to be deployed to:
../conf/file
a resulting path would be:
opt/depls/target/../conf/file
which would be resolved as:
opt/realdepls/conf/file
(i.e. the symlink would be resolved first and then the path would be
normalized)
This is not how people usually understand the paths and how they expect
the bundle deployer to function.
We therefore first manually normalize the path and only then obtain its
absolute path to point to it during the deployment process.
Notice that we no longer use the canonical paths anywhere in the deployer
code.
diff --git a/modules/core/util/src/main/java/org/rhq/core/util/file/FileUtil.java b/modules/core/util/src/main/java/org/rhq/core/util/file/FileUtil.java
index d3b3ddf..0de4e43 100644
--- a/modules/core/util/src/main/java/org/rhq/core/util/file/FileUtil.java
+++ b/modules/core/util/src/main/java/org/rhq/core/util/file/FileUtil.java
@@ -37,6 +37,7 @@ import java.util.ArrayList;
import java.util.Deque;
import java.util.LinkedList;
import java.util.List;
+import java.util.StringTokenizer;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
@@ -588,4 +589,183 @@ public class FileUtil {
return s == null || s.length() == 0;
}
-}
\ No newline at end of file
+ /**
+ * Normalizes the path of the file by removing any ".." and "."
+ * <p/>
+ * This method behaves very similar to Java7's {@code Path.normalize()} method with the exception of dealing with
+ * paths jumping "above" the FS root.
+ * <p/>
+ * Java7's normalization will normalize a path like {@code C:\..\asdf} to {@code C:\asdf}, while this method will
+ * return null, because it understands {@code C:\..\asdf} as an attempt to "go above" the file system root.
+ *
+ * @return the file with the normalized path or null if the ".."s would jump further up than the number of preceding
+ * path elements (e.g. passing files with paths like ".." or "path/../.." will return null).
+ */
+ public static File normalizePath(File file) {
+ String path = file.getPath();
+
+ int rootLength = FileSystem.get().getPathRootLength(path);
+ File root = rootLength == 0 ? null : new File(path.substring(0, rootLength));
+
+ StringTokenizer tokenizer = new StringTokenizer(path.substring(rootLength), FileSystem.get().getSeparatorChars(), true);
+ LinkedList<String> pathStack = new LinkedList<String>();
+
+ boolean previousWasDelimiter = false;
+
+ while (tokenizer.hasMoreTokens()) {
+ String token = tokenizer.nextToken();
+
+ if (File.separator.equals(token)) {
+ if (!previousWasDelimiter) {
+ pathStack.push(token);
+ previousWasDelimiter = true;
+ }
+ } else if ("..".equals(token)) {
+ //yes, this is correct - ".." will jump up the stack to the next-previous delimiter, so we should
+ //declare that we're at a delimiter position.
+ previousWasDelimiter = true;
+ if (pathStack.isEmpty()) {
+ return null;
+ } else {
+ //pop the previous delimiter(s)
+ pathStack.pop();
+
+ //and pop the previous path element
+ if (pathStack.isEmpty()) {
+ return null;
+ }
+ pathStack.pop();
+ }
+ } else if (".".equals(token)) {
+ previousWasDelimiter = true;
+ } else if (token.length() > 0) {
+ previousWasDelimiter = false;
+ pathStack.push(token);
+ } else {
+ previousWasDelimiter = false;
+ }
+ }
+
+ StringBuilder normalizedPath = new StringBuilder();
+
+ for (int i = pathStack.size(); --i >= 0; ) {
+ normalizedPath.append(pathStack.get(i));
+ }
+
+ File ret = root == null ? new File(normalizedPath.toString()) : new File(root, normalizedPath.toString());
+
+ if (file.isAbsolute() != ret.isAbsolute()) {
+ // if the normalization changed the path such that it is not absolute anymore
+ // (or that it wasn't absolute but now is, which shouldn't ever happen), return null.
+ // The fact that the original file was absolute and the normalized path isn't can be caused by
+ // the normalization "climbing past" the prefix of the absolute path which is the drive letter of Windows
+ // for example.
+ return null;
+ } else {
+ return ret;
+ }
+ }
+
+ private enum FileSystem {
+ UNIX {
+ @Override
+ public int getPathRootLength(String path) {
+ if (path != null && path.charAt(0) == '/') {
+ return 1;
+ } else {
+ return 0;
+ }
+ }
+
+ @Override
+ public String getSeparatorChars() {
+ return "/";
+ }
+ },
+ WINDOWS {
+ @Override
+ public int getPathRootLength(String path) {
+ if (path == null || path.length() < 3) {
+ return 0;
+ }
+
+ // C:\asdf
+ // C:asdf
+ // \\host\share\asdf
+
+ char c0 = path.charAt(0);
+ char c1 = path.charAt(1);
+ char c2 = path.charAt(2);
+
+ switch (c0) {
+ case '\\':
+ case '/':
+ if (isSlash(c1)) {
+ //UNC
+ int nextSlash = nextSlash(path, 2);
+ if (nextSlash < 3) {
+ throw new IllegalArgumentException("Invalid UNC path - no host specified");
+ }
+
+ int hostSlash = nextSlash;
+ nextSlash = nextSlash(path, nextSlash + 1);
+
+ if (nextSlash <= hostSlash) {
+ throw new IllegalArgumentException("Invalid UNC path - no share specified");
+ }
+
+ return nextSlash;
+ } else {
+ return 0;
+ }
+ default:
+ if (c1 == ':') {
+ char driveLetter = Character.toLowerCase(c0);
+ if ('a' <= driveLetter && 'z' >= driveLetter) {
+ return c2 == '\\' ? 3 : 2;
+ } else {
+ return 0;
+ }
+ } else {
+ return 0;
+ }
+ }
+ }
+
+ @Override
+ public String getSeparatorChars() {
+ return "\\/";
+ }
+ };
+
+ private static boolean isSlash(char c) {
+ return c == '\\' || c == '/';
+ }
+
+ private static int nextSlash(String str, int from) {
+ int len = str.length();
+ for(int i = from; i < len; ++i) {
+ if (isSlash(str.charAt(i))) {
+ return i;
+ }
+ }
+
+ return -1;
+ }
+
+ public static FileSystem get() {
+ switch (File.separatorChar) {
+ case '/':
+ return UNIX;
+ case '\\':
+ return WINDOWS;
+ default:
+ throw new IllegalStateException("Unsupported filesystem");
+ }
+ }
+
+ public abstract int getPathRootLength(String path);
+
+ public abstract String getSeparatorChars();
+ }
+}
diff --git a/modules/core/util/src/main/java/org/rhq/core/util/updater/DeploymentData.java b/modules/core/util/src/main/java/org/rhq/core/util/updater/DeploymentData.java
index c55c1bc..2431332 100644
--- a/modules/core/util/src/main/java/org/rhq/core/util/updater/DeploymentData.java
+++ b/modules/core/util/src/main/java/org/rhq/core/util/updater/DeploymentData.java
@@ -23,6 +23,7 @@
package org.rhq.core.util.updater;
import java.io.File;
+import java.io.IOException;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
@@ -115,7 +116,18 @@ public class DeploymentData {
this.deploymentProps = deploymentProps;
this.zipFiles = zipFiles;
this.rawFiles = rawFiles;
- this.destinationDir = getCanonicalFile(destinationDir);
+
+ //specifically do NOT resolve symlinks here. This must to be the last thing one needs to do before deploying
+ //the files. The problem is that we use the destination dir as root for the paths of the individual files to
+ //lay down. If the destinationDir uses symlinks and the individual paths of the files were relative
+ // including ..'s, it could happen that the files would be laid down on a different place than expected.
+ //Consider this scenario:
+ //destinationDir = /opt/my/destination -> /tmp/deployments
+ //file = ../conf/some.properties
+ //One expects the file to end up in /opt/my/conf/some.properties
+ //but if we canonicalized the destination dir upfront, we'd end up with /tmp/conf/some.properties.
+ this.destinationDir = destinationDir.getAbsoluteFile();
+
this.sourceDir = sourceDir;
this.ignoreRegex = ignoreRegex;
this.manageRootDir = manageRootDir;
@@ -133,15 +145,14 @@ public class DeploymentData {
}
// We need to "normalize" all raw file paths that have ".." in them to ensure everything works properly.
- // Any raw file pathname (the values in this.rawFiles) that needs to be normalized will be converted to
- // a canonical path. Note that any pathname that is relative but have ".." paths that end up taking the file
+ // Note that any pathname that is relative but have ".." paths that end up taking the file
// above the destination directory needs to be normalized and will end up being an absolute path
// (so all log messages will indicate the full absolute path and if the file
// needs to be backed up it will be backed up as if it was an external file that was specified with an absolute path).
// If the relative path has ".." but does not take the file above the destination directory will simply have its ".."
// normalized out but will still be a relative path (relative to destination directory) (we can't make it absolute
// otherwise Deployer's update will run into errors while backing up and scanning for deleted files).
- // See BZ 917085.
+ // See BZs 917085 and 917765.
for (Map.Entry<File, File> entry : this.rawFiles.entrySet()) {
File rawFile = entry.getValue();
String rawFilePath = rawFile.getPath();
@@ -149,10 +160,10 @@ public class DeploymentData {
boolean doubledot = rawFilePath.replace('\\', '/').matches(".*((/\\.\\.)|(\\.\\./)).*"); // finds "/.." or "../" in the string
if (doubledot) {
- File fileToCanonicalize;
+ File fileToNormalize;
if (rawFile.isAbsolute()) {
- fileToCanonicalize = rawFile;
+ fileToNormalize = rawFile;
} else {
boolean isWindows = (File.separatorChar == '\\');
if (isWindows) {
@@ -169,28 +180,28 @@ public class DeploymentData {
// figure out what the absolute, normalized path is for the raw file
if ((destDirDriveLetter == null || rawFileDriveLetter == null)
|| rawFileDriveLetter.equals(destDirDriveLetter)) {
- fileToCanonicalize = new File(this.destinationDir, rawFilePathBuilder.toString());
+ fileToNormalize = new File(this.destinationDir, rawFilePathBuilder.toString());
} else {
throw new IllegalArgumentException("Cannot normalize relative path [" + rawFilePath
+ "]; its drive letter is different than the destination directory ["
+ this.destinationDir.getAbsolutePath() + "]");
}
} else {
- fileToCanonicalize = new File(this.destinationDir, rawFilePath);
+ fileToNormalize = new File(this.destinationDir, rawFilePath);
}
}
- fileToCanonicalize = getCanonicalFile(fileToCanonicalize);
+ fileToNormalize = getNormalizedFile(fileToNormalize);
- if (isPathUnderBaseDir(this.destinationDir, fileToCanonicalize)) {
+ if (isPathUnderBaseDir(this.destinationDir, fileToNormalize)) {
// we can keep rawFile path relative, but we need to normalize out the ".." paths
String baseDir = this.destinationDir.getAbsolutePath();
- String absRawFilePath = fileToCanonicalize.getAbsolutePath();
- String canonicalRelativePath = absRawFilePath.substring(baseDir.length() + 1); // should always return a valid path; if not, let it throw exception (which likely means there is a bug here)
- entry.setValue(new File(canonicalRelativePath));
+ String absRawFilePath = fileToNormalize.getAbsolutePath();
+ String relativePath = absRawFilePath.substring(baseDir.length() + 1); // should always return a valid path; if not, let it throw exception (which likely means there is a bug here)
+ entry.setValue(new File(relativePath));
} else {
// raw file path has ".." such that the file is really above destination dir - use an absolute, canonical path
- entry.setValue(fileToCanonicalize);
+ entry.setValue(fileToNormalize);
}
}
}
@@ -198,6 +209,10 @@ public class DeploymentData {
return;
}
+ private static File getNormalizedFile(File fileToNormalize) {
+ return FileUtil.normalizePath(fileToNormalize);
+ }
+
public DeploymentProperties getDeploymentProps() {
return deploymentProps;
}
@@ -242,17 +257,6 @@ public class DeploymentData {
return zipsExploded;
}
- private File getCanonicalFile(File file) {
- try {
- file = file.getCanonicalFile();
- } catch (Exception e) {
- // ignore this - this really should never happen, but if it does,
- // we want to continue and hope using the non-normalized file is ok;
- file = file.getAbsoluteFile();
- }
- return file;
- }
-
private boolean isPathUnderBaseDir(File base, File path) {
// this method assumes base and path are absolute and canonical
if (base == null) {
diff --git a/modules/core/util/src/test/java/org/rhq/core/util/file/FileUtilTest.java b/modules/core/util/src/test/java/org/rhq/core/util/file/FileUtilTest.java
index 8ad0942..6970c53 100644
--- a/modules/core/util/src/test/java/org/rhq/core/util/file/FileUtilTest.java
+++ b/modules/core/util/src/test/java/org/rhq/core/util/file/FileUtilTest.java
@@ -32,6 +32,7 @@ import java.io.ByteArrayInputStream;
import java.io.File;
import java.io.FileInputStream;
import java.io.FileOutputStream;
+import java.nio.file.Paths;
import java.util.ArrayList;
import java.util.List;
import java.util.regex.Pattern;
@@ -326,56 +327,107 @@ public class FileUtilTest {
public void testGetPattern() {
Pattern regex;
- regex = assertPatternsRegex("(/basedir/(test1\\.txt))", new PathFilter("/basedir", "test1.txt"));
+ regex = assertPatternsRegex("(" + translateAbsoluteUnixPathToActualAsRegex("/basedir/(test1\\.txt)") + ")",
+ new PathFilter("/basedir", "test1.txt"));
- assert regex.matcher("/basedir/test1.txt").matches();
- assert !regex.matcher("/basedir/test2.txt").matches();
+ assert regex.matcher(translateAbsoluteUnixPathToActual("/basedir/test1.txt")).matches();
+ assert !regex.matcher(translateAbsoluteUnixPathToActual("/basedir/test2.txt")).matches();
- regex = assertPatternsRegex("(/basedir/easy\\.txt)|(/basedir/test\\.txt)", new PathFilter("/basedir/easy.txt",
+ regex = assertPatternsRegex("(" + translateAbsoluteUnixPathToActualAsRegex("/basedir/easy\\.txt") + ")|(" +
+ translateAbsoluteUnixPathToActualAsRegex("/basedir/test\\.txt") + ")", new PathFilter("/basedir/easy.txt",
null), new PathFilter("/basedir/test.txt", null));
- assert regex.matcher("/basedir/easy.txt").matches();
- assert regex.matcher("/basedir/test.txt").matches();
- assert !regex.matcher("/basedir/easyXtxt").matches();
- assert !regex.matcher("/basedir/testXtxt").matches();
- assert !regex.matcher("/basedir/easy.txtX").matches();
- assert !regex.matcher("/basedir/test.txtX").matches();
- assert !regex.matcher("/basedirX/easy.txt").matches();
- assert !regex.matcher("/basedirX/test.txt").matches();
+ assert regex.matcher(translateAbsoluteUnixPathToActual("/basedir/easy.txt")).matches();
+ assert regex.matcher(translateAbsoluteUnixPathToActual("/basedir/test.txt")).matches();
+ assert !regex.matcher(translateAbsoluteUnixPathToActual("/basedir/easyXtxt")).matches();
+ assert !regex.matcher(translateAbsoluteUnixPathToActual("/basedir/testXtxt")).matches();
+ assert !regex.matcher(translateAbsoluteUnixPathToActual("/basedir/easy.txtX")).matches();
+ assert !regex.matcher(translateAbsoluteUnixPathToActual("/basedir/test.txtX")).matches();
+ assert !regex.matcher(translateAbsoluteUnixPathToActual("/basedirX/easy.txt")).matches();
+ assert !regex.matcher(translateAbsoluteUnixPathToActual("/basedirX/test.txt")).matches();
assert !regex.matcher("easy.txt").matches() : "missing basedir";
assert !regex.matcher("test.txt").matches() : "missing basedir";
- regex = assertPatternsRegex("(/basedir/([^/]*\\.txt))", new PathFilter("/basedir", "*.txt"));
+ regex = assertPatternsRegex("(" + translateAbsoluteUnixPathToActualAsRegex("/basedir/([^/]*\\.txt)") + ")",
+ new PathFilter("/basedir", "*.txt"));
- assert regex.matcher("/basedir/foo.txt").matches();
- assert regex.matcher("/basedir/file with spaces.txt").matches();
- assert regex.matcher("/basedir/123.txt").matches();
- assert !regex.matcher("/basedir/subdir/foo.txt").matches();
- assert !regex.matcher("/basedir/foo.txt.swp").matches();
+ assert regex.matcher(translateAbsoluteUnixPathToActual("/basedir/foo.txt")).matches();
+ assert regex.matcher(translateAbsoluteUnixPathToActual("/basedir/file with spaces.txt")).matches();
+ assert regex.matcher(translateAbsoluteUnixPathToActual("/basedir/123.txt")).matches();
+ assert !regex.matcher(translateAbsoluteUnixPathToActual("/basedir/subdir/foo.txt")).matches();
+ assert !regex.matcher(translateAbsoluteUnixPathToActual("/basedir/foo.txt.swp")).matches();
- regex = assertPatternsRegex("(/var/lib/([^/]*\\.war))|(/var/lib/([^/]*\\.ear))", new PathFilter("/var/lib",
+ regex = assertPatternsRegex("(" + translateAbsoluteUnixPathToActualAsRegex("/var/lib/([^/]*\\.war)") + ")|(" +
+ translateAbsoluteUnixPathToActualAsRegex("/var/lib/([^/]*\\.ear)") + ")", new PathFilter("/var/lib",
"*.war"), new PathFilter("/var/lib", "*.ear"));
- assert regex.matcher("/var/lib/myapp.war").matches();
- assert regex.matcher("/var/lib/myapp.ear").matches();
- assert regex.matcher("/var/lib/my-app.war").matches();
- assert !regex.matcher("/var/lib/myapp.War").matches();
- assert !regex.matcher("/var/libs/myapp.war").matches();
- assert !regex.matcher("myapp.ear").matches();
- assert !regex.matcher("/var/lib/myapp.ear.rej").matches();
+ assert regex.matcher(translateAbsoluteUnixPathToActual("/var/lib/myapp.war")).matches();
+ assert regex.matcher(translateAbsoluteUnixPathToActual("/var/lib/myapp.ear")).matches();
+ assert regex.matcher(translateAbsoluteUnixPathToActual("/var/lib/my-app.war")).matches();
+ assert !regex.matcher(translateAbsoluteUnixPathToActual("/var/lib/myapp.War")).matches();
+ assert !regex.matcher(translateAbsoluteUnixPathToActual("/var/libs/myapp.war")).matches();
+ assert !regex.matcher(translateAbsoluteUnixPathToActual("myapp.ear")).matches();
+ assert !regex.matcher(translateAbsoluteUnixPathToActual("/var/lib/myapp.ear.rej")).matches();
- regex = assertPatternsRegex("(/conf/(server-.\\.conf))", new PathFilter("/conf", "server-?.conf"));
+ regex = assertPatternsRegex("(" + translateAbsoluteUnixPathToActualAsRegex("/conf/(server-.\\.conf)") + ")",
+ new PathFilter("/conf", "server-?.conf"));
- assert regex.matcher("/conf/server-1.conf").matches();
- assert regex.matcher("/conf/server-X.conf").matches();
- assert !regex.matcher("/conf/subconf/server-1.conf").matches();
- assert !regex.matcher("/conf/server.conf").matches();
+ assert regex.matcher(translateAbsoluteUnixPathToActual("/conf/server-1.conf")).matches();
+ assert regex.matcher(translateAbsoluteUnixPathToActual("/conf/server-X.conf")).matches();
+ assert !regex.matcher(translateAbsoluteUnixPathToActual("/conf/subconf/server-1.conf")).matches();
+ assert !regex.matcher(translateAbsoluteUnixPathToActual("/conf/server.conf")).matches();
- regex = assertPatternsRegex("(/etc/(.*[^/]*\\.conf))", new PathFilter("/etc", "**/*.conf"));
+ regex = assertPatternsRegex("(" + translateAbsoluteUnixPathToActualAsRegex("/etc/(.*[^/]*\\.conf)") + ")",
+ new PathFilter("/etc", "**/*.conf"));
- assert regex.matcher("/etc/yum.conf").matches();
- assert regex.matcher("/etc/httpd/httpd.conf").matches();
- assert !regex.matcher("/etc/foo.conf/foo").matches();
+ assert regex.matcher(translateAbsoluteUnixPathToActual("/etc/yum.conf")).matches();
+ assert regex.matcher(translateAbsoluteUnixPathToActual("/etc/httpd/httpd.conf")).matches();
+ assert !regex.matcher(translateAbsoluteUnixPathToActual("/etc/foo.conf/foo")).matches();
+ }
+
+ public void testNormalizePath() throws Exception {
+ if (File.separatorChar == '\\') {
+ //windows
+ checkNormalization("\\\\server\\share\\bar", "\\\\server\\share\\path\\..\\bar");
+ //we just consider the ".." the name of the share of the UNC path
+ checkNormalization("\\\\server\\..\\bar", "\\\\server\\..\\bar");
+ checkNormalization(null, "\\\\server\\share\\..\\bar");
+ checkNormalization("C:\\bar", "C:\\foo\\..\\bar");
+ checkNormalization(null, "C:\\..\\bar");
+
+ checkNormalization("\\foo", "/foo//");
+ checkNormalization("\\foo", "/foo/./");
+ checkNormalization("\\bar", "/foo/../bar");
+ checkNormalization("\\bar", "/foo/../bar/");
+ checkNormalization("\\baz", "/foo/../bar/../baz");
+ //we just consider the "." the name of the share of the UNC path
+ checkNormalization("\\\\foo\\.\\bar", "//foo//./bar");
+ checkNormalization(null, "/../");
+ checkNormalization(null, "../foo");
+ checkNormalization("foo", "foo/bar/..");
+ checkNormalization(null, "foo/../../bar");
+ checkNormalization("bar", "foo/../bar");
+ } else {
+ checkNormalization("/foo", "/foo//");
+ checkNormalization("/foo", "/foo/./");
+ checkNormalization("/bar", "/foo/../bar");
+ checkNormalization("/bar", "/foo/../bar/");
+ checkNormalization("/baz", "/foo/../bar/../baz");
+ checkNormalization("/foo/bar", "//foo//./bar");
+ checkNormalization(null, "/../");
+ checkNormalization(null, "../foo");
+ checkNormalization("foo", "foo/bar/..");
+ checkNormalization(null, "foo/../../bar");
+ checkNormalization("bar", "foo/../bar");
+ checkNormalization("~/bar", "~/foo/../bar/");
+ }
+ }
+
+ private void checkNormalization(String expectedResult, String path) {
+ File result = FileUtil.normalizePath(new File(path));
+ assert
+ expectedResult == null ? result == null : result != null && expectedResult.equals(result.getPath()) :
+ expectedResult + " failed. Should have been [" + expectedResult + "] but was [" + result + "]";
}
private Pattern assertPatternsRegex(String expectedPattern, PathFilter... filters) {
@@ -388,4 +440,23 @@ public class FileUtilTest {
return regex;
}
+ private static String translateAbsoluteUnixPathToActualAsRegex(String path) {
+ return translateAbsoluteUnixPathToActual(path, true);
+ }
+
+ private static String translateAbsoluteUnixPathToActual(String path) {
+ return translateAbsoluteUnixPathToActual(path, false);
+ }
+
+ private static String translateAbsoluteUnixPathToActual(String path, boolean asRegex) {
+ if (File.separatorChar == '\\') {
+ //get the current drive letter
+ //leave out the trailing "\" - we have an absolute unix path on input, so we "use" the "/" of it
+ String driveLetter = new File(".").getAbsoluteFile().toPath().getRoot().toString().substring(0, 2);
+
+ path = driveLetter + path.replace("/", asRegex? "\\\\" : "\\");
+ }
+
+ return path;
+ }
}
diff --git a/modules/core/util/src/test/java/org/rhq/core/util/updater/DeployerCanonicalPathTest.java b/modules/core/util/src/test/java/org/rhq/core/util/updater/DeployerCanonicalPathTest.java
index 7a62bf0..9520da0 100644
--- a/modules/core/util/src/test/java/org/rhq/core/util/updater/DeployerCanonicalPathTest.java
+++ b/modules/core/util/src/test/java/org/rhq/core/util/updater/DeployerCanonicalPathTest.java
@@ -23,15 +23,21 @@
package org.rhq.core.util.updater;
+import static org.testng.Assert.fail;
+
import java.io.ByteArrayInputStream;
import java.io.File;
import java.io.FileInputStream;
+import java.nio.file.Files;
+import java.nio.file.Path;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
import java.util.Set;
import java.util.regex.Pattern;
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
import org.testng.annotations.BeforeClass;
import org.testng.annotations.BeforeMethod;
import org.testng.annotations.Test;
@@ -42,13 +48,16 @@ import org.rhq.core.util.file.FileUtil;
import org.rhq.core.util.stream.StreamUtil;
/**
- * Tests deploying raw files to deployment locations specified with ".." in the path.
- * This will require the deployer code to transform the paths to canonical paths.
+ * Tests deploying raw files to deployment locations specified with ".." in the path. This will require the deployer
+ * code to transform the paths to canonical paths.
*
* @author John Mazzitelli
*/
@Test
public class DeployerCanonicalPathTest {
+
+ private static final Log LOG = LogFactory.getLog(DeployerCanonicalPathTest.class);
+
private TemplateEngine templateEngine;
@BeforeClass
@@ -69,8 +78,10 @@ public class DeployerCanonicalPathTest {
public void testInitialDeployRawFilesWithCanonicalPaths() throws Exception {
File tmpDirDest = FileUtil.createTempDirectory("DeployerCanonicalPathTest", ".dest", null);
File tmpDirSrc = FileUtil.createTempDirectory("DeployerCanonicalPathTest", ".src", null);
- File rawFileRelativeDest = new File("dir-does-not-existA/../rawA.txt"); // relative to "tmpDirDest" that we just created above
- File rawFileRelativeDest2 = new File("dir-does-not-existA/../../rawA.txt"); // relative to "tmpDirDest" but it takes us above it
+ File rawFileRelativeDest = new File(
+ "dir-does-not-existA/../rawA.txt"); // relative to "tmpDirDest" that we just created above
+ File rawFileRelativeDest2 = new File(
+ "dir-does-not-existA/../../rawA.txt"); // relative to "tmpDirDest" but it takes us above it
File rawFileAbsoluteDest = new File(System.getProperty("java.io.tmpdir"), "dir-does-not-existB/../rawB.txt");
try {
@@ -102,38 +113,41 @@ public class DeployerCanonicalPathTest {
System.out.println("map-->\n" + map);
System.out.println("diff->\n" + diff);
- String rawFileRelativeDestCanonical = new File(tmpDirDest, rawFileRelativeDest.getPath()).getCanonicalPath();
- String rawFileRelativeDestCanonical2 = new File(tmpDirDest, rawFileRelativeDest2.getPath())
- .getCanonicalPath();
- String rawFileAbsoluteDestCanonical = rawFileAbsoluteDest.getCanonicalPath();
+ String rawFileRelativeDestAbsolute = FileUtil.normalizePath(new File(tmpDirDest, rawFileRelativeDest.getPath()))
+ .getAbsolutePath();
+ String rawFileRelativeDestAbsolute2 = FileUtil.normalizePath(new File(tmpDirDest, rawFileRelativeDest2.getPath()))
+ .getAbsolutePath();
+ String rawFileAbsoluteDestAbsolute = FileUtil.normalizePath(rawFileAbsoluteDest).getAbsolutePath();
assert map.size() == 3 : map;
assert map.containsKey("rawA.txt") : map;
- assert new File(rawFileRelativeDestCanonical).exists();
- assert new File(rawFileRelativeDestCanonical2).exists();
- assert MessageDigestGenerator.getDigestString(new File(rawFileRelativeDestCanonical)).equals(
+ assert new File(rawFileRelativeDestAbsolute).exists();
+ assert new File(rawFileRelativeDestAbsolute2).exists();
+ assert MessageDigestGenerator.getDigestString(new File(rawFileRelativeDestAbsolute)).equals(
map.get("rawA.txt"));
- // rawFileRelativeDestCanonical2 should be treated just like an absolute, external file
- assert MessageDigestGenerator.getDigestString(new File(rawFileRelativeDestCanonical2)).equals(
- map.get(rawFileRelativeDestCanonical2));
- assert !MessageDigestGenerator.getDigestString(testRawFileA).equals(map.get("rawA.txt")) : "should have different hash, we realize this one!";
+ // rawFileRelativeDestAbsolute2 should be treated just like an absolute, external file
+ assert MessageDigestGenerator.getDigestString(new File(rawFileRelativeDestAbsolute2)).equals(
+ map.get(rawFileRelativeDestAbsolute2));
+ assert !MessageDigestGenerator.getDigestString(testRawFileA)
+ .equals(map.get("rawA.txt")) : "should have different hash, we realize this one!";
- assert map.containsKey(rawFileAbsoluteDestCanonical) : map;
- assert new File(rawFileAbsoluteDestCanonical).exists();
- assert MessageDigestGenerator.getDigestString(new File(rawFileAbsoluteDestCanonical)).equals(
- map.get(rawFileAbsoluteDestCanonical));
- assert !MessageDigestGenerator.getDigestString(testRawFileB).equals(map.get(rawFileAbsoluteDestCanonical)) : "should have different hash, we realized this one";
+ assert map.containsKey(rawFileAbsoluteDestAbsolute) : map;
+ assert new File(rawFileAbsoluteDestAbsolute).exists();
+ assert MessageDigestGenerator.getDigestString(new File(rawFileAbsoluteDestAbsolute)).equals(
+ map.get(rawFileAbsoluteDestAbsolute));
+ assert !MessageDigestGenerator.getDigestString(testRawFileB)
+ .equals(map.get(rawFileAbsoluteDestAbsolute)) : "should have different hash, we realized this one";
assert diff.getAddedFiles().size() == 3 : diff;
assert diff.getAddedFiles().contains(diff.convertPath("rawA.txt")) : diff;
- assert diff.getAddedFiles().contains(diff.convertPath(rawFileRelativeDestCanonical2)) : diff;
- assert diff.getAddedFiles().contains(diff.convertPath(rawFileAbsoluteDestCanonical)) : diff;
+ assert diff.getAddedFiles().contains(diff.convertPath(rawFileRelativeDestAbsolute2)) : diff;
+ assert diff.getAddedFiles().contains(diff.convertPath(rawFileAbsoluteDestAbsolute)) : diff;
assert diff.getRealizedFiles().size() == 3 : diff;
assert diff.getRealizedFiles().keySet().contains(diff.convertPath("rawA.txt")) : diff;
- assert diff.getRealizedFiles().keySet().contains(diff.convertPath(rawFileRelativeDestCanonical2)) : diff;
- assert diff.getRealizedFiles().keySet().contains(diff.convertPath(rawFileAbsoluteDestCanonical)) : diff;
+ assert diff.getRealizedFiles().keySet().contains(diff.convertPath(rawFileRelativeDestAbsolute2)) : diff;
+ assert diff.getRealizedFiles().keySet().contains(diff.convertPath(rawFileAbsoluteDestAbsolute)) : diff;
} finally {
FileUtil.purge(tmpDirDest, true);
FileUtil.purge(tmpDirSrc, true);
@@ -144,8 +158,10 @@ public class DeployerCanonicalPathTest {
public void testUpdateDeployRawFileWithRelativePath() throws Exception {
File tmpDirDest = FileUtil.createTempDirectory("DeployerCanonicalPathTest", ".dest", null);
File tmpDirSrc = FileUtil.createTempDirectory("DeployerCanonicalPathTest", ".src", null);
- File rawFileRelativeDest = new File("dir-does-not-existA/../rawA.txt"); // relative to "tmpDirDest" that we just created above
- File rawFileRelativeDest2 = new File("dir-does-not-existA/../../rawA.txt"); // relative to "tmpDirDest" but it takes us above it
+ File rawFileRelativeDest = new File(
+ "dir-does-not-existA/../rawA.txt"); // relative to "tmpDirDest" that we just created above
+ File rawFileRelativeDest2 = new File(
+ "dir-does-not-existA/../../rawA.txt"); // relative to "tmpDirDest" but it takes us above it
File rawFileAbsoluteDest = new File(System.getProperty("java.io.tmpdir"), "dir-does-not-existB/../rawB.txt");
try {
@@ -176,20 +192,24 @@ public class DeployerCanonicalPathTest {
// make sure the first raw file is in the dest dir
String f = rawFileRelativeDest.getPath();
- File destFile = new File(tmpDirDest, f).getCanonicalFile(); // notice f is assumed relative to tmpDirDest, must convert to canonical path
+ File destFile = new File(tmpDirDest, f)
+ .getCanonicalFile(); // notice f is assumed relative to tmpDirDest, must convert to canonical path
assert destFile.exists() : destFile;
- FileUtil.writeFile(new ByteArrayInputStream("modifiedR".getBytes()), destFile); // change the file so we back it up during update
+ FileUtil.writeFile(new ByteArrayInputStream("modifiedR".getBytes()),
+ destFile); // change the file so we back it up during update
// make sure the second raw file, though specified originally as a relative file, is in the external location
f = rawFileRelativeDest2.getPath();
destFile = new File(tmpDirDest, f).getCanonicalFile(); // must convert to canonical path
assert destFile.exists() : destFile;
- FileUtil.writeFile(new ByteArrayInputStream("modifiedR2".getBytes()), destFile); // change the file so we back it up during update
+ FileUtil.writeFile(new ByteArrayInputStream("modifiedR2".getBytes()),
+ destFile); // change the file so we back it up during update
// make sure the third raw file is in the external location
destFile = rawFileAbsoluteDest.getCanonicalFile(); // must convert to canonical path
assert destFile.exists() : destFile;
- FileUtil.writeFile(new ByteArrayInputStream("modifiedA".getBytes()), destFile); // change the file so we back it up during update
+ FileUtil.writeFile(new ByteArrayInputStream("modifiedA".getBytes()),
+ destFile); // change the file so we back it up during update
// UPDATE
// alter the src files so we backup our changed files
@@ -206,17 +226,17 @@ public class DeployerCanonicalPathTest {
System.out.println("map-->\n" + map);
System.out.println("diff->\n" + diff);
- String rawFileRelativeDestCanonical = new File(tmpDirDest, rawFileRelativeDest.getPath())
- .getCanonicalPath();
- String rawFileRelativeDestCanonical2 = new File(tmpDirDest, rawFileRelativeDest2.getPath())
- .getCanonicalPath();
- String rawFileAbsoluteDestCanonical = rawFileAbsoluteDest.getCanonicalPath();
+ String rawFileRelativeDestAbsolute = FileUtil.normalizePath(new File(tmpDirDest, rawFileRelativeDest.getPath()))
+ .getAbsolutePath();
+ String rawFileRelativeDestAbsolute2 = FileUtil.normalizePath(new File(tmpDirDest, rawFileRelativeDest2.getPath()))
+ .getAbsolutePath();
+ String rawFileAbsoluteDestAbsolute = FileUtil.normalizePath(rawFileAbsoluteDest).getAbsolutePath();
- assert new String(StreamUtil.slurp(new FileInputStream(new File(rawFileRelativeDestCanonical))))
+ assert new String(StreamUtil.slurp(new FileInputStream(new File(rawFileRelativeDestAbsolute))))
.equals("src.modifiedR");
- assert new String(StreamUtil.slurp(new FileInputStream(new File(rawFileRelativeDestCanonical2))))
+ assert new String(StreamUtil.slurp(new FileInputStream(new File(rawFileRelativeDestAbsolute2))))
.equals("src.modifiedR2");
- assert new String(StreamUtil.slurp(new FileInputStream(new File(rawFileAbsoluteDestCanonical))))
+ assert new String(StreamUtil.slurp(new FileInputStream(new File(rawFileAbsoluteDestAbsolute))))
.equals("src.modifiedA");
boolean isWindows = File.separatorChar == '\\';
@@ -225,9 +245,9 @@ public class DeployerCanonicalPathTest {
File backupRel2;
// test the second raw file, the one that was specified originally as a relative file but took us out of the dest dir
if (!isWindows) {
- backupRel2 = new File(metadir, "1/ext-backup/" + rawFileRelativeDestCanonical2);
+ backupRel2 = new File(metadir, "1/ext-backup/" + rawFileRelativeDestAbsolute2);
} else {
- StringBuilder str = new StringBuilder(rawFileRelativeDestCanonical2);
+ StringBuilder str = new StringBuilder(rawFileRelativeDestAbsolute2);
String driveLetter = FileUtil.stripDriveLetter(str);
if (driveLetter != null) {
driveLetter = "_" + driveLetter + '/';
@@ -239,9 +259,9 @@ public class DeployerCanonicalPathTest {
// test the third raw file, the one that was specified originally as an absolute, external file
File backupAbs;
if (!isWindows) {
- backupAbs = new File(metadir, "1/ext-backup/" + rawFileAbsoluteDestCanonical);
+ backupAbs = new File(metadir, "1/ext-backup/" + rawFileAbsoluteDestAbsolute);
} else {
- StringBuilder str = new StringBuilder(rawFileAbsoluteDestCanonical);
+ StringBuilder str = new StringBuilder(rawFileAbsoluteDestAbsolute);
String driveLetter = FileUtil.stripDriveLetter(str);
if (driveLetter != null) {
driveLetter = "_" + driveLetter + '/';
@@ -259,17 +279,83 @@ public class DeployerCanonicalPathTest {
assert map.size() == 3 : map;
assert diff.getChangedFiles().size() == 3 : diff;
assert diff.getChangedFiles().contains(diff.convertPath("rawA.txt")) : diff;
- assert diff.getChangedFiles().contains(diff.convertPath(rawFileRelativeDestCanonical2)) : diff;
- assert diff.getChangedFiles().contains(diff.convertPath(rawFileAbsoluteDestCanonical)) : diff;
+ assert diff.getChangedFiles().contains(diff.convertPath(rawFileRelativeDestAbsolute2)) : diff;
+ assert diff.getChangedFiles().contains(diff.convertPath(rawFileAbsoluteDestAbsolute)) : diff;
assert diff.getDeletedFiles().isEmpty() : diff;
assert diff.getBackedUpFiles().size() == 3 : diff;
assert diff.getBackedUpFiles().keySet().contains(diff.convertPath("rawA.txt")) : diff;
- assert diff.getBackedUpFiles().keySet().contains(diff.convertPath(rawFileRelativeDestCanonical2)) : diff;
- assert diff.getBackedUpFiles().keySet().contains(diff.convertPath(rawFileAbsoluteDestCanonical)) : diff;
+ assert diff.getBackedUpFiles().keySet().contains(diff.convertPath(rawFileRelativeDestAbsolute2)) : diff;
+ assert diff.getBackedUpFiles().keySet().contains(diff.convertPath(rawFileAbsoluteDestAbsolute)) : diff;
} finally {
FileUtil.purge(tmpDirDest, true);
FileUtil.purge(tmpDirSrc, true);
rawFileAbsoluteDest.getCanonicalFile().delete();
}
}
-}
\ No newline at end of file
+
+ public void testInitialDeploymentGlossesOverSymlinksInParents() throws Exception {
+ //java7 API, but we're in tests, and require java7 to build anyway
+ Path root = FileUtil.createTempDirectory("DeployerCanonicalPathTest", ".symlink-root", null).toPath();
+ Path symlinkTarget = FileUtil.createTempDirectory("DeployerCanonicalPathTest", ".symlink-target", null)
+ .toPath();
+
+ File src = FileUtil.createTempDirectory("DeployerCanonicalPathTest", ".src", null);
+
+ Path parent = root.resolve("parent");
+ parent.toFile().mkdirs();
+
+ try {
+ File destination = null;
+
+ try {
+ destination = Files.createSymbolicLink(parent.resolve("destination"), symlinkTarget).toFile();
+ } catch (UnsupportedOperationException e) {
+ LOG.info("Skipping testInitialDeploymentGlossesOverSymlinksInParents. The current filesystem doesn't support symlinks");
+ return;
+ }
+
+ // put some source files in our tmpDirSrc location
+ File testRawFileA = new File(src, "updater-testA.txt");
+ File testRawFileA2 = new File(src, "updater-testA2.txt");
+ File testRawFileB = new File(src, "updater-testB.txt");
+ File testRawFileADest = new File(destination, "../realDest/rawA.txt");
+ File testRawFileA2Dest = new File(destination, "../realDest/rawA2.txt");
+ File testRawFileBDest = new File(destination, "../../realDest/rawB.txt");
+ FileUtil.copyFile(new File("target/test-classes/updater-testA.txt"), testRawFileA);
+ FileUtil.copyFile(new File("target/test-classes/updater-testA.txt"), testRawFileA2);
+ FileUtil.copyFile(new File("target/test-classes/updater-testB.txt"), testRawFileB);
+
+ DeploymentProperties deploymentProps = new DeploymentProperties(0, "testbundle", "1.0.test", null);
+ Set<File> zipFiles = null;
+ Map<File, File> rawFiles = new HashMap<File, File>(3);
+ rawFiles.put(testRawFileA, testRawFileADest);
+ rawFiles.put(testRawFileA2, testRawFileA2Dest);
+ rawFiles.put(testRawFileB, testRawFileBDest);
+
+ DeploymentData dd = new DeploymentData(deploymentProps, zipFiles, rawFiles, src, destination, null, null,
+ templateEngine, null, true, null);
+ Deployer deployer = new Deployer(dd);
+ DeployDifferences diff = new DeployDifferences();
+ FileHashcodeMap map = deployer.deploy(diff);
+ System.out.println("map-->\n" + map);
+ System.out.println("diff->\n" + diff);
+
+ assert map.size() == 3 : map;
+
+ assert parent.resolve("realDest/rawA.txt").toFile().exists() : "rawA.txt not deployed correctly";
+ assert parent.resolve("realDest/rawA2.txt").toFile().exists() : "rawA2.txt not deployed correctly";
+ assert root.resolve("realDest/rawB.txt").toFile().exists() : "rawB.txt not deployed correctly";
+
+ //the symlink target, being the destination of the deployment should have the .rhqdeployments directory
+ //specified. No other files should exist there though.
+ assert symlinkTarget.resolve(".rhqdeployments").toFile()
+ .exists() : "Could not find .rhqdeployments on the expected location";
+ assert symlinkTarget.toFile().listFiles().length ==
+ 1 : "The target of the symlink should have no other files than .rhqdeployments";
+ } finally {
+ FileUtil.purge(root.toFile(), true);
+ FileUtil.purge(symlinkTarget.toFile(), true);
+ FileUtil.purge(src, true);
+ }
+ }
+}
commit 4e26dfab092c2b1eede832426b6757fa4d458c72
Author: Lukas Krejci <lkrejci(a)redhat.com>
Date: Wed Jun 12 00:13:48 2013 +0200
[BZ 973415] - Empty deployments trailing behind.
If a deployment has a sole component and we delete that component, I assume
it is fairly safe to remove such deployment because it would essentially
be left empty supplying no effective configuration of the server.
Removing empty deployments fixes the strange behavior where a new
deployment was created when creating a queue, topic, datasource, connection
factory and other resources, but once such resource was deleted we would
only delete the corresponding PS component, leaving the deployment and its
config files behind. If a new attempt to create a queue/topic/etc with the
same name was made, it would fail, because a deployment with a name
auto-generated from the component name would already exist.
diff --git a/modules/plugins/jboss-as-5/src/main/java/org/rhq/plugins/jbossas5/ManagedComponentComponent.java b/modules/plugins/jboss-as-5/src/main/java/org/rhq/plugins/jbossas5/ManagedComponentComponent.java
index da0b24b..805d5b0 100644
--- a/modules/plugins/jboss-as-5/src/main/java/org/rhq/plugins/jbossas5/ManagedComponentComponent.java
+++ b/modules/plugins/jboss-as-5/src/main/java/org/rhq/plugins/jbossas5/ManagedComponentComponent.java
@@ -235,15 +235,33 @@ public class ManagedComponentComponent extends AbstractManagedComponent implemen
ManagementView managementView = getConnection().getManagementView();
managementView.removeComponent(managedComponent);
ManagedDeployment parentDeployment = managedComponent.getDeployment();
- log.debug("Redeploying parent deployment '" + parentDeployment.getName()
- + "' in order to complete removal of component " + toString(managedComponent) + "...");
- DeploymentProgress progress = deploymentManager.redeploy(parentDeployment.getName());
- DeploymentStatus redeployStatus = DeploymentUtils.run(progress);
- if (redeployStatus.isFailed()) {
- log.error("Failed to redeploy parent deployment '" + parentDeployment.getName()
- + "during removal of component " + toString(managedComponent)
- + " - removal may not persist when the app server is restarted.", redeployStatus.getFailure());
+
+ if (parentDeployment.getComponents().size() > 1 || !parentDeployment.getChildren().isEmpty()) {
+ log.debug("Redeploying parent deployment '" + parentDeployment.getName()
+ + "' in order to complete removal of component " + toString(managedComponent) + "...");
+ DeploymentProgress progress = deploymentManager.redeploy(parentDeployment.getName());
+ DeploymentStatus status = DeploymentUtils.run(progress);
+ if (status.isFailed()) {
+ log.error("Failed to redeploy parent deployment '" + parentDeployment.getName()
+ + "during removal of component " + toString(managedComponent)
+ + " - removal may not persist when the app server is restarted.", status.getFailure());
+ }
+ } else {
+ //this is the last component of the deployment and nothing would be left there after
+ //the component was removed. Let's just undeploy it in addition to removing the component.
+ //This will make sure that the deployment doesn't leave behind any defunct config files, etc.
+ log.debug("Undeploying parent deployment '" + parentDeployment.getName()
+ + "' in order to complete removal of component " + toString(managedComponent) + "...");
+ parentDeployment = managementView.getDeployment(parentDeployment.getName());
+ DeploymentProgress progress = deploymentManager.remove(parentDeployment.getName());
+ DeploymentStatus status = DeploymentUtils.run(progress);
+ if (status.isFailed()) {
+ log.error("Failed to undeploy parent deployment '" + parentDeployment.getName()
+ + "during removal of component " + toString(managedComponent)
+ + " - removal may not persist when the app server is restarted.", status.getFailure());
+ }
}
+
managementView.load();
}
