Fixes CORS handling so that it uses the defaults #19522
Conversation
…methods and http.cors.allow-headers if none are specified in the config. Closes elastic#19520
abeyad
added
>bug
:Core/Infra/REST API
| } | ||
|
|
||
| public static String[] splitStringByCommaToArray(final String s) { | ||
| if (s == null || s.isEmpty()) return Strings.EMPTY_ARRAY; | ||
| else return s.split(","); | ||
| } | ||
|
|
||
| public static Set<String> splitStringToSet(final String s, final char c) { | ||
| public static Set<String> splitStringByCommaAndTrim(final String s) { |
There was a problem hiding this comment.
can you just call spliStringToSet with true? why do we need another variant when the method this calls is public
There was a problem hiding this comment.
actually, i see 9 uses of the original method, and none look like they would want to retain whitespace. i think we should just change the behavior?
There was a problem hiding this comment.
yeah i wasn't sure what was best, most uses are reading rest request params which shouldn't have the whitespace anyway. i'm up for changing it to always do trim (unless that's the delimiter char) and not have that option on the method. that would make my extra wrapper method even more unnecessary and could go away.
|
@rjernst did the last commit address your concerns? |
|
I don't think you need the doTrim local, but other than that yes, thank you. |
the beginning and end of all split strings.
| @@ -521,16 +534,29 @@ public static String cleanPath(String path) { | |||
| final int len = chars.length; | |||
| int start = 0; // starting index in chars of the current substring. | |||
| int pos = 0; // current index in chars. | |||
| int whitespaceStart = -1; // the position of the start of the current whitespace, -1 if not on whitespace | |||
There was a problem hiding this comment.
Instead of this, I think you could do just like with start, and have an end var that marks the current end of the token, and do not increment it when you find whitespace?
There was a problem hiding this comment.
This means the size of the token would always be end - start too.
|
LGTM, thanks! |
|
@rjernst thanks for the review! |
|
@jasontedor @abeyad has this been ported to netty 4? |
|
thx @jasontedor I see you checked the box on the meta issue too ;) |
Fixes CORS handling so that it uses the defaults for for http.cors.allow-methods
and http.cors.allow-headers if none are specified in the config.
Closes #19520