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 <mark.earl.waite@gmail.com>
  • Loading branch information
Seros and MarkEWaite authored 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"
+ "IF %ARG:~0,8%==Password type " + windowsArgEncodeFileName(passwordFile.getRemote()), null);
}
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("'")) {
filename = filename.replace("'", "'\\''");
}
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("\"")) {
filename = filename.replace("\"", "^\"");
}
return "\"" + filename + "\"";
}
}

// 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.