bdemers commented on code in PR #707:
URL: https://github.com/apache/directory-scimple/pull/707#discussion_r1904517935
##########
scim-spec/scim-spec-schema/src/main/java/org/apache/directory/scim/spec/filter/ExpressionBuildingListener.java:
##########
@@ -160,14 +154,8 @@ public FilterExpression getFilterExpression() {
private static Object parseJsonType(String jsonValue) {
if (jsonValue.startsWith("\"")) {
- String doubleEscaped = jsonValue.substring(1, jsonValue.length() - 1)
- // StringEscapeUtils follows the outdated JSON spec requiring "/" to
be escaped, this could subtly break things
- .replaceAll("\\\\/", "\\\\\\\\/")
- // Just in case someone needs a single-quote with a backslash in
front of it, this will be unnecessary with escapeJson()
- .replaceAll("\\\\'", "\\\\\\\\'");
-
- // TODO change this to escapeJson() when dependencies get upgraded
- return StringEscapeUtils.unescapeEcmaScript(doubleEscaped);
Review Comment:
I was hoping @chrisharm would chime in 🤞
My take is that the escape would happen at a different level:
- filters passed in as a query param would be URL escaped
- Patch paths, would be JSON escaped automatically by Jackson.
Unless I'm missing something i think it would always be problematic to
handle escaping outside of the context where it's used.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
