Skip to content

Timed-out blob upload PATCH is replayed into the same upload session #4526

Description

@jbruinink

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.FailoverHttpClientPatchRetryTest

The test fails because the server receives two complete requests:

FailoverHttpClientPatchRetryTest > timeoutAfterBodyWasConsumed_doesNotReplayPatch FAILED
    expected: 1
    but was : 2

Cause

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 PATCH in the generic HTTP I/O retry handler.
  • Send explicit Content-Length and Content-Range bounds and handle an out-of-order 416.
  • 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.

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions