Skip to content

Commit 78a71d8

Browse files
committed
Cleanup: make Path.contains() more precise.
Same problem as in maven-scm's SCM-695: to check whether one path refers to a parent directory of another path, it behooves one well to handle file separators, otherwise a directory path "foo/bar" may be seen incorrectly as a parent to be stripped from the file path "foo/barbar/someFile.txt".
1 parent e41072e commit 78a71d8

File tree

3 files changed

+36
-10
lines changed

3 files changed

+36
-10
lines changed

src/main/java/hudson/plugins/scm_sync_configuration/JenkinsFilesHelper.java

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,17 +6,26 @@
66

77
public class JenkinsFilesHelper {
88

9-
public static String buildPathRelativeToHudsonRoot(File file){
9+
public static String buildPathRelativeToHudsonRoot(File file) {
1010
File jenkinsRoot = Jenkins.getInstance().getRootDir();
11-
if(!file.getAbsolutePath().startsWith(jenkinsRoot.getAbsolutePath())){
11+
String jenkinsRootPath = jenkinsRoot.getAbsolutePath();
12+
String fileAbsolutePath = file.getAbsolutePath();
13+
if (fileAbsolutePath.equals(jenkinsRootPath)) {
14+
// Hmmm. Should never occur.
15+
throw new IllegalArgumentException("Cannot build relative path to $JENKINS_HOME for $JENKINS_HOME itself; would be empty.");
16+
}
17+
if (!jenkinsRootPath.endsWith(File.separator)) {
18+
jenkinsRootPath += File.separator;
19+
}
20+
if (!fileAbsolutePath.startsWith(jenkinsRootPath)) {
21+
// Oops, the file is not relative to $JENKINS_HOME
1222
return null;
1323
}
14-
String truncatedPath = file.getAbsolutePath().substring(jenkinsRoot.getAbsolutePath().length()+1); // "+1" because we don't need ending file separator
15-
return truncatedPath.replaceAll("\\\\", "/");
24+
String truncatedPath = fileAbsolutePath.substring(jenkinsRootPath.length());
25+
return truncatedPath.replace(File.separatorChar, '/');
1626
}
1727

1828
public static File buildFileFromPathRelativeToHudsonRoot(String pathRelativeToJenkinsRoot){
19-
File jenkinsRoot = Jenkins.getInstance().getRootDir();
20-
return new File(jenkinsRoot.getAbsolutePath(), pathRelativeToJenkinsRoot);
29+
return new File(Jenkins.getInstance().getRootDir(), pathRelativeToJenkinsRoot);
2130
}
2231
}

src/main/java/hudson/plugins/scm_sync_configuration/ScmSyncConfigurationBusiness.java

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -166,9 +166,15 @@ private void processCommitsQueue() {
166166
for(Commit commit: currentCommitQueue){
167167
String logMessage = "Processing commit : " + commit.toString();
168168
LOGGER.finest(logMessage);
169-
170169
// Preparing files to add / delete
170+
//
171+
// Two points:
172+
// 1. getPathContents() already returns only those paths that are not also to be deleted.
173+
// 2. For svn, we must not run svn add for files already in the repo. For git, we should run git add to stage the
174+
// change. The second happens to work per chance because the git checkIn implementation will use git commit -a
175+
// if a file set without files but only some directory is given, which we do.
171176
List<File> updatedFiles = new ArrayList<File>();
177+
172178
for(Map.Entry<Path,byte[]> pathContent : commit.getChangeset().getPathContents().entrySet()){
173179
Path pathRelativeToJenkinsRoot = pathContent.getKey();
174180
byte[] content = pathContent.getValue();

src/main/java/hudson/plugins/scm_sync_configuration/model/Path.java

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ public Path(File hudsonFile){
2424
}
2525

2626
public Path(String path, boolean isDirectory) {
27-
this.path = path;
27+
this.path = path.replace(File.separatorChar, '/'); // Make sure we use the system-independent separator.
2828
this.isDirectory = isDirectory;
2929
}
3030

@@ -39,7 +39,7 @@ public File getHudsonFile(){
3939
public File getScmFile(){
4040
// TODO: Externalize ScmSyncConfigurationBusiness.getCheckoutScmDirectoryAbsolutePath()
4141
// in another class ?
42-
return new File(ScmSyncConfigurationBusiness.getCheckoutScmDirectoryAbsolutePath()+File.separator+getPath());
42+
return new File(ScmSyncConfigurationBusiness.getCheckoutScmDirectoryAbsolutePath(), getPath());
4343
}
4444

4545
public String getFirstNonExistingParentScmPath(){
@@ -59,7 +59,18 @@ public boolean isDirectory() {
5959
}
6060

6161
public boolean contains(Path p){
62-
return this.isDirectory() && p.getPath().startsWith(this.getPath());
62+
if (this.isDirectory()) {
63+
String path = this.getPath();
64+
if (!path.endsWith("/")) {
65+
path += '/';
66+
}
67+
String otherPath = p.getPath();
68+
if (p.isDirectory() && !otherPath.endsWith("/")) {
69+
otherPath += '/';
70+
}
71+
return otherPath.startsWith(path);
72+
}
73+
return false;
6374
}
6475

6576
@Override

0 commit comments

Comments
 (0)