PatternSyntaxException may be thrown when looking for an environment entry or metric with a name that looks a bit like a regular expression #9730
Conversation
spring-projects-issues
added
the
status: waiting-for-triage
|
Interesting. Thanks for the PR. Just to help us understand the nature of the problem, can you please share an example of an endpoint call you were making where the current behaviour is inadequate? |
wilkinsona
added
the
status: waiting-for-feedback
|
For example, |
|
I get that something like |
…can be compiled * Code Format * Catch PatternSyntaxException instead of RuntimeException * Fix Checkstyle violations Revert "* Catch PatternSyntaxException instead of RuntimeException" This reverts commit 0b85e4f. * Ctach PatternSyntaxException instead of RuntimeException * Fix checkstyle violations * Delete flag parameter
….com/DylianBEGO/spring-boot.git into improve-regex-detection-for-endpoint
|
No, i don't have a key like this. I think it's more robust to not throw an exception when someone makes a mistake. For example, if we put What do you think ? |
|
It's a little bit tricky. On the one hand, if you pass in something that was intended to be a regex but you mess up the syntax, a bad request response would be helpful. On the other hand, we don't want to prevent people from searching for keys that look a bit like a regular expression… |
|
While I agree with the helpfulness of the bad request response, I don't see how this is possible without breaking the api. If we had a specific query param for the regex, we could then be sure of the intentions of the user and return an error in the case of an invalid regex. But with the path param syntax serving as a filter on keys with a IMO, this PR is the best we can do without breaking the api, we won't support my example but at least it won't throw expressions that look like a regex but are not. We could introduce a new query param for a regex on top of the existing behaviour I guess, although I'm not a fan of this solution as I think it's cumbersome when searching (having to put a query param instead of a path param). |
|
Thanks. In the course of some work on the Actuator for 2.0, it's become apparent that we really need to move to a query parameter for the pattern. Please see #9796 for more details on that. For 1.5.x, I agree that what you're proposing here is the best we can do. |
wilkinsona
added
priority: normal
and removed
status: waiting-for-feedback
wilkinsona
changed the title
|
Glad to hear that! Any change to get it merged in the 1.4.x branch as well? We're not planning on updating to 1.5 in the near future and I'd like to see that fix :) |
|
No, sorry. As mentioned when it was announced, 1.4.7 was the last 1.4.x release. |
|
Oh, that's too bad... Thanks for answer, I've missed this detail. |
Previously, if a name contained part of a regex but wasn't actually a regex, a PatternSyntaxException would be thrown and the request would fail. This commit updates NamePatternFilter to catch PatternSyntaxException and treat the regex-like input as a name insteead. See gh-9730
* gh-9730: Polish "Handle possible regexes defensively in NamePatternFilter" Handle possible regexes defensively in NamePatternFilter
|
Thanks for the PR, @DylianBEGO. I've merged it into 1.5.x and master with some minor polishing. |
[spring-projects#9730] in MVC endpoint * Bump to version 1.4.10.BUILD-COVEO [AUTH-610]
Sometimes, Spring detects a string as a regex because it contains regex parts. The problem is this string is not a regex and a PatternSyntaxException is raised when pattern try to compile it.
This modification checks if the regex compilation is possible, else we consider it is just a simple string.