sagarmiglani commented on code in PR #53:
URL:
https://github.com/apache/sling-org-apache-sling-xss/pull/53#discussion_r1919996539
##########
src/main/java/org/apache/sling/xss/impl/xml/Regexp.java:
##########
@@ -27,12 +27,19 @@ public class Regexp {
private String name;
private String value;
+ private Pattern pattern;
+
@JsonCreator
public Regexp(@JacksonXmlProperty(localName = "name", isAttribute = true)
String name,
@JacksonXmlProperty(localName = "value", isAttribute = true)
String regexp) {
this.name = name;
this.value = regexp;
+
+ if (regexp != null) {
Review Comment:
Thanks, @rombert and @joerghoh. I was expecting a discussion around this.
Yes, as per config.xml, the regexp can be `null`. However, in my opinion, the
usage of `getPattern` may not have been thoroughly considered, as I can see it
being used in streams. If the value of a `regexp` is `null`, it could lead to
an exception, potentially terminating the stream even if the other `regexp` in
attribute are not null, which might not have been anticipated.
With the current approach, if the regular expression is `null`, a `null`
pattern is returned, ensuring more predictable behavior. We should also
consider addressing this other places as well (like:
https://github.com/apache/sling-org-apache-sling-xss/blob/master/src/main/java/org/apache/sling/xss/impl/xml/Property.java#L121).
Please let me know your thoughts.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
