[GitHub] [atlas] vladhlinsky commented on a change in pull request #137: ATLAS-4263: Fix KafkaUtils to always enclose property values in double-quotes

[GitBox <gi...@apache.org>]

vladhlinsky commented on a change in pull request #137:
URL: https://github.com/apache/atlas/pull/137#discussion_r621897707



##########
File path: common/src/main/java/org/apache/atlas/utils/KafkaUtils.java
##########
@@ -318,20 +318,9 @@ static String surroundWithQuotes(String optionVal) {
             return optionVal;
         }
 
-        String ret = optionVal;
-
-        // For property values which have special chars like "@" or "/", we need to enclose it in
-        // double quotes, so that Kafka can parse it
-        // If the property is already enclosed in double quotes, then do nothing.
-        if (optionVal.indexOf(0) != '"' && optionVal.indexOf(optionVal.length() - 1) != '"') {

Review comment:
       Probably, the `charAt` method could be used instead of `indexOf` , but I think we should omit this check at all, since the option value may contain double quotes at the beginning/at the end or in the middle of the string.
   Thus, the following option values look valid to me:
   ```
   user1
   "user2
   user3"
   user"4
   "user5" (the actual value is "user5", not user5)
   ```




-- 
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:
users@infra.apache.org