Skip to content

Commit daa6737

Browse files
cppudgeclaude
andcommitted
fix: low-level correctness sweep (audit group C)
Digest references work end to end: split_image splits name@sha256:hex at the '@' (digest in the tag slot; trailing ':' or '@' normalizes to latest), and the new join_image re-attaches digests with '@' at every re-join site (create-spec reference, exists/inspect, pull ?tag= already accepted digests). The near-duplicate split_image_ref is deleted; its one caller (the sshd override) now takes digest spellings too. New detail::saturated_add clamps every user-sized budget at time_point::max() instead of overflowing the clock rep (UB whose typical wrap failed healthy starts instantly - and only on the stdlib whose rep is fine enough, the MSVC-100ns-vs-libstdc++-1ns trap): wait_until_ready, clamped_wait_plan, the poll-loop sleep legs, the three transport connect deadlines, compose's probe deadline, and the Kafka/Mongo hook phase budgets. url_encode drops locale-sensitive isalnum for an explicit ASCII test; create_container's lazy pull is gated on a case-insensitive "no such image" 404 body (a missing network also 404s - no more spurious re-pull with the error pinned on the wrong resource); the tar write path tolerates ARCHIVE_WARN like the read paths; sha256 gains the 55/56/64-byte padding-boundary vectors (NIST two-block included). Pipeline: MSVC 563 units, Linux core integration 135 pass/44 skip, module suite 34/34, WSL gcc 557 units, clang-tidy (17 TUs) and clang-format clean. Opus review: SHIP; both recommendations applied (case-insensitive pull gate, dead <utility> include). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0141x8KrNdD9NfmCh1vSMbpg
1 parent b0c1c30 commit daa6737

23 files changed

Lines changed: 229 additions & 66 deletions

docs/TODO.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -139,8 +139,8 @@ loopback/named-pipe servers live in tests/unit/{LoopbackServer,PipeServer}.hpp.
139139
utility image (2026-07-11); still out of scope by design: `GenericBuildableImage` output
140140
(the daemon resolves `FROM`), compose YAML services (the file is the user's), and raw
141141
`DockerClient` calls. The custom `with_image_name_substitutor` remains `GenericImage`-only.
142-
Utility-image overrides take `name[:tag]` references — digest (`@sha256:...`) spellings
143-
don't fit the sshd sidecar's name:tag builder. The age-based pull policy (2026-07-11)
142+
Utility-image overrides take `name[:tag]` or `name@sha256:...` references (digest
143+
spellings work since the 2026-07-12 split/join unification: a digest re-joins with '@'). The age-based pull policy (2026-07-11)
144144
applies to `GenericImage` runs only (utility images and compose stay Default/explicit).
145145
- **Host access residuals** — remote forwards are never cancelled once added; the
146146
tunnel pump wakes every 100ms even when idle (fine for test traffic).

docs/feature-notes.md

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -299,8 +299,12 @@ says nothing about freshness, so derive the tag from a hash of the build inputs
299299
change. `GenericImage::inspect(name, tag)` / `img.inspect()` (backed by
300300
`DockerClient::inspect_image[_raw]`, which also takes full references/digests) return a typed
301301
`ImageInspect` — id, repo tags/digests, created, os/arch, size, and the image config (labels,
302-
env, cmd, entrypoint, exposed ports, workdir, user); the instance form uses `image():tag()`
303-
verbatim, no substitutor. Built images are session-scoped: `build()` ships the managed-by /
302+
env, cmd, entrypoint, exposed ports, workdir, user); the instance form re-joins
303+
`image()`/`tag()` digest-aware, no substitutor. Digest-pinned references
304+
(`name@sha256:...`) work end to end (2026-07-12): `from_reference`/`with_image` split them
305+
at the '@' (the digest becomes the tag slot), and every re-join — the create-spec
306+
reference, the pull query (`?tag=` accepts a digest), the utility-image overrides — goes
307+
through the shared digest-aware `join_image` ('@', never ':'). Built images are session-scoped: `build()` ships the managed-by /
304308
session labels via `?labels=` (merged with the Dockerfile's own LABELs; on a duplicate key the
305309
query label wins, `docker build --label` parity) and boots the reaper, so Ryuk removes the
306310
image shortly after the process exits — base images and pulled layers are untouched. With the

