gnodet-bot commented on code in PR #2206:
URL: https://github.com/apache/maven-resolver/pull/2206#discussion_r4252325172
##########
maven-resolver-transport-jetty/src/main/java/org/eclipse/aether/transport/jetty/JettyTransporter.java:
##########
@@ -404,12 +407,40 @@ protected void implPut(PutTask task) throws Exception {
})
.send(listener);
response = listener.get(requestTimeout, TimeUnit.MILLISECONDS);
-
task.getListener().transportPropertiesAvailable(createTransportProperties(request,
rawResponseHeaders));
if (response.getStatus() >= MULTIPLE_CHOICES) {
JettyRFC9457Reporter.INSTANCE.generateException(listener,
(statusCode, reasonPhrase) -> {
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)) {
+ try {
+ task.getListener().transportStarted(0,
task.getDataLength());
+ } catch (TransferCancelledException e) {
+ return; // upload already succeeded — retroactive
notification cannot be cancelled
+ }
+ }
+ try (InputStream is = task.newInputStream()) {
+ byte[] buf = new byte[1024 * 32];
+ int n;
+ 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
+ }
+ }
+ }
+ }
+ // transportPropertiesAvailable is called after
transportStarted/transportProgressed
+ // to honour the TransportListener contract: "for remote put tasks
this is called
+ // after transportStarted(long, long)". Moving it here ensures
correct ordering in
+ // both the normal path and the retroactive-progress path.
+
task.getListener().transportPropertiesAvailable(createTransportProperties(request,
rawResponseHeaders));
Review Comment:
⚠️ **Regression:** `transportPropertiesAvailable` is now unreachable on
error responses (status ≥ 300).
The `generateException` call above throws `HttpTransporterException` for
3xx/4xx/5xx responses, which exits the `try` block before reaching this line.
Failed PUT transfers will no longer have transport properties attached to the
`TransferEvent` — a regression from both the original behavior and other
transporter implementations.
This call should remain **before** the `if (response.getStatus() >=
MULTIPLE_CHOICES)` block, where it was originally. The `TransportListener`
ordering constraint ("after `transportStarted`") is still satisfied there,
since the retroactive progress block already fired at that point.
--
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]
