Runtime hardening: contain handler exceptions and range-check timestamp decoding

docs/server-guide.md

@@ -34,6 +34,12 @@ class MyHandler final : public example::weather::WeatherHandler {
 - **Validation/serialization errors** (including malformed request input the framework catches
   before your handler runs) map to 400; any other failure is a non-leaking 500
   `InternalFailure`.
+- **A handler that throws** (rather than returning an `Error`) is contained by the transport
+  layer: the exception is converted into a 500 carrying an `x-correlation-id` header, and the
+  same id plus the exception's `what()` is written to `std::clog`. One throwing request fails
+  alone — it never unwinds into the transport's I/O thread and terminates the process. Prefer
+  returning `smithy::Error` for expected failures; the catch-all is a safety net, not a control
+  path.
 
 ## Running a server
 

examples/weather/generated_server_e2e_test.cc

@@ -7,6 +7,7 @@
 #include <chrono>
 #include <cstdint>
 #include <memory>
+#include <stdexcept>
 #include <string>
 #include <utility>
 #include <vector>
@@ -104,6 +105,42 @@ TEST_F(GeneratedServerEndToEndTest, GetCityRoundTrips) {
   EXPECT_FLOAT_EQ(city->coordinates.longitude, -122.3321F);
 }
 
+// A handler that throws (rather than returning a modeled Error) must be
+// contained as a 500 through the full generated dispatch stack — routing,
+// validation, then the handler — not crash the server. This drives the
+// generated WeatherServer over loopback and reads the raw response so it can
+// assert the status and correlation-id header the client abstraction hides.
+class ThrowingHandler : public ReferenceHandler {
+ public:
+  smithy::Outcome<GetCityOutput> GetCity(const GetCityInput& input) override {
+    (void)input;
+    throw std::runtime_error("handler blew up mid-request");
+  }
+};
+
+TEST(GeneratedServerFaultTest, ThrowingHandlerBecomesA500ThroughGeneratedDispatch) {
+  WeatherServer server(std::make_shared<ThrowingHandler>());
+  smithy::http::Loopback loopback;
+  ASSERT_TRUE(loopback.Start(server.Handler()).ok());
+
+  smithy::http::HttpRequest request;
+  request.method = "GET";
+  request.target = "/cities/seattle";  // valid route → reaches GetCity
+  const auto response = loopback.Send(request);
+
+  ASSERT_TRUE(response.ok()) << response.error().message();
+  EXPECT_EQ(response->status, 500);
+  EXPECT_FALSE(response->headers.Get("x-correlation-id").value_or("").empty());
+
+  // The server instance is still usable afterward: a well-behaved route works.
+  WeatherServer healthy(std::make_shared<ReferenceHandler>());
+  smithy::http::Loopback ok_loopback;
+  ASSERT_TRUE(ok_loopback.Start(healthy.Handler()).ok());
+  const auto ok = ok_loopback.Send(request);
+  ASSERT_TRUE(ok.ok()) << ok.error().message();
+  EXPECT_EQ(ok->status, 200);
+}
+
 // A transport that fails transiently proves the generated client's retry
 // path end to end (Phase 7): the third attempt reaches the server.
 class FlakyTransport final : public smithy::http::HttpClient {

runtime/BUILD.bazel

@@ -64,6 +64,7 @@ cc_library(
     name = "http",
     srcs = [
         "src/http/headers.cc",
+        "src/http/server_dispatch.cc",
         "src/http/socket_transport.cc",
         "src/http/trace_context.cc",
         "src/http/uri.cc",
@@ -72,6 +73,7 @@ cc_library(
         "include/smithy/http/headers.h",
         "include/smithy/http/loopback.h",
         "include/smithy/http/message.h",
+        "include/smithy/http/server_dispatch.h",
         "include/smithy/http/socket_transport.h",
         "include/smithy/http/trace_context.h",
         "include/smithy/http/transport.h",
@@ -127,7 +129,10 @@ cc_test(
 cc_test(
     name = "http_test",
     size = "small",
-    srcs = ["tests/http/http_test.cc"],
+    srcs = [
+        "tests/http/http_test.cc",
+        "tests/http/server_dispatch_test.cc",
+    ],
     copts = COPTS,
     deps = [
         ":http",

runtime/include/smithy/core/timestamp.h

@@ -30,6 +30,14 @@ class Timestamp {
   // Rounds to the nearest millisecond.
   static Timestamp FromEpochSeconds(double seconds);
 
+  // Range-checked factories for untrusted wire input. Serde uses these so a
+  // hostile or malformed number cannot overflow the int64/double arithmetic
+  // (undefined behavior) or land on an instant that formats to unparsable
+  // text: they reject any value whose civil year falls outside the
+  // RFC 3339 / IMF-fixdate representable window (0000-9999).
+  static Outcome<Timestamp> FromEpochSecondsChecked(double seconds);
+  static Outcome<Timestamp> FromEpochMillisecondsChecked(std::int64_t ms);
+
   std::int64_t epoch_milliseconds() const { return ms_; }
   double epoch_seconds() const { return static_cast<double>(ms_) / 1000.0; }
 

runtime/include/smithy/http/loopback.h

@@ -3,6 +3,7 @@
 
 #include <utility>
 
+#include "smithy/http/server_dispatch.h"
 #include "smithy/http/transport.h"
 
 namespace smithy::http {
@@ -23,7 +24,7 @@ class Loopback : public HttpClient, public HttpServerTransport {
   // HttpClient:
   Outcome<HttpResponse> Send(const HttpRequest& request) override {
     if (!handler_) return Error::Transport("loopback: no handler installed", /*retryable=*/false);
-    return handler_(request);
+    return InvokeHandlerGuarded(handler_, request);
   }
 
  private:

runtime/include/smithy/http/server_dispatch.h

@@ -0,0 +1,25 @@
+#ifndef SMITHY_HTTP_SERVER_DISPATCH_H_
+#define SMITHY_HTTP_SERVER_DISPATCH_H_
+
+#include "smithy/http/message.h"
+#include "smithy/http/transport.h"
+
+namespace smithy::http {
+
+// Invokes handler(request), converting any exception that escapes the handler
+// into a 500 response instead of letting it propagate. Server transports call
+// this rather than invoking the handler directly, so a throwing handler fails
+// exactly one request instead of unwinding out of the transport's I/O thread
+// and terminating the whole process.
+//
+// The synthesized 500 carries a generated correlation id in the
+// "x-correlation-id" header (and a minimal JSON body repeating it); the same id
+// plus the exception's what() is written to std::clog, so an otherwise-silent
+// crash leaves one greppable line tying the client-visible failure to a
+// server-side cause. handler may be empty — that yields a 503 (no correlation
+// id: nothing ran).
+HttpResponse InvokeHandlerGuarded(const RequestHandler& handler, const HttpRequest& request);
+
+}  // namespace smithy::http
+
+#endif  // SMITHY_HTTP_SERVER_DISPATCH_H_

runtime/include/smithy/http/transport.h

@@ -24,8 +24,10 @@ class HttpClient {
   }
 };
 
-// What a server transport calls for each incoming request. Handlers must not
-// throw; failures are expressed as HTTP responses.
+// What a server transport calls for each incoming request. Handlers express
+// failures as HTTP responses; a handler that nonetheless throws is contained
+// by the transport (see smithy/http/server_dispatch.h) as a 500 with a
+// correlation id rather than taking down the process.
 using RequestHandler = std::function<HttpResponse(const HttpRequest&)>;
 
 // Server-side transport: binds a listener and dispatches requests to a

runtime/src/cbor/cbor.cc

@@ -281,13 +281,19 @@ class Decoder {
         auto inner = DecodeValue(depth - 1);
         if (!inner) return std::move(inner).error();
         if (*tag == kTagEpochTimestamp) {
+          // Tag 1 content is epoch seconds (integer or float). Route both
+          // through the range-checked factory: a large integer would overflow
+          // int64 when scaled to milliseconds, and an out-of-range float would
+          // overflow the cast — both undefined behavior on untrusted input.
           if (inner->is_int()) {
-            return Document(TimestampValue{Timestamp::FromEpochMilliseconds(inner->as_int() * 1000),
-                                           TimestampFormat::kEpochSeconds});
+            auto ts = Timestamp::FromEpochSecondsChecked(static_cast<double>(inner->as_int()));
+            if (!ts) return std::move(ts).error();
+            return Document(TimestampValue{*ts, TimestampFormat::kEpochSeconds});
           }
           if (inner->is_double()) {
-            return Document(TimestampValue{Timestamp::FromEpochSeconds(inner->as_double()),
-                                           TimestampFormat::kEpochSeconds});
+            auto ts = Timestamp::FromEpochSecondsChecked(inner->as_double());
+            if (!ts) return std::move(ts).error();
+            return Document(TimestampValue{*ts, TimestampFormat::kEpochSeconds});
           }
           return Fail("tag 1 content is not a number");
         }

runtime/src/core/document_serde.cc

@@ -19,7 +19,7 @@ Outcome<Timestamp> TimestampFromDocument(const Document& doc, TimestampFormat fo
     if (format != TimestampFormat::kEpochSeconds) {
       return Error::Serialization("timestamp: numeric value for a string-formatted timestamp");
     }
-    return Timestamp::FromEpochSeconds(doc.AsNumber());
+    return Timestamp::FromEpochSecondsChecked(doc.AsNumber());
   }
   if (doc.is_string()) {
     if (format == TimestampFormat::kEpochSeconds) {

runtime/src/core/timestamp.cc

@@ -213,7 +213,21 @@ Outcome<Timestamp> ParseEpochSeconds(std::string_view text) {
   errno = 0;
   const double seconds = std::strtod(buffer.c_str(), nullptr);
   if (errno == ERANGE || !std::isfinite(seconds)) return invalid();
-  return Timestamp::FromEpochSeconds(seconds);
+  return Timestamp::FromEpochSecondsChecked(seconds);
+}
+
+// Epoch-milliseconds bounds of the RFC 3339 / IMF-fixdate representable window
+// (0000-01-01T00:00:00.000Z .. 9999-12-31T23:59:59.999Z). Instants outside it
+// both overflow the arithmetic below and format to text no conformant peer can
+// parse, so untrusted numbers beyond it are rejected rather than corrupted.
+constexpr std::int64_t kMinRepresentableMs = -62167219200000;  // year 0000-01-01
+constexpr std::int64_t kMaxRepresentableMs = 253402300799999;  // year 9999-12-31T23:59:59.999
+
+Outcome<Timestamp> CheckedFromMs(std::int64_t ms) {
+  if (ms < kMinRepresentableMs || ms > kMaxRepresentableMs) {
+    return Error::Serialization("timestamp: instant out of representable range (year 0000-9999)");
+  }
+  return Timestamp::FromEpochMilliseconds(ms);
 }
 
 }  // namespace
@@ -222,6 +236,25 @@ Timestamp Timestamp::FromEpochSeconds(double seconds) {
   return Timestamp(static_cast<std::int64_t>(std::llround(seconds * 1000.0)));
 }
 
+Outcome<Timestamp> Timestamp::FromEpochSecondsChecked(double seconds) {
+  if (!std::isfinite(seconds)) {
+    return Error::Serialization("timestamp: epoch-seconds is not finite");
+  }
+  // Bound the value before scaling so neither `seconds * 1000` nor the cast to
+  // int64 can overflow; CheckedFromMs then applies the exact year window.
+  constexpr double kMaxSeconds = 253402300800.0;  // just past year 9999
+  constexpr double kMinSeconds = -62167219200.0;  // year 0000-01-01
+  if (seconds < kMinSeconds || seconds > kMaxSeconds) {
+    return Error::Serialization(
+        "timestamp: epoch-seconds out of representable range (year 0000-9999)");
+  }
+  return CheckedFromMs(static_cast<std::int64_t>(std::llround(seconds * 1000.0)));
+}
+
+Outcome<Timestamp> Timestamp::FromEpochMillisecondsChecked(std::int64_t ms) {
+  return CheckedFromMs(ms);
+}
+
 std::string Timestamp::Format(TimestampFormat format) const {
   const CivilTime c = Decompose(ms_);
   std::array<char, 40> buffer{};

runtime/src/http/beast_transport.cc

@@ -19,6 +19,7 @@
 #include <utility>
 #include <vector>
 
+#include "smithy/http/server_dispatch.h"
 #include "smithy/http/uri.h"
 
 namespace smithy::http {
@@ -137,10 +138,11 @@ struct BeastServerTransport::State : std::enable_shared_from_this<State> {
           self->active.fetch_add(1);
           const bool keep_alive = parser->get().keep_alive() && !self->stopping;
           const HttpRequest request = ToSmithyRequest(parser->get());
-          // Handlers are synchronous for now (ADR-0003 keeps them exception-free);
-          // they run on the pool thread that completed the read.
-          const HttpResponse response =
-              self->handler ? self->handler(request) : HttpResponse{503, {}, ""};
+          // Handlers run synchronously on the pool thread that completed the
+          // read. InvokeHandlerGuarded contains any exception the handler
+          // throws as a 500 — otherwise it would unwind out of io_context::run
+          // and terminate the process, dropping every in-flight request.
+          const HttpResponse response = InvokeHandlerGuarded(self->handler, request);
           auto wire = std::make_shared<bhttp::response<bhttp::string_body>>(
               ToWireResponse(response, keep_alive));
           auto& wire_stream = *stream;

runtime/src/http/server_dispatch.cc

@@ -0,0 +1,42 @@
+#include "smithy/http/server_dispatch.h"
+
+#include <exception>
+#include <iostream>
+#include <string>
+
+#include "smithy/core/uuid.h"
+
+namespace smithy::http {
+namespace {
+
+HttpResponse InternalError(const HttpRequest& request, const std::string& what) {
+  const std::string correlation_id = GenerateUuidV4();
+  // The built-in default sink. A structured-logging seam can replace this, but
+  // an unhandled handler exception must never be silent — this line is often
+  // the only server-side trace of a 500.
+  std::clog << "smithy: handler threw; correlation-id=" << correlation_id << " request=\""
+            << request.method << ' ' << request.target << "\" what=\"" << what << "\"\n";
+  HttpResponse response;
+  response.status = 500;
+  response.headers.Set("content-type", "application/json");
+  response.headers.Set("x-correlation-id", correlation_id);
+  response.body = "{\"message\":\"internal error\",\"correlationId\":\"" + correlation_id + "\"}";
+  return response;
+}
+
+}  // namespace
+
+HttpResponse InvokeHandlerGuarded(const RequestHandler& handler, const HttpRequest& request) {
+  if (!handler) {
+    return HttpResponse{503, {}, "", ""};
+  }
+  try {
+    return handler(request);
+  } catch (const std::exception& e) {
+    return InternalError(request, e.what());
+  } catch (...) {
+    return InternalError(request, "unknown exception");
+  }
+}
+
+}  // namespace smithy::http

runtime/src/http/socket_transport.cc

@@ -19,6 +19,8 @@
 #include <unistd.h>
 #endif
 
+#include "smithy/http/server_dispatch.h"
+
 namespace smithy::http {
 namespace {
 
@@ -297,7 +299,7 @@ void SocketHttpServer::AcceptLoop() {
         request.target = line.substr(first + 1, second - first - 1);
         request.headers = std::move(message->headers);
         request.body = std::move(message->body);
-        response = handler_(request);
+        response = InvokeHandlerGuarded(handler_, request);
       }
     } else {
       response = HttpResponse{400, {}, message.error().message()};

runtime/src/server/middleware.cc

@@ -2,6 +2,8 @@
 
 #include <cctype>
 #include <cstddef>
+#include <exception>
+#include <iostream>
 #include <optional>
 #include <ranges>
 #include <string>
@@ -33,7 +35,16 @@ Middleware Observe(std::function<void(const RequestObservation&)> callback,
       observation.trace_parent = request.headers.Get("traceparent").value_or("");
       observation.status = response.status;
       observation.duration = std::chrono::duration_cast<std::chrono::milliseconds>(now() - start);
-      callback(observation);
+      // A throwing observation sink (e.g. a metrics backend under backpressure)
+      // must not discard an already-built response or unwind into the
+      // transport thread; swallow it after logging.
+      try {
+        callback(observation);
+      } catch (const std::exception& e) {
+        std::clog << "smithy: Observe callback threw: " << e.what() << "\n";
+      } catch (...) {
+        std::clog << "smithy: Observe callback threw a non-std exception\n";
+      }
       return response;
     };
   };

runtime/tests/cbor/cbor_test.cc

@@ -86,6 +86,19 @@ TEST(CborTest, DecodesTag1Timestamps) {
   EXPECT_EQ(fractional->as_timestamp().value.epoch_milliseconds(), 1363896240500);
 }
 
+// A ~9-byte tag-1 payload whose integer seconds, scaled to milliseconds, would
+// overflow int64 (the old `as_int() * 1000` was undefined behavior). Likewise a
+// tag-1 double far outside the representable range. Both must be a clean
+// decode error, not UB.
+TEST(CborTest, RejectsOutOfRangeTag1TimestampsWithoutOverflow) {
+  // c1 1b 7fffffffffffffff = tag 1 + uint64 9223372036854775807 seconds.
+  EXPECT_FALSE(Decode(FromHex("c11b7fffffffffffffff")).ok());
+  // c1 3b 7fffffffffffffff = tag 1 + a huge negative integer.
+  EXPECT_FALSE(Decode(FromHex("c13b7fffffffffffffff")).ok());
+  // c1 fb 7fe0000000000000 = tag 1 + double ~1.8e308 (near DBL_MAX).
+  EXPECT_FALSE(Decode(FromHex("c1fb7fe0000000000000")).ok());
+}
+
 TEST(CborTest, RoundTripsComplexDocument) {
   DocumentMap map;
   map.emplace("null", Document(nullptr));