From 7968e3f38cdcaab582ccf61ca49bf85ed3da728c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erik=20M=C3=BCller?= <9016208+Seros@users.noreply.github.com> Date: Wed, 12 Jul 2023 02:03:30 +0200 Subject: [PATCH] =?UTF-8?q?Escape=20special=20characters=20for=20GitUserna?= =?UTF-8?q?mePasswordBinding=20in=20withCrede=E2=80=A6=20(#1443)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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 --- .../git/GitUsernamePasswordBinding.java | 41 +++++++++++++++---- .../git/GitUsernamePasswordBindingTest.java | 13 ++++-- 2 files changed, 42 insertions(+), 12 deletions(-) diff --git a/src/main/java/jenkins/plugins/git/GitUsernamePasswordBinding.java b/src/main/java/jenkins/plugins/git/GitUsernamePasswordBinding.java index 2c5f41ee5a..a0ee7e3161 100644 --- a/src/main/java/jenkins/plugins/git/GitUsernamePasswordBinding.java +++ b/src/main/java/jenkins/plugins/git/GitUsernamePasswordBinding.java @@ -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; } @@ -170,6 +175,26 @@ protected FilePath write(StandardUsernamePasswordCredentials credentials, FilePa protected Class 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 diff --git a/src/test/java/jenkins/plugins/git/GitUsernamePasswordBindingTest.java b/src/test/java/jenkins/plugins/git/GitUsernamePasswordBindingTest.java index c6cd22103a..46654be9d5 100644 --- a/src/test/java/jenkins/plugins/git/GitUsernamePasswordBindingTest.java +++ b/src/test/java/jenkins/plugins/git/GitUsernamePasswordBindingTest.java @@ -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), @@ -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) { @@ -309,7 +312,7 @@ 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)); @@ -317,11 +320,13 @@ public void test_GenerateGitScript_write() throws IOException, InterruptedExcept 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)); } /**