[Fix-16682][ApiServer] like query with % support special characher(% \ _) #16683
Conversation
shouwangyw
requested review from
caishunfeng,
SbloodyS and
Gallardot
as code owners
| public static String handleEscapes(String str) { | ||
| str = StringUtils.trim(str); | ||
| if (StringUtils.isNotBlank(str)) { | ||
| str = str.replace("\\", "\\\\"); | ||
| str = str.replace("_", "\\_"); | ||
| str = str.replace("%", "\\%"); | ||
| str = str.replaceAll("[\n|\r\t]", ""); |
There was a problem hiding this comment.
This is a general processing method, and we should not modify it. Workflow name and task name should not contain special characters such as %. We should add verification and documentation when creating it.
There was a problem hiding this comment.
This is a general processing method, and we should not modify it. Workflow name and task name should not contain special characters such as
%. We should add verification and documentation when creating it.
I understand that this method is intended for special characters, and should be able to support like fuzzy queries.
And for general processing the character _ and \ should also be supported.This is not just a processing for workflow name and task name,validation should be implemented elsewhere.
Here are some self-tests：
There was a problem hiding this comment.
Different fields should use different verification methods. For example, the workflow name should consist of uppercase and lowercase English letters or Chinese and underscores, and other symbols are illegal characters. But the workflow description can contain more special characters.
There was a problem hiding this comment.
Different fields should use different verification methods. For example, the workflow name should consist of uppercase and lowercase English letters or Chinese and underscores, and other symbols are illegal characters. But the workflow description can contain more special characters.
I understand this, but this general method is not for verification, but for supporting special characters in like fuzzy queries.
There was a problem hiding this comment.
This is a general processing method, and we should not modify it. Workflow name and task name should not contain special characters such as
%. We should add verification and documentation when creating it.
This issue only solves the problem of workflow list query. We should follow this, and the rest do not belong to this issue.
There was a problem hiding this comment.
This issue is not just about solving the workflow list query problem, the issue only takes the workflow list as an example. This is indeed a general process.
If so, we need to analyze specific problems, because different query scenarios need to use different rules. The current implementation needs optimization.
This process involves many calls, and every time you write a new query list interface, you may need to call this method.
This may not be a good implementation, but it is the smallest change,for the time being, this is also implemented in our actual project
There was a problem hiding this comment.
Since the caller of this method is too extensive, it is impossible to comprehensively evaluate the influence surface. In order to reduce the impact and achieve the optimization effect, it is better to deal with it separately.
There was a problem hiding this comment.
Since the caller of this method is too extensive, it is impossible to comprehensively evaluate the influence surface. In order to reduce the impact and achieve the optimization effect, it is better to deal with it separately.
sure, and some places have not been searched by like, i will check it again.
There was a problem hiding this comment.
I have checked all the calling places, and most of them use like % query. In addition, remove some don't need % escapr processing. @SbloodyS
There was a problem hiding this comment.
I have checked all the calling places, and most of them use like % query. In addition, remove some don't need % escapr processing. @SbloodyS
Since the caller of this method is too extensive, it is impossible to comprehensively evaluate the influence surface. In order to reduce the impact and achieve the optimization effect, it is better to deal with it separately.
The scope of this revision is relatively large and it does not belong to the description scope of this issue #16682 . In this Fix PR, we only need to extract the query of workflow list separately and fix it. The other is to create a DSIP for scheme design and discussion.
SbloodyS
changed the title
Like text query condition support (% \ _)Like text query condition support (% \ _)
shouwangyw
changed the title
Like text query condition support (% \ _)Like fuzzy query support (% \ _)
shouwangyw
changed the title
Like fuzzy query support (% \ _)
shouwangyw
changed the title
|
This caused by the specifical meaning of _ and % in mysql, there might exist other symbosl same with these. I didn't find out good idea to solve these problem, seems we need to escape these characters, but this will introduce more problems. |
This is not just a problem in MySQL,the PG used in our project have The same problem, and it has been fixed and is currently in use online. Our project testers initially discovered this issue and recorded a bug for us to fix. |
My concern is we escape the search condition will cause more bugs, I would prefer the user use the escaped keyword in front-end. On the other hand, we cannot escape all parameters, this bug only affect |
Front-end escaping maybe is not suitable, this change will only be used in like fuzzy queries, others that do not require escaping are also removed. |
|
Purpose of the pull request
This pull request fix the querying list, text query condition use 'like' query does not support (% \ _)
Fix #16682
Brief change log
Change ParameterUtils#handleEscapes
Verify this pull request
This pull request is code cleanup without any test coverage.
(or)
This pull request is already covered by existing tests, such as (please describe tests).
(or)
This change added tests and can be verified as follows:
(or)
Pull Request Notice
Pull Request Notice
If your pull request contain incompatible change, you should also add it to
docs/docs/en/guide/upgrede/incompatible.md