Skip to content

Commit

Permalink
fix: shell command string will now have single quotes sanitized
Browse files Browse the repository at this point in the history
Single quotes in folder or file names were causing corruption of the shell command.
The fix changes the ' character to '"'"'
For example a folder called te'st would now get shellQuoted to 'te'"'"'st'
1. ' ends the first quote,
2. " starts second quote,
3. ' is valid character in double quotes
4. " ends the second quote
5. ' starts the third quote
6. The end result when fed into the shell, is as we desired: te'st

#4535

iss-4535

Changed to be much shorter as pointed out, manual still passes tests.
New test added
Changelog moved.

iss-4535

Fixed test brackets that mvn spotless:apply unfortunately didn't.

#4535

iss-4535

Altered some catch statements to catch exceptions rather than throwables for sonar

#4535
  • Loading branch information
BrianDevC authored and manusa committed Nov 4, 2022
1 parent 96039b9 commit 12cd4ca
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 11 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -4,6 +4,7 @@

#### Bugs
* Fix #4534: Java Generator CLI default handling of skipGeneratedAnnotations
* Fix #4535: The shell command string will now have single quotes sanitized
* Fix #4547: preventing timing issues with leader election cancel

#### Improvements
Expand Down
Expand Up @@ -185,26 +185,26 @@ public Loggable withLogWaitTimeout(Integer logWaitTimeout) {
public PortForward portForward(int port, ReadableByteChannel in, WritableByteChannel out) {
try {
return new PortForwarderWebsocket(httpClient, this.context.getExecutor()).forward(getResourceUrl(), port, in, out);
} catch (Throwable t) {
throw KubernetesClientException.launderThrowable(t);
} catch (Exception e) {
throw KubernetesClientException.launderThrowable(e);
}
}

@Override
public LocalPortForward portForward(int port) {
try {
return new PortForwarderWebsocket(httpClient, this.context.getExecutor()).forward(getResourceUrl(), port);
} catch (Throwable t) {
throw KubernetesClientException.launderThrowable(t);
} catch (Exception e) {
throw KubernetesClientException.launderThrowable(e);
}
}

@Override
public LocalPortForward portForward(int port, int localPort) {
try {
return new PortForwarderWebsocket(httpClient, this.context.getExecutor()).forward(getResourceUrl(), port, localPort);
} catch (Throwable t) {
throw KubernetesClientException.launderThrowable(t);
} catch (Exception e) {
throw KubernetesClientException.launderThrowable(e);
}
}

Expand Down Expand Up @@ -274,8 +274,8 @@ public ExecWatch exec(String... command) {
try {
URL url = getExecURLWithCommandParams(actualCommands);
return setupConnectionToPod(url.toURI());
} catch (Throwable t) {
throw KubernetesClientException.launderThrowable(forOperationType("exec"), t);
} catch (Exception e) {
throw KubernetesClientException.launderThrowable(forOperationType("exec"), e);
}
}

Expand All @@ -284,8 +284,8 @@ public ExecWatch attach() {
try {
URL url = getAttachURL();
return setupConnectionToPod(url.toURI());
} catch (Throwable t) {
throw KubernetesClientException.launderThrowable(forOperationType("attach"), t);
} catch (Exception e) {
throw KubernetesClientException.launderThrowable(forOperationType("attach"), e);
}
}

Expand Down Expand Up @@ -591,7 +591,7 @@ public BytesLimitTerminateTimeTailPrettyLoggable usingTimestamps() {
}

public static String shellQuote(String value) {
return "'" + value.replace("'", "'\\\\''") + "'";
return "'" + value.replace("'", "'\\''") + "'";
}

@Override
Expand Down
Expand Up @@ -179,6 +179,24 @@ void createExecCommandForUpload_withNormalFile_shouldCreateValidExecCommandForUp
assertThat(result, equalTo("mkdir -p '/tmp/foo' && base64 -d - > '/tmp/foo/cp.log'"));
}

@Test
void createExecCommandForUpload_withSingleQuoteInPath() {
// When
String result = PodUpload.createExecCommandForUpload("/tmp/fo'o/cp.log");

// Then
assertThat(result, equalTo("mkdir -p '/tmp/fo\'\\'\'o' && base64 -d - > '/tmp/fo\'\\'\'o/cp.log'"));
}

@Test
void createExecCommandForUpload_withMultipleSingleQuotesInPath() {
// When
String result = PodUpload.createExecCommandForUpload("/tmp/f'o'o/c'p.log");

// Then
assertThat(result, equalTo("mkdir -p '/tmp/f\'\\'\'o\'\\'\'o' && base64 -d - > '/tmp/f\'\\'\'o\'\\'\'o/c\'\\'\'p.log'"));
}

void uploadFileAndVerify(PodUploadTester<Boolean> fileUploadMethodToTest) throws IOException, InterruptedException {
this.operation = operation.file("/mock/dir/file");
WebSocket.Builder builder = Mockito.mock(WebSocket.Builder.class, Mockito.RETURNS_SELF);
Expand Down
Expand Up @@ -274,6 +274,7 @@ void uploadFile() throws IOException {

assertUploaded("pod-standard", tmpFile, "/tmp/toBeUploaded");
assertUploaded("pod-standard", tmpFile, "/tmp/001_special_!@#\\$^&(.mp4");
assertUploaded("pod-standard", tmpFile, "/tmp/002'special");
}

private void assertUploaded(String podName, final Path tmpFile, String filename) throws IOException {
Expand Down

0 comments on commit 12cd4ca

Please sign in to comment.