gnodet-bot commented on code in PR #28749:
URL: https://github.com/apache/camel/pull/28749#discussion_r4429439231
##########
components/camel-as2/camel-as2-api/src/main/java/org/apache/camel/component/as2/api/entity/EntityParser.java:
##########
@@ -637,10 +647,33 @@ public static MultipartSignedEntity
parseMultipartSignedEntityBody(
parseException.initCause(e);
throw parseException;
} finally {
+ inbuffer.stopRawCapture();
inbuffer.setCharsetDecoder(previousDecoder);
}
}
+ /**
+ * Removes the boundary delimiter line a body-part parser consumed last,
and the line break preceding it, from the
+ * raw bytes of the part - both belong to the boundary (RFC 2046, 5.1.1).
Returns the bytes unchanged when the last
+ * line read is not that delimiter.
+ */
+ static byte[] stripBoundaryDelimiter(byte[] raw, int lastLineRawLength,
String boundary) {
+ if (raw == null || lastLineRawLength <= 0 || lastLineRawLength >
raw.length) {
+ return raw;
+ }
+ int end = raw.length - lastLineRawLength;
+ String lastLine = new String(raw, end, lastLineRawLength,
StandardCharsets.US_ASCII);
+ if (!lastLine.startsWith("--" + boundary)) {
+ return raw;
+ }
+ if (end >= 2 && raw[end - 2] == CR && raw[end - 1] == LF) {
+ end -= 2;
+ } else if (end >= 1 && raw[end - 1] == LF) {
+ end -= 1;
Review Comment:
💡 **Minor:** The method handles CRLF and bare-LF line endings but not a
bare-CR (`\r` without a following `\n`). RFC 2046 allows CR and LF to appear
individually in some contexts. This is likely an edge case in practice (most
AS2 implementations use CRLF), but worth noting if strict RFC compliance is
desired. A third branch after L672 could handle it:
```
} else if (end >= 1 && raw[end - 1] == CR) {
end -= 1;
}
```
##########
components/camel-as2/camel-as2-api/src/main/java/org/apache/camel/component/as2/api/entity/MultipartSignedEntity.java:
##########
@@ -41,6 +44,40 @@ protected MultipartSignedEntity(ContentType contentType,
String contentTransferE
this.isMainBody = isMainBody;
}
+ /**
+ * The first body part exactly as received - its MIME headers and its body
in the transfer encoding the sender used
+ * - which is what the signature was created over (RFC 1847, 2.1) and what
the MIC is computed over (RFC 4130, 7.1).
+ * Set by the parser; null for an entity built locally.
+ */
+ public byte[] getSignedDataRaw() {
+ return signedDataRaw;
Review Comment:
💡 **Suggestion:** `getSignedDataRaw()` returns the internal array directly —
callers that mutate it would corrupt the stored raw bytes used for
signature/MIC verification. Consider returning a defensive copy:
```suggestion
return signedDataRaw != null ? signedDataRaw.clone() : null;
```
Or document that the returned array must not be modified (weaker guarantee,
but acceptable if callers are trusted).
##########
components/camel-as2/camel-as2-api/src/main/java/org/apache/camel/component/as2/api/io/AS2SessionInputBuffer.java:
##########
@@ -233,12 +239,50 @@ public boolean isLastLineReadEnrichedByCarriageReturn() {
return lastLineReadEnrichedByCarriageReturn;
}
+ /**
+ * Starts keeping the raw bytes every following {@link
#readLine(CharArrayBuffer, InputStream)} consumes - line
+ * terminators included, no charset decoding - until {@link
#stopRawCapture()}.
+ */
+ public void startRawCapture() {
+ this.rawCapture = new ByteArrayBuffer(this.buffer.length);
Review Comment:
💡 **Minor:** `startRawCapture()` unconditionally overwrites any active
capture — if called twice without an intervening `stopRawCapture()`, the first
capture is silently discarded. This can't happen today given the single
call-site in `EntityParser`, but a guard or at least a comment would make the
contract clearer:
```suggestion
this.rawCapture = new ByteArrayBuffer(this.buffer.length); //
silently replaces any active capture
```
##########
components/camel-as2/camel-as2-api/src/main/java/org/apache/camel/component/as2/api/entity/MultipartSignedEntity.java:
##########
@@ -41,6 +44,40 @@ protected MultipartSignedEntity(ContentType contentType,
String contentTransferE
this.isMainBody = isMainBody;
}
+ /**
+ * The first body part exactly as received - its MIME headers and its body
in the transfer encoding the sender used
+ * - which is what the signature was created over (RFC 1847, 2.1) and what
the MIC is computed over (RFC 4130, 7.1).
+ * Set by the parser; null for an entity built locally.
+ */
+ public byte[] getSignedDataRaw() {
+ return signedDataRaw;
+ }
+
+ public void setSignedDataRaw(byte[] signedDataRaw) {
Review Comment:
💡 **Suggestion:** `setSignedDataRaw` is `public` but is only called from
`EntityParser`. Consider narrowing the visibility to package-private — it
prevents external code from overwriting the canonical raw bytes after parsing:
```suggestion
void setSignedDataRaw(byte[] signedDataRaw) {
```
Both classes are in the same package (`entity`), so `EntityParser` can still
call it.
--
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]