Skip to content

Commit

Permalink
Escape special characters for GitUsernamePasswordBinding in withCrede… (
Browse files Browse the repository at this point in the history
#1443)

* Escape special characters for GitUsernamePasswordBinding in withCredentials

* Fix test for GitUsernamePasswordBinding in withCredentials

* Adapt writing ASKPASS from git-client-plugin

* Fix tests

* Reduce diffs by retaining old, ugly formatting of comment

In the future, the repository will be formatted with spotless and that
ugliness will be banished forever.  Helps my code review to make the
diffs small now.

* Make filename encoding methods private

No need to make them visible outside the class where they are used.

Also adds the same comment on these methods as is used on the methods
in the git client plugin so that future consumers will know to not use
them for any other purpose than their current very limited use.

* Add more sample passwords

* Move assertions into existing conditional

---------

Co-authored-by: Mark Waite <[email protected]>
  • Loading branch information
Seros and MarkEWaite committed Jul 12, 2023
1 parent c75dea6 commit 7968e3f
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 12 deletions.
41 changes: 33 additions & 8 deletions src/main/java/jenkins/plugins/git/GitUsernamePasswordBinding.java
Original file line number Diff line number Diff line change
Expand Up @@ -145,23 +145,28 @@ protected GenerateGitScript(String gitUsername, String gitPassword,
protected FilePath write(StandardUsernamePasswordCredentials credentials, FilePath workspace)
throws IOException, InterruptedException {
FilePath gitEcho;

FilePath usernameFile = workspace.createTempFile("username", ".txt");
usernameFile.write(this.userVariable + "\n", null);
FilePath passwordFile = workspace.createTempFile("password", ".txt");
passwordFile.write(this.passVariable + "\n", null);

//Hard Coded platform dependent newLine
if (this.unixNodeType) {
gitEcho = workspace.createTempFile("auth", ".sh");
// [#!/usr/bin/env sh] to be used if required, could have some corner cases
gitEcho.write("case $1 in\n"
+ " Username*) echo " + this.userVariable
+ " ;;\n"
+ " Password*) echo " + this.passVariable
+ " ;;\n"
gitEcho.write("#!/bin/sh\n"
+ "\n"
+ "case \"$1\" in\n"
+ " Username*) cat " + unixArgEncodeFileName(usernameFile.getRemote()) + ";;\n"
+ " Password*) cat " + unixArgEncodeFileName(passwordFile.getRemote()) + ";;\n"
+ " esac\n", null);
gitEcho.chmod(0500);
} else {
gitEcho = workspace.createTempFile("auth", ".bat");
gitEcho.write("@ECHO OFF\r\n"
+ "SET ARG=%~1\r\n"
+ "IF %ARG:~0,8%==Username (ECHO " + this.userVariable + ")\r\n"
+ "IF %ARG:~0,8%==Password (ECHO " + this.passVariable + ")", null);
+ "IF %ARG:~0,8%==Username type " + windowsArgEncodeFileName(usernameFile.getRemote()) + "\r\n"

Check warning on line 168 in src/main/java/jenkins/plugins/git/GitUsernamePasswordBinding.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered line

Line 168 is not covered by tests
+ "IF %ARG:~0,8%==Password type " + windowsArgEncodeFileName(passwordFile.getRemote()), null);

Check warning on line 169 in src/main/java/jenkins/plugins/git/GitUsernamePasswordBinding.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered line

Line 169 is not covered by tests
}
return gitEcho;
}
Expand All @@ -170,6 +175,26 @@ protected FilePath write(StandardUsernamePasswordCredentials credentials, FilePa
protected Class<StandardUsernamePasswordCredentials> type() {
return StandardUsernamePasswordCredentials.class;
}

/* Escape all single quotes in filename, then surround filename in single quotes.
* Only useful to prepare filename for reference from a shell script.
*/
private String unixArgEncodeFileName(String filename) {
if (filename.contains("'")) {

Check warning on line 183 in src/main/java/jenkins/plugins/git/GitUsernamePasswordBinding.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line

Line 183 is only partially covered, one branch is missing
filename = filename.replace("'", "'\\''");

Check warning on line 184 in src/main/java/jenkins/plugins/git/GitUsernamePasswordBinding.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered line

Line 184 is not covered by tests
}
return "'" + filename + "'";
}

/* Escape all double quotes in filename, then surround filename in double quotes.
* Only useful to prepare filename for reference from a DOS batch file.
*/
private String windowsArgEncodeFileName(String filename) {
if (filename.contains("\"")) {

Check warning on line 193 in src/main/java/jenkins/plugins/git/GitUsernamePasswordBinding.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line

Line 193 is only partially covered, 2 branches are missing
filename = filename.replace("\"", "^\"");

Check warning on line 194 in src/main/java/jenkins/plugins/git/GitUsernamePasswordBinding.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered line

Line 194 is not covered by tests
}
return "\"" + filename + "\"";

Check warning on line 196 in src/main/java/jenkins/plugins/git/GitUsernamePasswordBinding.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered line

Line 196 is not covered by tests
}
}

// Mistakenly defined GitUsernamePassword in first release, prefer gitUsernamePassword as symbol
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,8 +116,10 @@ private boolean isTimeAvailable() {
"&Ampersand&",
"He said \"Hello\", then left.",
"default=@#(*^!",
"has_a_trailing_quote=@#(*^!'",
"here's-a-quote",
"special%%_342@**",
"%interior-single-quote%_786'@**",
};
private static GitTool[] gitTools = {
new GitTool("Default", "git", null),
Expand All @@ -126,10 +128,11 @@ private boolean isTimeAvailable() {
new JGitTool(),
};

/* Create two test data items using random selections from the larger set of data */
/* Create three test data items using random selections from the larger set of data */
private static Object[][] testData = new Object[][]{
{userNames[random.nextInt(userNames.length)], passwords[random.nextInt(passwords.length)], gitTools[random.nextInt(gitTools.length)]},
{userNames[random.nextInt(userNames.length)], passwords[random.nextInt(passwords.length)], gitTools[random.nextInt(gitTools.length)]},
{userNames[random.nextInt(userNames.length)], passwords[random.nextInt(passwords.length)], gitTools[random.nextInt(gitTools.length)]},
};

public GitUsernamePasswordBindingTest(String username, String password, GitTool gitToolInstance) {
Expand Down Expand Up @@ -309,19 +312,21 @@ public void test_getGitClientInstance() throws IOException, InterruptedException
}

@Test
public void test_GenerateGitScript_write() throws IOException, InterruptedException {
public void test_GenerateGitScript_write() throws Exception {
assumeTrue("Test class max time " + MAX_SECONDS_FOR_THESE_TESTS + " exceeded", isTimeAvailable());
GitUsernamePasswordBinding.GenerateGitScript tempGenScript = new GitUsernamePasswordBinding.GenerateGitScript(this.username, this.password, credentials.getId(), !isWindows());
assertThat(tempGenScript.type(), is(StandardUsernamePasswordCredentials.class));
FilePath tempScriptFile = tempGenScript.write(credentials, rootFilePath);
if (!isWindows()) {
assertThat(tempScriptFile.mode(), is(0500));
assertThat("File extension not sh", FilenameUtils.getExtension(tempScriptFile.getName()), is("sh"));
assertThat(tempScriptFile.readToString(), containsString("Username*) cat"));
assertThat(tempScriptFile.readToString(), containsString("Password*) cat"));
} else {
assertThat("File extension not bat", FilenameUtils.getExtension(tempScriptFile.getName()), is("bat"));
assertThat(tempScriptFile.readToString(), containsString("IF %ARG:~0,8%==Username type"));
assertThat(tempScriptFile.readToString(), containsString("IF %ARG:~0,8%==Password type"));
}
assertThat(tempScriptFile.readToString(), containsString(this.username));
assertThat(tempScriptFile.readToString(), containsString(this.password));
}

/**
Expand Down

0 comments on commit 7968e3f

Please sign in to comment.