src/Deadline.hpp

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
#pragma once
2+
3+
#include <chrono>
4+
5+
namespace testcontainers::detail {
6+
7+
/// `at + delta` with saturation at `time_point::max()`. User-sized budgets
8+
/// ("wait forever" spelled as milliseconds::max(), hours(300000), ...) must
9+
/// clamp to the far future: the plain `+` is signed-integer overflow (UB),
10+
/// and the typical wrap lands the deadline in the PAST, timing a healthy
11+
/// wait out instantly. The overflow window is stdlib-dependent — MSVC's
12+
/// steady_clock rep ticks at 100ns while libstdc++'s at 1ns — so the same
13+
/// budget can be green on one toolchain and wrapped on the other; the
14+
/// comparison therefore happens in milliseconds, BEFORE any conversion to
15+
/// the clock's finer rep. A non-positive delta yields `at` unchanged (an
16+
/// already-due deadline).
17+
inline std::chrono::steady_clock::time_point saturated_add(std::chrono::steady_clock::time_point at,
18+
std::chrono::milliseconds delta) {
19+
using clock = std::chrono::steady_clock;
20+
if (delta <= std::chrono::milliseconds::zero()) {
21+
return at;
22+
}
23+
// Truncation makes headroom conservative (never larger than the real
24+
// room), so `delta < headroom` guarantees the converted add fits.
25+
const auto headroom =
26+
std::chrono::duration_cast<std::chrono::milliseconds>(clock::time_point::max() - at);
27+
return delta >= headroom ? clock::time_point::max() : at + delta;
28+
}
29+
30+
} // namespace testcontainers::detail

src/DockerComposeContainer.cpp

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
#include "testcontainers/DockerComposeContainer.hpp"
22

