gnodet-bot commented on code in PR #29055:
URL: https://github.com/apache/camel/pull/29055#discussion_r4458223221
##########
components/camel-as2/camel-as2-component/src/main/java/org/apache/camel/component/as2/AS2Configuration.java:
##########
@@ -665,17 +667,38 @@ public String getAsyncMdnAllowedHosts() {
}
/**
- * Comma separated list of host names an asynchronous MDN may be delivered
to. The delivery address is chosen by the
- * sender of the AS2 message, so this is the list of hosts the operator
vouches for.
+ * Comma separated list of hosts an asynchronous MDN may be delivered to.
The delivery address is chosen by the
+ * sender of the AS2 message, so this is the list of destinations the
operator vouches for. Each entry is a host
+ * with an optional port in host:port form; an IPv6 literal is written in
square brackets, as in a URL, optionally
+ * followed by :port. A bare-host entry (for example partner.example)
matches only the default port for the scheme
+ * (80 or 443), so a non-default port is listed explicitly (for example
partner.example:8443).
Review Comment:
**Doc inaccuracy — `asyncMdnAllowedHosts`: "80 or 443" implies scheme-aware
default-port resolution that doesn't exist.**
`AS2AsynchronousMDNManager.send()` only delivers over a plain `Socket`; the
existing guard
```java
if (!"http".equals(scheme)) {
throw new HttpException("... the delivery address must use http. TLS
delivery of asynchronous MDNs is not supported");
}
```
refuses every non-http URL — including https — before `isAllowedAuthority`
is ever called. The `defaultPort` is therefore always 80; the "443 for https"
branch is dead. An operator who writes a bare-host entry expecting it to match
port 443 for an https delivery address will be surprised to find both the
allow-list check and delivery itself fail.
```suggestion
* followed by :port. A bare-host entry (for example partner.example)
matches only the default port (80), so a
* non-default port is listed explicitly (for example
partner.example:8443).
```
##########
components/camel-as2/camel-as2-component/src/main/java/org/apache/camel/component/as2/AS2Configuration.java:
##########
@@ -665,17 +667,38 @@ public String getAsyncMdnAllowedHosts() {
}
/**
- * Comma separated list of host names an asynchronous MDN may be delivered
to. The delivery address is chosen by the
- * sender of the AS2 message, so this is the list of hosts the operator
vouches for.
+ * Comma separated list of hosts an asynchronous MDN may be delivered to.
The delivery address is chosen by the
+ * sender of the AS2 message, so this is the list of destinations the
operator vouches for. Each entry is a host
+ * with an optional port in host:port form; an IPv6 literal is written in
square brackets, as in a URL, optionally
+ * followed by :port. A bare-host entry (for example partner.example)
matches only the default port for the scheme
+ * (80 or 443), so a non-default port is listed explicitly (for example
partner.example:8443).
*
- * When set, an asynchronous MDN whose delivery address names a host
outside the list is refused, and the configured
- * mdnUserName/mdnPassword/mdnAccessToken are attached only to a host on
the list. When not set, the MDN is still
- * delivered to the sender-supplied address but no credentials are
attached.
+ * When set, an asynchronous MDN whose delivery address is not on the list
is refused, and the configured
+ * mdnUserName/mdnPassword/mdnAccessToken are attached only to an address
on the list. When not set, the MDN is
+ * still delivered to the sender-supplied address but no credentials are
attached.
*
* The option applies to the consumer endpoint it is set on: consumers
sharing a serverPortNumber each apply their
* own list.
*/
public void setAsyncMdnAllowedHosts(String asyncMdnAllowedHosts) {
this.asyncMdnAllowedHosts = asyncMdnAllowedHosts;
}
+
+ public boolean isAsyncMdnRequireHttps() {
+ return asyncMdnRequireHttps;
+ }
+
+ /**
+ * Whether an asynchronous MDN may only be delivered over https. The
delivery address, including its scheme, is
+ * chosen by the sender of the AS2 message; by default both http and https
are accepted. When set to true, an
Review Comment:
**Doc inaccuracy — `asyncMdnRequireHttps`: "by default both http and https
are accepted" is wrong.**
Only http is accepted. Https is always refused (plain-socket
implementation). With `asyncMdnRequireHttps=true`, http is refused by the new
check and https is refused by the existing plain-socket guard — so no delivery
is possible. The Javadoc should make this clear so an operator setting this
flag understands the current limitation. Since this Javadoc drives the
generated DSL docs and catalog JSON, fixing it here propagates everywhere.
```suggestion
* Whether an asynchronous MDN may only be delivered over https. The
delivery address, including its scheme, is
* chosen by the sender of the AS2 message; by default only http is
accepted (TLS delivery of asynchronous MDNs is not yet implemented). When set
to true, an
```
##########
components/camel-as2/camel-as2-component/src/main/docs/as2-component.adoc:
##########
@@ -56,4 +56,17 @@ The settings of the shared server connection still come from
the first endpoint
`sslContextParameters` and `sslContext` (used for the TLS listener),
`mdnMessageTemplate`, `server`, `serverFqdn` and `as2Version`. They are per
port, not per consumer.
+=== Delivering asynchronous MDNs
+
+When a received message asks for an asynchronous MDN, the
`as2://server/listen` consumer delivers the MDN to the
+address that the sender gave in the `Receipt-Delivery-Option` header. When
`asyncMdnAllowedHosts` is set, the MDN is
+delivered only to an address on the list, and the `mdnUserName`, `mdnPassword`
and `mdnAccessToken` credentials are
+attached only for such an address. Each entry of the list is a host with an
optional port:
+
+* a bare host, such as `partner.example`, matches only the default port for
the scheme (80 for `http`, 443 for
+`https`);
Review Comment:
Same "443 for https" inaccuracy in the user-facing docs. Since https
delivery is not supported, the default port is always 80. There is no https
code path that would use port 443 as a default.
```suggestion
* a bare host, such as `partner.example`, matches only the default port
(80), so a non-default port is listed explicitly (for example
`partner.example:8443`);
```
--
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]
