gnodet-bot commented on code in PR #2206:
URL: https://github.com/apache/maven-resolver/pull/2206#discussion_r4252187719
##########
maven-resolver-transport-jetty/src/main/java/org/eclipse/aether/transport/jetty/JettyTransporter.java:
##########
@@ -410,6 +413,30 @@ protected void implPut(PutTask task) throws Exception {
throw new HttpTransporterException(statusCode);
});
}
+ // On some platforms (macOS) with certain configurations (proxy
auth, TLS/HTTP2),
+ // Jetty may complete a small PUT without firing onRequestContent
callbacks, leaving
+ // the progress listener uninformed even though the upload
succeeded.
+ // Fire the missing events now that we know the request completed
successfully.
+ if (task.getDataLength() > 0 && !progressed.get()) {
Review Comment:
💡 **`transportPropertiesAvailable` ordering inversion in the retroactive
path.**
The `TransportListener` Javadoc specifies the ordering contract for PUT
tasks:
> *for remote put tasks this is called after `transportStarted(long, long)`*
In the normal path (Jetty fires `onRequestContent`) this holds:
`transportStarted` fires during the request send, then
`transportPropertiesAvailable` fires after the response is received (line 410).
But in the retroactive path, `transportPropertiesAvailable` fires at line 410
(pre-existing), then `transportStarted` fires at line 423 — **backwards**
relative to the contract.
In the current `TransferTransportListener` implementation,
`transportPropertiesAvailable` just stores the map and only applies it in
`transferSucceeded`/`transferFailed`, so there is no functional breakage today.
But this silently violates a documented SPI contract that future
implementations could rely on.
Fix: move the `transportPropertiesAvailable` call to after the retroactive
block (line 439), so the order matches the contract in both paths:
```suggestion
if (task.getDataLength() > 0 && !progressed.get()) {
```
Actually, the suggestion block can't reorder pre-existing code — the fix
would be to move `task.getListener().transportPropertiesAvailable(...)` from
line 410 to after the `if (!progressed.get())` block. Low severity, but worth
fixing to keep the SPI contract clean.
--
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]
