kirktrue commented on code in PR #16990:
URL: https://github.com/apache/kafka/pull/16990#discussion_r1733554093


##########
clients/src/main/java/org/apache/kafka/common/security/oauthbearer/internals/secured/AccessTokenRetrieverFactory.java:
##########
@@ -70,6 +73,8 @@ public static AccessTokenRetriever create(Map<String, ?> 
configs,
             if (jou.shouldCreateSSLSocketFactory(tokenEndpointUrl))
                 sslSocketFactory = jou.createSSLSocketFactory();
 
+            boolean urlencodeHeader = validateUrlencodeHeader(cu);

Review Comment:
   I've seen the use of `urlencode` (all one token) as a fairly accepted way to 
join the words 'URL' and 'encode'. For example:
   
   * 
[IANA](https://www.iana.org/assignments/media-types/application/x-www-form-urlencoded)
   * [PHP](https://www.php.net/urlencode)
   * 
[Python](https://docs.python.org/3/library/urllib.parse.html#urllib.parse.urlencode)
   
   In the JDK’s `URLEncoder` class, it references the MIME type `application/ 
x-www-form-urlencoded`.
   
   In addition, the original code that introduced the encoding option uses the 
configuration `SASL_OAUTHBEARER_HEADER_URLENCODE`, which should arguably be 
`SASL_OAUTHBEARER_HEADER_URL_ENCODE`.
   
   Personally, `urlencode` is a bit funny-looking to me, but I was attempting 
to maintain internal consistency with what appears to be an accepted way to 
capitalize that token.
   
   Is it OK to leave it as is for now?



-- 
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.

To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to