allthingssecurity commented on code in PR #27342:
URL: https://github.com/apache/camel/pull/27342#discussion_r4181332873
##########
components/camel-cloudevents/src/main/java/org/apache/camel/component/cloudevents/transformer/CloudEventJsonDataTypeTransformer.java:
##########
@@ -106,11 +105,52 @@ private String createCouldEventJsonObject(Map<String,
Object> cloudEventAttribut
return builder.append("}").toString();
}
- private boolean isJson(String data) {
- if (data == null || data.isEmpty()) {
+ /**
+ * Appends the value as a Json string, escaping the quote, the backslash
and the control characters (RFC 8259,
+ * section 7).
+ */
+ private static void appendJsonString(StringBuilder builder, String value) {
+ builder.append('"');
+ for (int i = 0; i < value.length(); i++) {
+ char ch = value.charAt(i);
+ switch (ch) {
+ case '"' -> builder.append("\\\"");
+ case '\\' -> builder.append("\\\\");
+ case '\n' -> builder.append("\\n");
+ case '\r' -> builder.append("\\r");
+ case '\t' -> builder.append("\\t");
+ case '\b' -> builder.append("\\b");
+ case '\f' -> builder.append("\\f");
+ default -> {
+ if (ch < 0x20) {
+ builder.append(String.format("\\u%04x", (int) ch));
+ } else {
+ builder.append(ch);
+ }
+ }
+ }
+ }
+ builder.append('"');
+ }
+
+ /**
+ * Whether the data is a Json object or array, which is then set as nested
Json value. Text that only starts like
+ * Json (such as a log line "[INFO] ...") is set as a Json string.
+ */
+ private static boolean isJson(String data) {
+ if (data == null || data.isBlank()) {
return false;
}
- return data.trim().startsWith("{") || data.trim().startsWith("[");
+ String trimmed = data.trim();
+ if (!trimmed.startsWith("{") && !trimmed.startsWith("[")) {
+ return false;
+ }
+ try {
+ Jsoner.deserialize(trimmed);
Review Comment:
Agreed, the full parse was wasteful. Replaced in 7b4f6ceb1df5 by a single
pass that validates the RFC 8259 grammar
(objects, arrays, strings and escapes, numbers, literals, commas, colons,
matching brackets, only whitespace after the
value) without building anything; the only allocation is a 32-entry nesting
stack, grown for deeper data.
Rough numbers on a 1 MB JSON array (5,901 objects; JDK 21; median of 50 runs
after warm-up; scratch test, not
committed):
| check | time per call | allocated per call |
|---|---|---|
| `Jsoner.deserialize` (previous commit) | 8.6 to 9.0 ms | 28 MB |
| validating scan (now) | 1.2 ms | ~0 |
| whole transformation with the scan | 1.3 ms | |
Against the original `startsWith` check the scan adds about 1.2 ms per MB of
JSON body; a body that does not start with
`{` or `[` is not scanned.
I did not take the "trust `datacontenttype`" shortcut alone: the transformer
defaults `datacontenttype` to
`application/json` when the header is absent, so the default case (the
`[INFO] ...` log line from the JIRA) would be
nested unchecked again, and a malformed body declared as JSON would still
give an invalid event. A side benefit: the
scan is stricter than `Jsoner`, which accepts `[1 2]`, `{"a" 1}`, `[1,]`,
`[01]`, `["a\x"]` and raw control characters
in strings. The previous commit nested such bodies and the event was invalid
JSON; they are now written as a string.
If you would rather have zero cost when the content type is explicitly JSON
(skip the scan for an explicit
`application/json` / `*+json` header, scan only when it is absent or not
JSON), that is a small change. The cost is that
a malformed body declared as JSON gives an invalid event. Your call, I can
switch either way. The trade-off is now in the
PR description. Tests added: a truncated body declared `application/json` is
written as a string and the event parses;
unit tests of the scan with valid data (all number forms and escapes, 100
levels deep) and 48 invalid texts. Module suite:
14 tests, 0 failures.
_Claude Code on behalf of allthingssecurity_
--
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]
