-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Escape column names and paths in order by clause with backticks #38645
Escape column names and paths in order by clause with backticks #38645
Conversation
…event potential hql injection
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, thanks! I'll let @FroMage give his opinion on the disableEscaping()
method, if any.
Status for workflow
|
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, thanks!
|
||
private static StringBuilder escapeColumnName(String columnName) { | ||
StringBuilder sb = new StringBuilder(); | ||
String[] path = columnName.split("\\."); |
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.
Can we have dots in property names? I assume not. I'm asking because if there's a way to escape dots like \.
in column names, then we'd have to respect that, but I don't think column names support dot escaping.
I just cheked the HQL grammar and there are no escaped dots, so we're good.
This change includes previous escaping implementation as well as handles sorting by embedded columns which was the reason for the issue reopening.
Sort.disableEscaping() method was introduced to give a simple workaround to the users who face any other future regression issues.
An integration test was added to replicate and cover #38521