You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cxf.apache.org by "ffang (via GitHub)" <gi...@apache.org> on 2026/10/18 19:42:28 UTC
[PR] CXF-9258: Fix WS-RM retransmission of messages with attachments [cxf]
ffang opened a new pull request, #3598:
URL: https://github.com/apache/cxf/pull/3598
AttachmentOutInterceptor replaces the SOAP Content-Type with the multipart wrapper and marks the message as already processed, so a retransmitted message was sent without its attachments, or with a nested multipart, both when it was resent from memory and when it was recovered from the RMStore.
In-memory path:
- Save the bare SOAP Content-Type (SAVED_CONTENT_TYPE) before AttachmentOutInterceptor replaces it
- On resend, clear ATTACHMENT_OUT_CHECKED and the AttachmentSerializer content and restore the saved Content-Type, so the multipart is rebuilt correctly
Persisted path (PersistenceUtils):
- encodeRMContent writes the root part with the SOAP Content-Type instead of the multipart one, keeps the root part encoding, and treats an empty attachment collection as no attachments
- decodeRMContent restores the root Content-Type from the stored message
- For inbound (destination) messages the root type is taken from ATTACHMENT_PART_HEADERS, with the charset stripped
- The root type is never a multipart type: if it cannot be determined, fall back to text/xml with a warning so a multipart is never nested
- The root part of a captured MTOM message is stored as an XOP part, and decodeRMContent unwraps the SOAP type from an XOP root part and enables MTOM, so a recovered MTOM message is retransmitted as an XOP package
Tests:
- PersistenceUtilsTest: round trips, SOAP 1.2 action and charset handling, inbound path, missing root type, encoding, MTOM/XOP round trips
- RetransmissionAttachmentTest: SwA and MTOM retransmission, and retransmission of SwA and MTOM messages recovered from the client store after a restart
- ServerAttachmentRecoveryTest: an inbound message with an attachment that could not be delivered is persisted by the destination and redelivered with its attachment after a server restart
- The recovery tests use their own in-memory database per test, as an in-memory HSQLDB database lives as long as the JVM
--
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-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CXF-9258: Fix WS-RM retransmission of messages with attachments [cxf]
Posted by "github-advanced-security[bot] (via GitHub)" <gi...@apache.org>.
github-advanced-security[bot] commented on code in PR #3598:
URL: https://github.com/apache/cxf/pull/3598#discussion_r4316822028
##########
rt/ws/rm/src/main/java/org/apache/cxf/ws/rm/persistence/PersistenceUtils.java:
##########
@@ -103,22 +119,146 @@
return context;
}
+ /**
+ * Content type of the SOAP root message (without the multipart wrapper), or null if unknown.
+ * Captured outbound messages provide it via SAVED_CONTENT_TYPE; for inbound multipart
+ * messages it is taken from the headers of the root part. The result is never a multipart type,
+ * as the multipart wrapper is recreated when the message is written, so a multipart type would
+ * end up nested into another multipart message.
+ */
+ private static String getRootContentType(Message msg) {
+ String ct = (String)msg.get(RMMessageConstants.SAVED_CONTENT_TYPE);
+ if (ct == null) {
+ Map<String, List<String>> ih =
+ CastUtils.cast((Map<?, ?>)msg.get(AttachmentDeserializer.ATTACHMENT_PART_HEADERS));
+ List<String> types = ih != null ? ih.get("Content-Type") : null;
+ if (types != null && !types.isEmpty()) {
+ // the charset is added again by the AttachmentSerializer when the message is written
+ ct = removeCharset(types.get(0));
+ }
+ }
+ if (ct == null) {
+ ct = (String) msg.get(Message.CONTENT_TYPE);
+ }
+ if (isMultipart(ct)) {
+ return getMultipartRootType(ct);
+ }
+ return ct;
+ }
+
+ /**
+ * Content type of the root part of a multipart message of the given type, used when the multipart
+ * type is the only known type, like when it was stored as the type of the root part. The start-info
+ * parameter written by the AttachmentSerializer holds the SOAP content type with its parameters,
+ * like the SOAP 1.2 action, and the type parameter is the one of the root part, application/xop+xml
+ * for an XOP package.
+ */
+ private static String getMultipartRootType(String multipartType) {
+ String rootType = null;
+ try {
+ MimeType mt = new MimeType(multipartType);
+ String type = mt.getParameter("type");
+ String startInfo = mt.getParameter("start-info");
+ if (XOP_CONTENT_TYPE.equalsIgnoreCase(type)) {
+ MimeType xop = new MimeType(XOP_CONTENT_TYPE);
+ if (startInfo != null) {
+ xop.setParameter("type", removeCharset(startInfo));
+ }
+ rootType = xop.toString();
+ } else {
+ String soapType = startInfo != null ? startInfo : type;
+ if (soapType != null && !isMultipart(soapType)) {
+ rootType = removeCharset(soapType);
+ }
+ }
+ } catch (MimeTypeParseException e) {
+ LOG.log(Level.FINE, "Invalid multipart Content-Type " + multipartType, e);
Review Comment:
## CodeQL / Insertion of sensitive information into log files
This [potentially sensitive information](1) is written to a log file.
[Show more details](https://github.com/apache/cxf/security/code-scanning/2254)
##########
rt/ws/rm/src/main/java/org/apache/cxf/ws/rm/persistence/PersistenceUtils.java:
##########
@@ -103,22 +119,146 @@
return context;
}
+ /**
+ * Content type of the SOAP root message (without the multipart wrapper), or null if unknown.
+ * Captured outbound messages provide it via SAVED_CONTENT_TYPE; for inbound multipart
+ * messages it is taken from the headers of the root part. The result is never a multipart type,
+ * as the multipart wrapper is recreated when the message is written, so a multipart type would
+ * end up nested into another multipart message.
+ */
+ private static String getRootContentType(Message msg) {
+ String ct = (String)msg.get(RMMessageConstants.SAVED_CONTENT_TYPE);
+ if (ct == null) {
+ Map<String, List<String>> ih =
+ CastUtils.cast((Map<?, ?>)msg.get(AttachmentDeserializer.ATTACHMENT_PART_HEADERS));
+ List<String> types = ih != null ? ih.get("Content-Type") : null;
+ if (types != null && !types.isEmpty()) {
+ // the charset is added again by the AttachmentSerializer when the message is written
+ ct = removeCharset(types.get(0));
+ }
+ }
+ if (ct == null) {
+ ct = (String) msg.get(Message.CONTENT_TYPE);
+ }
+ if (isMultipart(ct)) {
+ return getMultipartRootType(ct);
+ }
+ return ct;
+ }
+
+ /**
+ * Content type of the root part of a multipart message of the given type, used when the multipart
+ * type is the only known type, like when it was stored as the type of the root part. The start-info
+ * parameter written by the AttachmentSerializer holds the SOAP content type with its parameters,
+ * like the SOAP 1.2 action, and the type parameter is the one of the root part, application/xop+xml
+ * for an XOP package.
+ */
+ private static String getMultipartRootType(String multipartType) {
+ String rootType = null;
+ try {
+ MimeType mt = new MimeType(multipartType);
+ String type = mt.getParameter("type");
+ String startInfo = mt.getParameter("start-info");
+ if (XOP_CONTENT_TYPE.equalsIgnoreCase(type)) {
+ MimeType xop = new MimeType(XOP_CONTENT_TYPE);
+ if (startInfo != null) {
+ xop.setParameter("type", removeCharset(startInfo));
+ }
+ rootType = xop.toString();
+ } else {
+ String soapType = startInfo != null ? startInfo : type;
+ if (soapType != null && !isMultipart(soapType)) {
+ rootType = removeCharset(soapType);
+ }
+ }
+ } catch (MimeTypeParseException e) {
+ LOG.log(Level.FINE, "Invalid multipart Content-Type " + multipartType, e);
Review Comment:
## CodeQL / Log Injection
This log entry depends on a [user-provided value](1).
This log entry depends on a [user-provided value](2).
This log entry depends on a [user-provided value](3).
[Show more details](https://github.com/apache/cxf/security/code-scanning/2255)
--
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-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CXF-9258: Fix WS-RM retransmission of messages with attachments [cxf]
Posted by "ffang (via GitHub)" <gi...@apache.org>.
ffang commented on PR #3598:
URL: https://github.com/apache/cxf/pull/3598#issuecomment-6270538638
> **Bug (checked)**
>
> 1. The charset-stripping regex breaks quoted type values (PersistenceUtils.java:64-65, used at :145)
> CHARSET_PARAM’s unquoted branch [^;\s]* also matches a closing ". Take an XOP root part from a non-CXF peer like:
>
> application/xop+xml; type="application/soap+xml; charset=utf-8"
>
> The regex removes ; charset=utf-8", closing quote included, and leaves application/xop+xml; type="application/soap+xml. TYPE_PARAM’s quoted branch then fails to match. The unquoted branch returns "application/soap+xml with a stray leading quote, and that ends up in CONTENT_TYPE / SAVED_CONTENT_TYPE on redelivery. Changing the unquoted branch to [^;\s"]* fixes it, or the parameters could be parsed with jakarta.activation.MimeType. A test case for this input would be worth adding.
>
> **Possible issues (not checked)**
>
> 1. The text/xml fallback ignores the SOAP version (PersistenceUtils.java:157, :173). If the stored type is multipart, which is what earlier CXF versions persisted, or an XOP root has no type parameter, the root type becomes text/xml. A recovered SOAP 1.2 message would then be resent as text/xml and lose its action. The stored multipart type’s type/start-info parameters usually hold the correct value and could be used instead.
> 2. Single-use attachment streams on resend (RetransmissionQueueImpl.java:817). Clearing ATTACHMENT_OUT_CHECKED makes every in-memory resend write the original DataHandlers again. A DataSource backed by a stream that can only be read once would send an empty attachment or fail. The only sign of the failure would be a logged RESEND_FAILED_MSG.
> 3. The outbound check depends on CONTENT_TYPE (RMCaptureOutInterceptor.java:226). SAVED_CONTENT_TYPE is set only when CONTENT_TYPE is non-null, and encodeRMContent uses its presence to mean “this message is outbound”. An outbound MTOM message that has no Content-Type yet would be stored as plain SwA rather than XOP. Checking MessageUtils.isOutbound(msg) would avoid this.
> 4. Regex parsing of Content-Type parameters (PersistenceUtils.java:142). Joining several root Content-Type header values with "; " can produce strings like text/xml; text/xml. A real MIME type parser would be more robust; this is the same root cause as item 1.
> 5. Duplicate persistence path (DestinationSequence.java:193). Its own no-attachments branch still stores the raw Message.CONTENT_TYPE and skips the new getRootContentType normalisation. Calling encodeRMContent unconditionally would keep the two paths consistent.
>
> **Minor** Redundant line (RetransmissionQueueImpl.java:818). message.removeContent(AttachmentSerializer.class) repeats what the loop over getContentFormats() a few lines below already does.
Thanks for the thorough review, @coheigea. I've pushed a follow-up commit, "CXF-9258: Address review comments".
### Bug: the charset regex breaks quoted `type` values
Confirmed, good catch. Your input produced `"application/soap+xml` with a stray quote. The regex also removed `charset` from *inside* the quoted `type` value. It no longer uses regular expressions: the Content-Type parameters are now parsed with `jakarta.activation.MimeType`. So only the top-level `charset` is removed, and escaped quotes in quoted values (like the SOAP 1.2 `action`) are handled by the parser. I added your example as a test (`testDecodeInboundXopRootWithCharsetInType`), plus one with an escaped action.
### 1. The `text/xml` fallback ignores the SOAP version
Agreed. It's worse than losing the action: a SOAP 1.2 envelope resent as `text/xml` gets a fault from the server and is retried until it gives up. When only a multipart type is available, which is what earlier versions stored, the root type now comes from that type:
- `start-info`, which keeps the parameters such as the action;
- otherwise `type`;
- for `type="application/xop+xml"`, an XOP root type built from `start-info`, so a recovered MTOM message is resent as an XOP package again.
`text/xml` is only the last resort. New tests decode messages stored the way earlier versions did, for SOAP 1.1, SOAP 1.2 and SOAP 1.2 with MTOM.
### 2. Single-use attachment streams on resend
Agreed this is real, but it isn't new in this PR. Before it, retransmissions didn't include the attachments at all. It's also worse than a resend problem: with a persistent store, `encodeRMContent` reads the attachments at `WRITE_ENDING`, before `AttachmentOutEndingInterceptor` writes them. So a one-shot stream is already empty on the *first* transmission, and that's true on 4.2.4 too.
The main real-world case is WS-Security, whose secured attachments can only be read once. I'm handling that in [CXF-6955](https://issues.apache.org/jira/browse/CXF-6955), which builds on this PR, so I'd rather keep it out of this one. Making arbitrary user `DataSource`s retransmittable would mean caching every attachment at capture time; that could be a separate follow-up if we think it's worth the cost.
### 3. The outbound check depends on `CONTENT_TYPE`
For SOAP this can't happen today: `SoapPreProtocolOutInterceptor` sets the Content-Type at `POST_LOGICAL`, before `RMCaptureOutInterceptor` runs at `PRE_STREAM`. But `MessageUtils.isOutbound(msg)` states the intent much more clearly, so XOP storage now uses it. When the root type has to come from the multipart type, it can already be an XOP type, so it's unwrapped before the serializer adds XOP again. Tests cover both cases.
### 4. Regex parsing of Content-Type parameters
Fixed with the same `MimeType` change. Only the first `Content-Type` value of the root part is used instead of joining all of them, and an invalid value falls back to its type/subtype.
### 5. Duplicate persistence path in `DestinationSequence`
Agreed. A new `PersistenceUtils.encodeRMContent(RMMessage, Message, CachedOutputStream)` overload stores the content as is when there are no attachments, so no extra copy, but with the normalised root type. `DestinationSequence` now calls it with no branch of its own.
### Minor: redundant `removeContent(AttachmentSerializer.class)`
Removed, thanks.
The same notes still apply:
- It says the commit is pushed, but dc1f17a374 is local only.
- The CXF-6955 paragraph under item 2 can be cut if you'd rather not mention it yet.
- @coheigea is Colm's GitHub handle from the review; it will notify him.
✻ Sautéed for 10s · done 10:12 AM
❯ what this means " /** Encodes a message with an attachment like earlier versions, with the multipart type as root type */
- private static RMMessage encodeEarlierVersion(String multipartType) throws IOException {
"
That helper builds test data in the format CXF used to store messages before this PR, so the tests can check that the new code still decodes it.
What older versions stored. Before CXF-9258, encodeRMContent copied Message.CONTENT_TYPE into the root part of the stored message. By capture time, AttachmentOutInterceptor has already replaced that value with the multipart wrapper type. So the root part's Content-Type was itself a multipart type, nested inside the stored multipart. This is the old code (HEAD~1 before the PR):
msgImpl1.put(Message.CONTENT_TYPE, msg.get(Message.CONTENT_TYPE)); // already "multipart/related; ..."
AttachmentSerializer serializer = new AttachmentSerializer(msgImpl1);
serializer.setXop(false);
The stored content then looks roughly like this (SOAP 1.2 example):
--uuid:...
Content-Type: multipart/related; charset=UTF-8; type="application/soap+xml"; boundary="uuid:1";
start="<ro...@cxf.apache.org>"; start-info="application/soap+xml; action=\"urn:test\""
Content-ID: <ro...@cxf.apache.org>
<soap:Envelope ...>
--uuid:...
Content-Type: text/plain
...
What the helper does. encodeEarlierVersion(multipartType) repeats those old steps directly with AttachmentSerializer:
1. It sets CONTENT_TYPE to the multipart type given.
2. It serializes the envelope and one attachment with setXop(false).
3. It returns the RMMessage.
Why the tests need it. Suppose an application is upgraded while unacknowledged messages are still in its RM store. The new version recovers messages that the old version wrote. When the new decoder reads that root part, it sees a multipart type and has to work out the real SOAP type. That's Colm's item #1. Before the review fix this fell back to text/xml, so a SOAP 1.2 message was resent with the wrong type. Now the decoder takes the type from start-info/type. The three tests that use the helper check the SOAP 1.1, SOAP 1.2 and SOAP 1.2 + MTOM cases.
The Javadoc is admittedly terse. A clearer wording could be:
/**
* Encodes a message with an attachment the way CXF did before CXF-9258: the root part gets the
* multipart Content-Type of the message, which AttachmentOutInterceptor had already set, so the
* stored message has a nested multipart type as its root part type.
*/
Should I amend dc1f17a374 with that wording?
✻ Cogitated for 14s · done 10:41 AM
Confirmed, good catch. Your input produced `"application/soap+xml` with a stray quote. The regex also removed `charset` from *inside* the quoted `type` value. It no longer uses regular expressions: the Content-Type parameters are now parsed with `jakarta.activation.MimeType`. So only the top-level `charset` is removed, and escaped quotes in quoted values (like the SOAP 1.2 `action`) are handled by the parser. I added your example as a test (`testDecodeInboundXopRootWithCharsetInType`), plus one with an escaped action.
### 1. The `text/xml` fallback ignores the SOAP version
Agreed. It's worse than losing the action: a SOAP 1.2 envelope resent as `text/xml` gets a fault from the server and is retried until it gives up. When only a multipart type is available, which is what earlier versions stored, the root type now comes from that type:
- `start-info`, which keeps the parameters such as the action;
- otherwise `type`;
- for `type="application/xop+xml"`, an XOP root type built from `start-info`, so a recovered MTOM message is resent as an XOP package again.
`text/xml` is only the last resort. New tests decode messages stored the way earlier versions did, for SOAP 1.1, SOAP 1.2 and SOAP 1.2 with MTOM.
### 2. Single-use attachment streams on resend
Agreed this is real, but it isn't new in this PR. Before it, retransmissions didn't include the attachments at all. It's also worse than a resend problem: with a persistent store, `encodeRMContent` reads the attachments at `WRITE_ENDING`, before `AttachmentOutEndingInterceptor` writes them. So a one-shot stream is already empty on the *first* transmission, and that's true on 4.2.4 too.
The main real-world case is WS-Security, whose secured attachments can only be read once. I'm handling that in [CXF-6955](https://issues.apache.org/jira/browse/CXF-6955), which builds on this PR, so I'd rather keep it out of this one. Making arbitrary user `DataSource`s retransmittable would mean caching every attachment at capture time; that could be a separate follow-up if we think it's worth the cost.
### 3. The outbound check depends on `CONTENT_TYPE`
For SOAP this can't happen today: `SoapPreProtocolOutInterceptor` sets the Content-Type at `POST_LOGICAL`, before `RMCaptureOutInterceptor` runs at `PRE_STREAM`. But `MessageUtils.isOutbound(msg)` states the intent much more clearly, so XOP storage now uses it. When the root type has to come from the multipart type, it can already be an XOP type, so it's unwrapped before the serializer adds XOP again. Tests cover both cases.
### 4. Regex parsing of Content-Type parameters
Fixed with the same `MimeType` change. Only the first `Content-Type` value of the root part is used instead of joining all of them, and an invalid value falls back to its type/subtype.
### 5. Duplicate persistence path in `DestinationSequence`
Agreed. A new `PersistenceUtils.encodeRMContent(RMMessage, Message, CachedOutputStream)` overload stores the content as is when there are no attachments, so no extra copy, but with the normalised root type. `DestinationSequence` now calls it with no branch of its own.
### Minor: redundant `removeContent(AttachmentSerializer.class)`
Removed, thanks.
--
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-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CXF-9258: Fix WS-RM retransmission of messages with attachments [cxf]
Posted by "ffang (via GitHub)" <gi...@apache.org>.
ffang merged PR #3598:
URL: https://github.com/apache/cxf/pull/3598
--
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-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org