tillrohrmann commented on a change in pull request #10532: 
[FLINK-15053][runtime] Escape all dynamical property values for taskmanager
URL: https://github.com/apache/flink/pull/10532#discussion_r360350401
 
 

 ##########
 File path: 
flink-runtime/src/main/java/org/apache/flink/runtime/clusterframework/BootstrapTools.java
 ##########
 @@ -621,23 +621,38 @@ public Config getAkkaConfig() {
         *
         * @param baseConfig The base configuration.
         * @param targetConfig The target configuration.
-        * @return Dynamic properties as string, separated by whitespace.
+        * @return Dynamic properties as string array.
         */
-       public static String getDynamicProperties(Configuration baseConfig, 
Configuration targetConfig) {
+       public static String[] getDynamicProperties(Configuration baseConfig, 
Configuration targetConfig) {
 
-               String[] newAddedConfigs = 
targetConfig.keySet().stream().flatMap(
+               return targetConfig.keySet().stream().flatMap(
                        (String key) -> {
                                final String baseValue = 
baseConfig.getString(ConfigOptions.key(key).stringType().noDefaultValue());
                                final String targetValue = 
targetConfig.getString(ConfigOptions.key(key).stringType().noDefaultValue());
 
                                if (!baseConfig.keySet().contains(key) || 
!baseValue.equals(targetValue)) {
                                        return Stream.of("-" + 
CommandLineOptions.DYNAMIC_PROPERTY_OPTION.getOpt() + key +
-                                               
CommandLineOptions.DYNAMIC_PROPERTY_OPTION.getValueSeparator() + targetValue);
+                                               
CommandLineOptions.DYNAMIC_PROPERTY_OPTION.getValueSeparator() + 
escapeDynamicPropertyValue(targetValue));
                                } else {
                                        return Stream.empty();
                                }
                        })
                        .toArray(String[]::new);
-               return String.join(" ", newAddedConfigs);
+       }
+
+       /**
+        * Escape all the dynamic property values. Each value will be 
surrounded with single quotes. This works for all chars
+        * except single quote itself. To escape the single quote, close the 
quoting before it, insert the escaped single
+        * quote, and then re-open the quoting. For example, the value is 
my'value and the escaped value is 'my'\''value'.
+        *
+        * @param value value to be escaped
+        * @return escaped value
+        */
+       private static String escapeDynamicPropertyValue(String value) {
+               if (value.contains("'")) {
+                       return "'" + value.replace("'", "'\\''") + "'";
+               } else {
+                       return "'" + value + "'";
+               }
 
 Review comment:
   Nit: One could also think about using Guava's `Escaper` which might be a bit 
faster than Java regex replace.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[email protected]


With regards,
Apache Git Services

Reply via email to