Timed-out blob upload PATCH is replayed into the same upload session
Description
When a registry consumes a blob upload PATCH but takes longer than Jib's read timeout to return
202 Accepted, Jib automatically sends the same PATCH body again to the same upload URL.
The body is replayable, but the upload operation is not necessarily idempotent. A streaming
registry can append the complete body again. The closing PUT then fails with DIGEST_INVALID
because the upload contains blob || blob.
This appears related to #3994, which also reports DIGEST_INVALID after a timed-out registry
request was retried.
Reproduction
The following self-contained unit test reproduces the behavior on current Jib master
(fb949e2676afbbd7dd7a1ef61e20251931325654). It uses only a local JDK HttpServer and
synthetic data.
Add
jib-core/src/test/java/com/google/cloud/tools/jib/http/FailoverHttpClientPatchRetryTest.java:
package com.google.cloud.tools.jib.http;
import static com.google.common.truth.Truth.assertThat;
import com.google.cloud.tools.jib.blob.Blobs;
import com.google.common.io.ByteStreams;
import com.sun.net.httpserver.HttpServer;
import java.io.IOException;
import java.net.InetSocketAddress;
import java.net.URL;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.atomic.AtomicInteger;
import org.junit.Test;
public class FailoverHttpClientPatchRetryTest {
@Test
public void timeoutAfterBodyWasConsumed_doesNotReplayPatch() throws IOException {
byte[] body = "crepecake".getBytes(StandardCharsets.UTF_8);
List<byte[]> receivedBodies = Collections.synchronizedList(new ArrayList<>());
AtomicInteger requests = new AtomicInteger();
HttpServer server = HttpServer.create(new InetSocketAddress(0), 0);
ExecutorService executor = Executors.newCachedThreadPool();
server.setExecutor(executor);
server
.createContext("/upload")
.setHandler(
exchange -> {
receivedBodies.add(ByteStreams.toByteArray(exchange.getRequestBody()));
if (requests.getAndIncrement() == 0) {
try {
Thread.sleep(500); // Longer than the client read timeout.
} catch (InterruptedException ex) {
Thread.currentThread().interrupt();
}
}
exchange.sendResponseHeaders(202, -1);
exchange.close();
});
try {
server.start();
Request patch =
Request.builder()
.setBody(new BlobHttpContent(Blobs.from("crepecake"), "application/octet-stream"))
.setHttpTimeout(100)
.build();
IOException failure = null;
try (Response ignored =
new FailoverHttpClient(true, false, event -> {})
.call(
"PATCH",
new URL("http://localhost:" + server.getAddress().getPort() + "/upload"),
patch)) {
// A timed-out PATCH has an unknown outcome and must not be replayed blindly.
} catch (IOException ex) {
failure = ex;
}
assertThat(requests.get()).isEqualTo(1);
assertThat(receivedBodies).hasSize(1);
assertThat(receivedBodies.get(0)).isEqualTo(body);
assertThat(failure).isNotNull();
} finally {
server.stop(0);
executor.shutdownNow();
}
}
}Run:
./gradlew :jib-core:test \
--tests com.google.cloud.tools.jib.http.FailoverHttpClientPatchRetryTestThe test fails because the server receives two complete requests:
FailoverHttpClientPatchRetryTest > timeoutAfterBodyWasConsumed_doesNotReplayPatch FAILED
expected: 1
but was : 2Cause
FailoverHttpClient installs a generic HttpBackOffIOExceptionHandler.
BlobHttpContent.retrySupported() only checks whether the body can be read again, without taking
the HTTP method or registry upload semantics into account. BlobPusher.Writer sends the body with
PATCH and reuses the same upload URL.
Possible solution directions
- Do not automatically retry
PATCHin the generic HTTP I/O retry handler. - Send explicit
Content-LengthandContent-Rangebounds and handle an out-of-order416. - After an ambiguous timeout, query the current upload offset or start a fresh upload session.
I would be happy to prepare a PR once the preferred recovery behavior is agreed.
Source: GoogleContainerTools/jib