Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Merge pull request #3134 from jglick/IdStrategy-NFE-JENKINS-47909
[JENKINS-47909] Handle false hex escapes

(cherry picked from commit 7c06a9b)
  • Loading branch information
daniel-beck authored and olivergondza committed Nov 30, 2017
1 parent 94250ec commit 8058675
Show file tree
Hide file tree
Showing 5 changed files with 34 additions and 3 deletions.
5 changes: 4 additions & 1 deletion core/src/main/java/hudson/model/User.java
Expand Up @@ -453,7 +453,9 @@ public synchronized void doSubmitDescription( StaplerRequest req, StaplerRespons
if (o instanceof User) {
if (idStrategy().equals(id, legacyUserDir.getName()) && !idStrategy().filenameOf(legacyUserDir.getName())
.equals(legacyUserDir.getName())) {
if (!legacyUserDir.renameTo(configFile.getParentFile())) {
if (legacyUserDir.renameTo(configFile.getParentFile())) {
LOGGER.log(Level.INFO, "Migrated user record from {0} to {1}", new Object[] {legacyUserDir, configFile.getParentFile()});
} else {
LOGGER.log(Level.WARNING, "Failed to migrate user record from {0} to {1}",
new Object[]{legacyUserDir, configFile.getParentFile()});
}
Expand All @@ -478,6 +480,7 @@ public synchronized void doSubmitDescription( StaplerRequest req, StaplerRespons
try {
Files.createDirectory(configFile.getParentFile().toPath());
Files.move(unsanitizedLegacyConfigFile.toPath(), configFile.toPath());
LOGGER.log(Level.INFO, "Migrated unsafe user record from {0} to {1}", new Object[] {unsanitizedLegacyConfigFile, configFile});
} catch (IOException | InvalidPathException e) {
LOGGER.log(
Level.WARNING,
Expand Down
12 changes: 10 additions & 2 deletions core/src/main/java/jenkins/model/IdStrategy.java
Expand Up @@ -303,7 +303,11 @@ public String idFromFilename(@Nonnull String filename) {
} else {
break;
}
buf.append(Character.valueOf((char)Integer.parseInt(hex.toString(), 16)));
try {
buf.append(Character.valueOf((char)Integer.parseInt(hex.toString(), 16)));
} catch (NumberFormatException x) {
buf.append('$').append(hex);
}
}
}
return buf.toString();
Expand Down Expand Up @@ -509,7 +513,11 @@ public String idFromFilename(@Nonnull String filename) {
} else {
break;
}
buf.append(Character.valueOf((char)Integer.parseInt(hex.toString(), 16)));
try {
buf.append(Character.valueOf((char)Integer.parseInt(hex.toString(), 16)));
} catch (NumberFormatException x) {
buf.append('$').append(hex);
}
}
}
return buf.toString();
Expand Down
1 change: 1 addition & 0 deletions core/src/test/java/jenkins/model/IdStrategyTest.java
Expand Up @@ -29,6 +29,7 @@ public void caseInsensitive() {
assertCaseInsensitiveRoundTrip("NUL", "$006eul");
assertEquals("foo", idStrategy.idFromFilename("~foo"));
assertEquals("0123 _-@a", idStrategy.idFromFilename("0123 _-@~a"));
assertEquals("big$money", idStrategy.idFromFilename("big$money"));
}

@Test
Expand Down
19 changes: 19 additions & 0 deletions test/src/test/java/hudson/model/UserTest.java
Expand Up @@ -813,6 +813,25 @@ public void emptyUsernameConfigMigrated() {
assertThat(empty.getFullName(), equalTo("Empty"));
}

@Issue("JENKINS-47909")
@LocalData
@Test
public void shellyUsernameMigrated() {
File rootDir = new File(Jenkins.getInstance().getRootDir(), "users");
User user = User.getById("bla$phem.us", false);
assertCorrectConfig(user, "users/bla$0024phem.us/config.xml");
assertFalse(new File(rootDir, "bla$phem.us").exists());
assertTrue(user.getConfigFile().getFile().exists());
assertThat(user.getFullName(), equalTo("Weird Username"));
user = User.getById("make\u1000000", false);
assertNotNull("we do not prevent accesses to the phony name, alas", user);
user = User.getById("make$1000000", false);
assertCorrectConfig(user, "users/make$00241000000/config.xml");
assertFalse(new File(rootDir, "make$1000000").exists());
assertTrue("but asking for the real name triggers migration", user.getConfigFile().getFile().exists());
assertThat(user.getFullName(), equalTo("Greedy Fella"));
}

private static void assertCorrectConfig(User user, String unixPath) {
assertThat(user.getConfigFile().getFile().getPath(), endsWith(unixPath.replace('/', File.separatorChar)));
}
Expand Down
Binary file not shown.

0 comments on commit 8058675

Please sign in to comment.