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();
}