ben-roling commented on a change in pull request #3441:
URL: https://github.com/apache/storm/pull/3441#discussion_r839822444
##########
File path: storm-webapp/src/main/java/org/apache/storm/daemon/ui/UIHelpers.java
##########
@@ -1746,19 +1746,21 @@ private static Double nullToZero(Double value) {
}
/**
- * sanitizeStreamName.
- * @param streamName streamName
- * @return sanitizeStreamName
+ * Sanitizes streamName for use as an identifier in the visualization.
Replaces all characters except A-Z, a-z,
+ * '.', '-', and '_' with '_'. If streamName does not start with A-Z or
a-z, adds '_s' prefix.
+ * @param streamName non-null, non-empty streamName
+ * @return sanitized stream name
*/
public static String sanitizeStreamName(String streamName) {
- Pattern pattern = Pattern.compile("(?![A-Za-z_\\-:\\.]).");
- Pattern pattern2 = Pattern.compile("^[A-Za-z]");
- Matcher matcher = pattern2.matcher(streamName);
- Matcher matcher2 = pattern.matcher("\\s" + streamName);
- if (matcher.find()) {
- matcher2 = pattern.matcher(streamName);
- }
- return matcher2.replaceAll("_");
+ if (streamName == null || streamName.isEmpty()) {
+ throw new IllegalArgumentException("streamName: " + streamName ==
null ? "null" : "empty");
Review comment:
I've restored parity with how null and empty were handled before any of
my changes:
https://github.com/apache/storm/pull/3441/commits/177976b929bf18dd0542bb4131cd801e99a827f7
If streamName is null, it will result in NPE just as it did before. I don't
think there is any way streamName would ever be null. I don't think it would
ever be empty either, but since I have at least a small degree of uncertainty
this way we can be sure it at least behaves the same way it used to.
--
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.
To unsubscribe, e-mail: dev-unsubscr...@storm.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
