New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
SQL: fix handling of escaped chars in JDBC connection string #58429
SQL: fix handling of escaped chars in JDBC connection string #58429
Conversation
This commit fixes an issue emerging when the connection string URI contains escaped characters. The original URI is pre-parsed in order to re-assemble a new URI having the optional elements filled in with defaults. The new URI has been using however the unescaped query and fragment parts. So if these contained any escaped `&` or `=` (such as in the password option value), the unescaping would reveal them and make them later interfere with the options parsing. The commit changes that, so that the new URI be built from the unescaped "raw" parts of the original URI.
@elasticmachine update branch |
@elasticmachine run elasticsearch-ci/default-distro |
@elasticmachine update branch |
Pinging @elastic/es-ql (:Query Languages/SQL) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. left a minor comment.
@@ -208,7 +215,8 @@ public boolean indexIncludeFrozen() { | |||
} | |||
|
|||
public static boolean canAccept(String url) { | |||
return (StringUtils.hasText(url) && url.trim().startsWith(JdbcConfiguration.URL_PREFIX)); | |||
return (StringUtils.hasText(url) && |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
minor: I would trim the url and use a local var to avoid trimming it potentially twice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
- prevent double trimming of the connection string; - improve docs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thx for the randomization of the prefix in the tests!
…#58429) * Fix: preserve URI query and fragment char escaping This commit fixes an issue emerging when the connection string URI contains escaped characters. The original URI is pre-parsed in order to re-assemble a new URI having the optional elements filled in with defaults. The new URI has been using however the unescaped query and fragment parts. So if these contained any escaped `&` or `=` (such as in the password option value), the unescaping would reveal them and make them later interfere with the options parsing. The commit changes that, so that the new URI be built from the unescaped "raw" parts of the original URI. (cherry picked from commit 94eb5a0)
…#58429) * Fix: preserve URI query and fragment char escaping This commit fixes an issue emerging when the connection string URI contains escaped characters. The original URI is pre-parsed in order to re-assemble a new URI having the optional elements filled in with defaults. The new URI has been using however the unescaped query and fragment parts. So if these contained any escaped `&` or `=` (such as in the password option value), the unescaping would reveal them and make them later interfere with the options parsing. The commit changes that, so that the new URI be built from the unescaped "raw" parts of the original URI. (cherry picked from commit 94eb5a0)
…58429) (#58977) SQL: fix handling of escaped chars in JDBC connection string (#58429) This commit fixes an issue emerging when the connection string URI contains escaped characters. The original URI is pre-parsed in order to re-assemble a new URI having the optional elements filled in with defaults. The new URI has been using however the unescaped query and fragment parts. So if these contained any escaped `&` or `=` (such as in the password option value), the unescaping would reveal them and make them later interfere with the options parsing. The commit changes that, so that the new URI be built from the unescaped "raw" parts of the original URI. (cherry picked from commit 94eb5a0)
…58429) (#58976) SQL: fix handling of escaped chars in JDBC connection string (#58429) This commit fixes an issue emerging when the connection string URI contains escaped characters. The original URI is pre-parsed in order to re-assemble a new URI having the optional elements filled in with defaults. The new URI has been using however the unescaped query and fragment parts. So if these contained any escaped `&` or `=` (such as in the password option value), the unescaping would reveal them and make them later interfere with the options parsing. The commit changes that, so that the new URI be built from the unescaped "raw" parts of the original URI. (cherry picked from commit 94eb5a0)
This PR fixes an issue emerging when the connection string URI
contains escaped characters.
The original URI is pre-parsed in order to re-assemble a new URI having
the optional elements filled in with defaults. The new URI has been
using however the unescaped query and fragment parts. So if these
contained any escaped
&
or=
(such as in the password option value),the unescaping would reveal them and make them later interfere with the
options parsing.
The PR changes that, so that the new URI be built from the unescaped
"raw" parts of the original URI.
Closes #57927