[GitHub] gianm commented on issue #5938: URL encode datasources, task ids, authenticator names.

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

gianm commented on issue #5938: URL encode datasources, task ids, authenticator names.
URL: https://github.com/apache/incubator-druid/pull/5938#issuecomment-409604350
 
 
   @himanshug, re:
   
   > looks like most of the fixes are in the calls to makeWorkerUrl(..), is it possible to put the fix inside impl of that method instead ?
   
   I made that change and re-pushed. I also consolidated the two makeWorkerURL methods into one helper method in TaskRunnerUtils.
   
   @jihoonson, re:
   
   > I think this is a very common mistake, so we need another way to prevent such mistakes. I suggest to add a sort of UrlBuilder which enforces encoding to input elements if needed.
   
   I considered doing this, but then we have a new problem of how to get callers to use UrlBuilder rather than making their own URLs. So I ended up just adding a urlEncode method to StringUtils and calling that where appropriate. (Note: it isn't always appropriate, since sometimes the format parameter is a literal path string like `"foo/bar"` and in that case we should _not_ url encode.)

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org