33
#include "Config.hpp"
4+
#include "Deadline.hpp"
45
#include "HostAddress.hpp"
56
#include "RandomHex.hpp"
67
#include "Reaper.hpp"
@@ -604,8 +605,10 @@ void DockerComposeContainer::start() {
604605
: get_service_port(exposed.service, exposed.instance, exposed.port);
605606
const std::string host = get_service_host(exposed.service);
606607

607-
// The same user-configurable timeout that governs compose's --wait.
608-
const auto deadline = std::chrono::steady_clock::now() + wait_timeout_;
608+
// The same user-configurable timeout that governs compose's --wait
609+
// (saturated: a "wait forever"-sized value clamps, never wraps).
610+
const auto deadline =
611+
detail::saturated_add(std::chrono::steady_clock::now(), wait_timeout_);
609612
bool connected = false;
610613
while (std::chrono::steady_clock::now() < deadline) {
611614
if (detail::tcp_probe(host, host_port, detail::probe_budget(deadline))) {

src/GenericImage.cpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,11 +26,11 @@ GenericImage& GenericImage::with_image(const std::string& reference) {
2626

2727
bool GenericImage::exists(const std::string& name, const std::string& tag) {
2828
DockerClient client = DockerClient::from_environment();
29-
return client.image_exists(name + ":" + tag);
29+
return client.image_exists(docker::join_image(name, tag));
3030
}
3131

3232
ImageInspect GenericImage::inspect(const std::string& name, const std::string& tag) {
33-
return DockerClient::from_environment().inspect_image(name + ":" + tag);
33+
return DockerClient::from_environment().inspect_image(docker::join_image(name, tag));
3434
}
3535

3636
ImageInspect GenericImage::inspect() const { return inspect(image_, tag_); }
@@ -48,7 +48,8 @@ ContainerRequest GenericImage::to_request() const {
4848

4949
// Resolve the effective image reference: a custom substitutor overrides the
5050
// default env-prefix substitution (TESTCONTAINERS_HUB_IMAGE_NAME_PREFIX).
51-
const std::string raw_ref = image_ + ":" + tag_;
51+
// join_image, not '+ ":" +': a digest tag re-attaches with '@'.
52+
const std::string raw_ref = docker::join_image(image_, tag_);
5253
request.spec.image =
5354
substitutor_ ? substitutor_(raw_ref) : docker::substitute_image_name(raw_ref);
5455

src/HostPortForwarding.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@
2424
#include "Config.hpp"
2525
#include "RandomHex.hpp"
2626
#include "Runner.hpp"
27-
#include "docker/Auth.hpp" // split_image_ref (sshd.container.image override)
27+
#include "docker/ApiMapping.hpp" // split_image (sshd.container.image override)
2828
#include "testcontainers/Container.hpp"
2929
#include "testcontainers/ContainerRequest.hpp"
3030
#include "testcontainers/Error.hpp"
@@ -42,8 +42,8 @@ using asio::ip::tcp;
4242
/// openssh; its own entrypoint+command set ${USERNAME} (root) to ${PASSWORD}
4343
/// and run sshd in the foreground with GatewayPorts=yes. Overridable via env
4444
/// TESTCONTAINERS_SSHD_CONTAINER_IMAGE / properties key sshd.container.image
45-
/// (a name[:tag] reference); the hub prefix applies through GenericImage's
46-
/// default substitutor either way.
45+
/// (a name[:tag] or name@sha256:... reference); the hub prefix applies
46+
/// through GenericImage's default substitutor either way.
4747
constexpr const char* kSshdImage = "testcontainers/sshd";
4848
constexpr const char* kSshdTag = "1.3.0";
4949

@@ -53,7 +53,7 @@ std::pair<std::string, std::string> sshd_image_and_tag() {
5353
if (!override_ref) {
5454
return {kSshdImage, kSshdTag};
5555
}
56-
return docker::split_image_ref(*override_ref);
56+
return docker::split_image(*override_ref);
5757
}
5858

5959
/// Per-direction buffer cap per forwarded connection. When a buffer is full

src/WaitStrategies.cpp

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
#include <boost/beast/core/tcp_stream.hpp>
1717
#include <boost/beast/http.hpp>
1818

19+
#include "Deadline.hpp"
1920
#include "HostAddress.hpp"
2021
#include "docker/Ports.hpp"
2122
#include "testcontainers/ContainerPort.hpp"
@@ -165,8 +166,9 @@ void wait_for_log(DockerClient& client, const std::string& id, const wait_for::L
165166
// FollowEnd::StreamEnded: the container stopped. Its logs are final
166167
// unless it restarts (restart policy), so pause briefly and re-follow;
167168
// an expired budget lands in the final snapshot check above.
168-
std::this_thread::sleep_until(std::min(Clock::now() + std::chrono::milliseconds(200),
169-
deadline + std::chrono::milliseconds(1)));
169+
std::this_thread::sleep_until(
170+
std::min(Clock::now() + std::chrono::milliseconds(200),
171+
detail::saturated_add(deadline, std::chrono::milliseconds(1))));
170172
}
171173
}
172174

@@ -486,8 +488,11 @@ void wait_for_command(DockerClient& client, const std::string& id, const wait_fo
486488
if (Clock::now() >= deadline) {
487489
throw timeout_error();
488490
}
491+
// Both legs saturate: `interval` is a user knob (poll_interval) and
492+
// `deadline` may already sit at the clamped far future.
489493
std::this_thread::sleep_until(
490-
std::min(Clock::now() + interval, deadline + std::chrono::milliseconds(1)));
494+
std::min(detail::saturated_add(Clock::now(), interval),
495+
detail::saturated_add(deadline, std::chrono::milliseconds(1))));
491496
}
492497
}
493498

@@ -508,7 +513,11 @@ void wait_until_ready(DockerClient& client, const std::string& id,
508513
// and docs/TODO.md for the analysis.
509514
const DockerClient::Session session(client);
510515

511-
const Clock::time_point deadline = Clock::now() + timeout;
516+
// saturated_add, not '+': the startup timeout is a raw user knob, and a
517+
// "wait forever"-sized value must clamp instead of wrapping into the past
518+
// (which failed healthy starts instantly — and only on the stdlib whose
519+
// clock rep is fine enough to overflow).
520+
const Clock::time_point deadline = detail::saturated_add(Clock::now(), timeout);
512521

513522
for (const WaitFor& w : waits) {
514523
std::visit(

src/WaitStrategies.hpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
#include <utility>
99
#include <vector>
1010

11+
#include "Deadline.hpp"
1112
#include "testcontainers/WaitFor.hpp"
1213

1314
namespace testcontainers {
@@ -58,7 +59,9 @@ struct ClampedWaitPlan {
5859
inline ClampedWaitPlan clamped_wait_plan(std::chrono::steady_clock::time_point now,
5960
std::chrono::milliseconds value,
6061
std::chrono::steady_clock::time_point deadline) {
61-
const std::chrono::steady_clock::time_point wake = now + value;
62+
// saturated_add, not '+': a milliseconds::max()-sized duration must clamp
63+
// to the far future (and so time out), not overflow into the past.
64+
const std::chrono::steady_clock::time_point wake = detail::saturated_add(now, value);
6265
return {wake < deadline ? wake : deadline, wake > deadline};
6366
}
6467

src/docker/ApiMapping.cpp

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -927,13 +927,33 @@ void throw_if_pull_error(const std::string& pull_stream, const std::string& imag
927927

928928
std::pair<std::string, std::string> split_image(const std::string& image) {
929929
const std::size_t slash = image.rfind('/');
930+
const std::size_t at = image.rfind('@');
931+
if (at != std::string::npos && (slash == std::string::npos || at > slash)) {
932+
// Digest reference. The daemon accepts the digest wherever a tag
933+
// goes (pull's ?tag= parameter takes "Tag or digest"); only the
934+
// re-join differs — join_image uses '@'. A bare trailing '@' is
935+
// normalized like a bare trailing ':'.
936+
if (at + 1 == image.size()) {
937+
return {image.substr(0, at), "latest"};
938+
}
939+
return {image.substr(0, at), image.substr(at + 1)};
940+
}
930941
const std::size_t colon = image.rfind(':');
931942
if (colon != std::string::npos && (slash == std::string::npos || colon > slash)) {
943+
if (colon + 1 == image.size()) {
944+
return {image.substr(0, colon), "latest"}; // "redis:" means "redis"
945+
}
932946
return {image.substr(0, colon), image.substr(colon + 1)};
933947
}
934948
return {image, "latest"};
935949
}
936950

951+
std::string join_image(const std::string& name, const std::string& tag) {
952+
// A legal tag ([A-Za-z0-9_][A-Za-z0-9._-]{0,127}) cannot contain ':'; a
953+
// digest ("sha256:<hex>") always does.
954+
return tag.find(':') != std::string::npos ? name + "@" + tag : name + ":" + tag;
955+
}
956+
937957
std::string build_build_query(const BuildOptions& options,
938958
const std::function<std::string(const std::string&)>& encode) {
939959
std::string query;

src/docker/ApiMapping.hpp

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -142,10 +142,19 @@ std::string expect_string_field(const std::string& body, const char* field,
142142
/// throw DockerError if it reports an error (Docker returns HTTP 200 even then).
143143
void throw_if_pull_error(const std::string& pull_stream, const std::string& image);
144144

145-
/// Split "name[:tag]" into {name, tag}; tag defaults to "latest". Handles a
146-
/// registry host with a port (e.g. "my-reg:5000/img" -> {"my-reg:5000/img", "latest"}).
145+
/// Split "name[:tag]" or "name@sha256:<hex>" into {name, tag-or-digest}: the
146+
/// tag separator is the last ':' after the last '/' (a registry host:port is
147+
/// not a tag), an '@' after the last '/' starts a digest (returned verbatim,
148+
/// "sha256:..."), and no tag — a bare trailing ':' included — means "latest".
149+
/// `join_image` is the inverse; every re-join must go through it, since a
150+
/// digest re-attaches with '@', not ':'.
147151
std::pair<std::string, std::string> split_image(const std::string& image);
148152

153+
/// Re-join a split_image pair into a reference the daemon accepts: '@' when
154+
/// the tag is a digest, ':' otherwise. (A legal tag cannot contain ':'; a
155+
/// digest always does.)
156+
std::string join_image(const std::string& name, const std::string& tag);
157+
149158
/// Build the query string (incl. leading '?') for `POST /build`: t, dockerfile,
150159
/// nocache, pull, target (when set), and buildargs / labels (JSON objects,
151160
/// percent-encoded). `encode` is the caller's URL-encoder. Unit-testable

0 commit comments

Comments
 (0)