gnodet-bot commented on code in PR #2206:
URL: https://github.com/apache/maven-resolver/pull/2206#discussion_r4252085736
##########
maven-resolver-transport-jetty/src/main/java/org/eclipse/aether/transport/jetty/JettyTransporter.java:
##########
@@ -410,6 +413,22 @@ 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()) {
+ if (started.compareAndSet(false, true)) {
+ task.getListener().transportStarted(0,
task.getDataLength());
+ }
+ try (InputStream is = task.newInputStream()) {
+ byte[] buf = new byte[1024 * 32];
+ int n;
+ while ((n = is.read(buf)) != -1) {
+
task.getListener().transportProgressed(ByteBuffer.wrap(buf, 0, n));
+ }
Review Comment:
⚠️ **Uncaught `TransferCancelledException` in retroactive path — upload
already succeeded.**
`transportStarted()` (line 422) and `transportProgressed()` (line 428) both
declare `throws TransferCancelledException`. Neither call is guarded by a
try-catch here. The retroactive block only runs **after** the server returned
2xx, so if the listener throws TCE the upload succeeded on the server but the
caller sees an exception from `implPut()` — it may retry a PUT that already
landed.
The normal path handles TCE by aborting the in-flight Jetty request
(`r.abort(e)`), which is not available here. The fix is to catch and discard
TCE in the retroactive path:
```suggestion
while ((n = is.read(buf)) != -1) {
try {
task.getListener().transportProgressed(ByteBuffer.wrap(buf, 0, n));
} catch (TransferCancelledException e) {
break; // upload already succeeded — retroactive
notification cannot be cancelled
}
}
```
The `transportStarted()` call on line 422 needs the same treatment:
```java
if (started.compareAndSet(false, true)) {
try {
task.getListener().transportStarted(0,
task.getDataLength());
} catch (TransferCancelledException e) {
return; // upload already succeeded
}
}
```
--
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]