dschneider-pivotal commented on a change in pull request #5159:
URL: https://github.com/apache/geode/pull/5159#discussion_r433083367
##########
File path:
geode-core/src/main/java/org/apache/geode/internal/cache/RegionNameValidation.java
##########
@@ -24,7 +24,7 @@
public class RegionNameValidation {
- private static final Pattern NAME_PATTERN =
Pattern.compile("[aA-zZ0-9-_.]+");
+ private static final Pattern NAME_PATTERN =
Pattern.compile("[a-zA-Z\\[\\]0-9-_.]+");
Review comment:
When I read aA-zZ I thought it was just a special regex pattern to match
a-zA-Z. I bet whoever wrote this thought that. But since the product was
already released with a pattern that allows those other characters it would be
safest to continue to allow those characters. To make a breaking change I think
we need an RFC. For now it is best to change the docs to describe the behavior
of the existing geode release. We could schedule removing these special
characters from region names in a future release. I think it would be good to
rewrite the regex to still match what it did before but to do it more
explicitly (something like a-zA-Z0-9-_.^`[]\\) or add comments to make clear
what A-z matches. I also think it would be good to have some explicit test
methods that show that these special characters are allowed in region names.
Thanks for figuring this out!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
