fix: shell command string will now have single quotes sanitized

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 fabric8io#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. fabric8io#4535 iss-4535 Altered some catch statements to catch exceptions rather than throwables for sonar fabric8io#4535
BrianDevC · Nov 4, 2022 · 3105c42 · 3105c42
1 parent 96039b9
commit 3105c42
Show file tree

Hide file tree

Showing 4 changed files with 31 additions and 11 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -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

diff --git a/...nt/src/main/java/io/fabric8/kubernetes/client/dsl/internal/core/v1/PodOperationsImpl.java b/...nt/src/main/java/io/fabric8/kubernetes/client/dsl/internal/core/v1/PodOperationsImpl.java
@@ -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);
     }
   }
 
@@ -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);
     }
   }
 
@@ -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);
     }
   }
 
@@ -591,7 +591,7 @@ public BytesLimitTerminateTimeTailPrettyLoggable usingTimestamps() {
   }
 
   public static String shellQuote(String value) {
-    return "'" + value.replace("'", "'\\\\''") + "'";
+    return "'" + value.replace("'", "'\\''") + "'";
   }
 
   @Override

diff --git a/...ent/src/test/java/io/fabric8/kubernetes/client/dsl/internal/uploadable/PodUploadTest.java b/...ent/src/test/java/io/fabric8/kubernetes/client/dsl/internal/uploadable/PodUploadTest.java
@@ -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);

diff --git a/kubernetes-itests/src/test/java/io/fabric8/kubernetes/PodIT.java b/kubernetes-itests/src/test/java/io/fabric8/kubernetes/PodIT.java
@@ -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 {