[CXF-8947] - Avoid expensive regex operations in Rfc3986UriValidator #1483
Conversation
…if URI.getHost() returns a host name Signed-off-by: Adam Anderson <[email protected]>
| if (HttpUtils.isHttpScheme(uri.getScheme())) { | ||
| // If URI.getHost() returns a host name, validate it and | ||
| // skip the expensive regular expression logic. | ||
| final String uriHost = uri.getHost(); |
There was a problem hiding this comment.
@WhiteCat22 the reason for this validator to exists sadly is the fact that Java's URI is not RFC-3986 complaint. The host is not trustful source here hence we validate it against the pattern.
There was a problem hiding this comment.
That is a good point that we had not considered.
| private static final Set<String> KNOWN_HTTP_VERBS_WITH_NO_REQUEST_CONTENT = | ||
| new HashSet<>(Arrays.asList(new String[]{"GET", "HEAD", "OPTIONS", "TRACE"})); | ||
| private static final Set<String> KNOWN_HTTP_VERBS_WITH_NO_RESPONSE_CONTENT = | ||
| new HashSet<>(Arrays.asList(new String[]{"HEAD", "OPTIONS"})); | ||
|
|
||
| private static final Pattern HTTP_SCHEME_PATTERN = Pattern.compile("^(?i)(http|https)$"); |
There was a problem hiding this comment.
This pattern is super straightforward to look up, what are exactly the gains here (vs adding the set + comparator + lowecasing)?
There was a problem hiding this comment.
Hi @reta , Sorry for the delay. The reason we changed this was because HashSet.contains(String) uses "WAY" less CPU than Pattern.matcher(String).matches()
There was a problem hiding this comment.
Fair enough, may be we could just have two constants instead?
https://issues.apache.org/jira/browse/CXF-8947