Testing & CI hardening phase 2: fuzz the HTTP/1.1 parser, even out malformed-server coverage (#48)

.github/workflows/ci.yml

@@ -87,7 +87,7 @@ jobs:
           CXX: clang++
         run: |
           set -euo pipefail
-          for target in json_decode cbor_decode uri server_dispatch regex; do
+          for target in json_decode cbor_decode uri server_dispatch regex http1; do
             echo "== fuzzing $target"
             bazelisk build --config=fuzz "//fuzz:${target}_fuzz"
             ./bazel-bin/fuzz/${target}_fuzz -max_total_time=30 -print_final_stats=1
@@ -181,7 +181,8 @@ jobs:
       # its shape is locked by the golden diff check in the codegen job.
       - name: clang-format
         run: |
-          find runtime examples codegen/compile-tests \( -name '*.h' -o -name '*.cc' \) \
+          find runtime examples codegen/compile-tests protocol-tests \
+            \( -name '*.h' -o -name '*.cc' \) \
             ! -path '*/generated/*' | xargs clang-format --dry-run --Werror
       # Excluded from tidy: src/json/json.cc includes the nlohmann backend,
       # which only exists inside the Bazel build graph; beast_src.cc is the

CHANGELOG.md

@@ -79,5 +79,19 @@ via `git_override` until then.
   (`Search(text, &steps)` instrumentation) instead of a wall-clock limit.
 - `make verify` / `make verify-full`: one-command local verification
   mirroring the CI jobs one-to-one.
+- **HTTP/1.1 parser extracted and fuzzed** (`smithy/http/http1.h`): the
+  socket transports' hand-rolled message reader is now a pure,
+  callback-fed function with a libFuzzer harness (`//fuzz:http1_fuzz`, in
+  the CI smoke loop) and a platform-independent hostile bank
+  (`http1_hostile_test.cc`) covering smuggling framing, hostile
+  content-lengths, truncation-everywhere, and header floods. Hardening
+  found while banking: an empty or `+`-signed Content-Length previously
+  parsed as a valid length; both now reject (digits-only per RFC 9110).
+- **Malformed-server coverage evened out**: hand-written suites pin how the
+  generated simpleRestJson and rpcv2Cbor servers reject hostile requests
+  (unparseable bodies, protocol-precondition violations, wrong
+  content-type/method/route) the way jsonrpc2's generated suite always did —
+  including the previously-unasserted simpleRestJson `@pattern`-violation
+  wire message.
 
 [Unreleased]: https://github.com/aaylward/smithy-cpp/commits/main

Makefile

@@ -37,7 +37,8 @@ goldens:
 
 .PHONY: lint
 lint:
-	find runtime examples codegen/compile-tests \( -name '*.h' -o -name '*.cc' \) \
+	find runtime examples codegen/compile-tests protocol-tests \
+		\( -name '*.h' -o -name '*.cc' \) \
 		! -path '*/generated/*' | xargs clang-format --dry-run --Werror
 	buildifier --lint=warn --mode=check -r .
 
@@ -53,7 +54,7 @@ sanitize:
 
 .PHONY: fuzz-smoke
 fuzz-smoke:
-	set -e; for target in json_decode cbor_decode uri server_dispatch regex; do \
+	set -e; for target in json_decode cbor_decode uri server_dispatch regex http1; do \
 		echo "== fuzzing $$target"; \
 		CC=clang CXX=clang++ $(BAZEL) build --config=fuzz "//fuzz:$${target}_fuzz"; \
 		./bazel-bin/fuzz/$${target}_fuzz -max_total_time=30 -print_final_stats=1; \
@@ -73,7 +74,8 @@ benchmarks:
 # Rewrites instead of checking: the fix-it twin of `lint` + codegen's spotless.
 .PHONY: format
 format:
-	find runtime examples codegen/compile-tests \( -name '*.h' -o -name '*.cc' \) \
+	find runtime examples codegen/compile-tests protocol-tests \
+		\( -name '*.h' -o -name '*.cc' \) \
 		! -path '*/generated/*' | xargs clang-format -i
 	buildifier --lint=warn -r .
 	cd codegen && $(GRADLE) spotlessApply

fuzz/BUILD.bazel

@@ -11,6 +11,7 @@ FUZZ_TARGETS = {
         "//runtime:cbor",
         "//runtime:core",
     ],
+    "http1": ["//runtime:http"],
     "regex": ["//runtime:core"],
     "uri": ["//runtime:http"],
     "server_dispatch": [

fuzz/http1_fuzz.cc

@@ -0,0 +1,61 @@
+// Fuzz target: the hand-rolled HTTP/1.1 message reader behind the socket
+// transports (issue #48 — the one network-facing parser that had no fuzz
+// coverage). The input's first byte picks the read mode and a chunking
+// pattern; the rest is the wire stream, delivered in varying slices so the
+// incremental header/body accumulation paths get exercised, not just the
+// all-at-once happy path. The parser must never crash, over-read, or accept
+// a body larger than its documented cap; the start-line helpers must never
+// crash on arbitrary bytes.
+#include <algorithm>
+#include <cstddef>
+#include <cstdint>
+#include <cstdlib>
+#include <cstring>
+#include <string>
+#include <string_view>
+
+#include "smithy/http/http1.h"
+
+namespace {
+
+constexpr std::size_t kMaxBodyBytes = std::size_t{64} * 1024 * 1024;
+
+}  // namespace
+
+extern "C" int LLVMFuzzerTestOneInput(const std::uint8_t* data, std::size_t size) {
+  if (size == 0) return 0;
+  const std::uint8_t control = data[0];
+  const bool body_until_eof = (control & 1) != 0;
+  // Chunk sizes cycle through a pattern seeded by the control byte: sizes of
+  // 1 stress byte-at-a-time accumulation, larger ones the buffered path.
+  const std::size_t patterns[4][3] = {{1, 1, 1}, {1, 7, 4096}, {3, 8192, 2}, {8192, 8192, 8192}};
+  const std::size_t* chunk_sizes = patterns[(control >> 1) & 3];
+
+  const char* wire = reinterpret_cast<const char*>(data + 1);
+  std::size_t remaining = size - 1;
+  std::size_t offset = 0;
+  int call = 0;
+  const auto read = [&](char* buffer, std::size_t capacity) -> long {
+    const std::size_t want = std::min(capacity, chunk_sizes[call++ % 3]);
+    const std::size_t take = std::min(want, remaining - offset);
+    if (take == 0) return 0;  // EOF
+    std::memcpy(buffer, wire + offset, take);
+    offset += take;
+    return static_cast<long>(take);
+  };
+
+  auto message = smithy::http::ReadHttp1Message(read, body_until_eof);
+  if (message.ok()) {
+    if (message->body.size() > kMaxBodyBytes) std::abort();
+    // Whatever parsed must be internally consistent enough to iterate.
+    for (const auto& [name, value] : message->headers.entries()) {
+      if (name.find("\r\n") != std::string::npos) std::abort();
+      (void)value;
+    }
+    std::string method;
+    std::string target;
+    (void)smithy::http::ParseRequestLine(message->start_line, &method, &target);
+    (void)smithy::http::ParseStatusLine(message->start_line);
+  }
+  return 0;
+}

protocol-tests/rpcv2cbor/malformed/BUILD.bazel

@@ -0,0 +1,21 @@
+load("@rules_cc//cc:defs.bzl", "cc_test")
+
+# Hand-written malformed-server coverage for rpcv2Cbor (issue #48): the
+# official conformance suite has no httpMalformedRequestTests for this
+# protocol, so this pins the generated server's reject paths the way
+# jsonrpc2's generated server_malformed_tests.cc does for its protocol.
+# Lives outside generated/ because that tree is a golden regenerated
+# byte-for-byte in CI.
+
+cc_test(
+    name = "server_malformed_test",
+    size = "small",
+    srcs = ["server_malformed_test.cc"],
+    deps = [
+        "//protocol-tests/rpcv2cbor/generated:server",
+        "//runtime:cbor",
+        "//runtime:core",
+        "//runtime:http",
+        "@googletest//:gtest_main",
+    ],
+)

protocol-tests/rpcv2cbor/malformed/server_malformed_test.cc

@@ -0,0 +1,175 @@
+// Hand-written malformed-server suite for rpcv2Cbor (issue #48). The
+// official Smithy conformance suite carries no httpMalformedRequestTests for
+// this protocol, so this pins how the generated RpcV2Protocol server rejects
+// hostile requests — protocol preconditions, unparseable CBOR, bad routing —
+// before the handler runs. Lives outside generated/ because that tree is a
+// golden the codegen CI job regenerates byte-for-byte.
+
+#include <gtest/gtest.h>
+
+#include <memory>
+#include <string>
+
+#include "smithy/cbor/cbor.h"
+#include "smithy/protocoltests/rpcv2cbor/server.h"
+
+namespace smithy::protocoltests::rpcv2cbor {
+namespace {
+
+class RecordingHandler : public RpcV2ProtocolHandler {
+ public:
+  smithy::Outcome<EmptyInputOutputOutput> EmptyInputOutput(const EmptyInputOutputInput&) override {
+    ++calls;
+    return EmptyInputOutputOutput{};
+  }
+  smithy::Outcome<Float16Output> Float16(const Float16Input&) override {
+    ++calls;
+    return Float16Output{};
+  }
+  smithy::Outcome<FractionalSecondsOutput> FractionalSeconds(
+      const FractionalSecondsInput&) override {
+    ++calls;
+    return FractionalSecondsOutput{};
+  }
+  smithy::Outcome<GreetingWithErrorsOutput> GreetingWithErrors(
+      const GreetingWithErrorsInput&) override {
+    ++calls;
+    return GreetingWithErrorsOutput{};
+  }
+  smithy::Outcome<NoInputOutputOutput> NoInputOutput(const NoInputOutputInput&) override {
+    ++calls;
+    return NoInputOutputOutput{};
+  }
+  smithy::Outcome<OperationWithDefaultsOutput> OperationWithDefaults(
+      const OperationWithDefaultsInput&) override {
+    ++calls;
+    return OperationWithDefaultsOutput{};
+  }
+  smithy::Outcome<OptionalInputOutputOutput> OptionalInputOutput(
+      const OptionalInputOutputInput&) override {
+    ++calls;
+    return OptionalInputOutputOutput{};
+  }
+  smithy::Outcome<RecursiveShapesOutput> RecursiveShapes(const RecursiveShapesInput&) override {
+    ++calls;
+    return RecursiveShapesOutput{};
+  }
+  smithy::Outcome<RpcV2CborDenseMapsOutput> RpcV2CborDenseMaps(
+      const RpcV2CborDenseMapsInput&) override {
+    ++calls;
+    return RpcV2CborDenseMapsOutput{};
+  }
+  smithy::Outcome<RpcV2CborListsOutput> RpcV2CborLists(const RpcV2CborListsInput&) override {
+    ++calls;
+    return RpcV2CborListsOutput{};
+  }
+  smithy::Outcome<RpcV2CborSparseMapsOutput> RpcV2CborSparseMaps(
+      const RpcV2CborSparseMapsInput&) override {
+    ++calls;
+    return RpcV2CborSparseMapsOutput{};
+  }
+  smithy::Outcome<SimpleScalarPropertiesOutput> SimpleScalarProperties(
+      const SimpleScalarPropertiesInput&) override {
+    ++calls;
+    return SimpleScalarPropertiesOutput{};
+  }
+  smithy::Outcome<SparseNullsOperationOutput> SparseNullsOperation(
+      const SparseNullsOperationInput&) override {
+    ++calls;
+    return SparseNullsOperationOutput{};
+  }
+  int calls = 0;
+};
+
+class RpcV2CborMalformedTest : public testing::Test {
+ protected:
+  smithy::http::HttpRequest WellFormedRequest(const std::string& operation) {
+    smithy::http::HttpRequest request;
+    request.method = "POST";
+    request.target = "/service/RpcV2Protocol/operation/" + operation;
+    request.headers.Set("smithy-protocol", "rpc-v2-cbor");
+    request.headers.Set("content-type", "application/cbor");
+    return request;
+  }
+
+  smithy::http::HttpResponse Send(const smithy::http::HttpRequest& request) {
+    return server_.Handler()(request);
+  }
+
+  // The protocol serializes errors as a CBOR map carrying __type.
+  std::string ErrorTypeOf(const smithy::http::HttpResponse& response) {
+    EXPECT_EQ(response.headers.Get("smithy-protocol").value_or("<missing>"), "rpc-v2-cbor");
+    EXPECT_EQ(response.headers.Get("content-type").value_or("<missing>"), "application/cbor");
+    const auto body = smithy::cbor::Decode(Blob::FromString(response.body));
+    EXPECT_TRUE(body.ok());
+    if (!body.ok() || !body->is_map()) return "<unparseable>";
+    const smithy::Document* type = body->Find("__type");
+    return type == nullptr ? "<missing>" : std::string(type->as_string());
+  }
+
+  std::shared_ptr<RecordingHandler> handler_ = std::make_shared<RecordingHandler>();
+  RpcV2ProtocolServer server_{handler_};
+};
+
+TEST_F(RpcV2CborMalformedTest, MissingSmithyProtocolHeaderIsRejected) {
+  auto request = WellFormedRequest("NoInputOutput");
+  request.headers.Remove("smithy-protocol");
+  const auto response = Send(request);
+  EXPECT_EQ(response.status, 400);
+  EXPECT_EQ(ErrorTypeOf(response), "SerializationException");
+  EXPECT_EQ(handler_->calls, 0);
+}
+
+TEST_F(RpcV2CborMalformedTest, WrongSmithyProtocolHeaderIsRejected) {
+  auto request = WellFormedRequest("NoInputOutput");
+  request.headers.Set("smithy-protocol", "rpc-v2-json");
+  const auto response = Send(request);
+  EXPECT_EQ(response.status, 400);
+  EXPECT_EQ(ErrorTypeOf(response), "SerializationException");
+  EXPECT_EQ(handler_->calls, 0);
+}
+
+TEST_F(RpcV2CborMalformedTest, WrongContentTypeIs415) {
+  auto request = WellFormedRequest("SimpleScalarProperties");
+  request.headers.Set("content-type", "application/json");
+  request.body = "{}";
+  const auto response = Send(request);
+  EXPECT_EQ(response.status, 415);
+  EXPECT_EQ(ErrorTypeOf(response), "UnsupportedMediaTypeException");
+  EXPECT_EQ(handler_->calls, 0);
+}
+
+TEST_F(RpcV2CborMalformedTest, TruncatedCborBodyIsSerializationException) {
+  auto request = WellFormedRequest("SimpleScalarProperties");
+  request.body = "\x18";  // uint8 header, argument byte missing
+  const auto response = Send(request);
+  EXPECT_EQ(response.status, 400);
+  EXPECT_EQ(ErrorTypeOf(response), "SerializationException");
+  EXPECT_EQ(handler_->calls, 0);
+}
+
+TEST_F(RpcV2CborMalformedTest, NonMapCborBodyIsSerializationException) {
+  auto request = WellFormedRequest("SimpleScalarProperties");
+  request.body = "\x01";  // a bare integer where a structure map is required
+  const auto response = Send(request);
+  EXPECT_EQ(response.status, 400);
+  EXPECT_EQ(ErrorTypeOf(response), "SerializationException");
+  EXPECT_EQ(handler_->calls, 0);
+}
+
+TEST_F(RpcV2CborMalformedTest, UnknownOperationIs404) {
+  const auto response = Send(WellFormedRequest("NoSuchOperation"));
+  EXPECT_EQ(response.status, 404);
+  EXPECT_EQ(handler_->calls, 0);
+}
+
+TEST_F(RpcV2CborMalformedTest, WrongMethodIs405) {
+  auto request = WellFormedRequest("NoInputOutput");
+  request.method = "GET";
+  const auto response = Send(request);
+  EXPECT_EQ(response.status, 405);
+  EXPECT_EQ(handler_->calls, 0);
+}
+
+}  // namespace
+}  // namespace smithy::protocoltests::rpcv2cbor

protocol-tests/simplerestjson/malformed/BUILD.bazel

@@ -0,0 +1,34 @@
+load("@rules_cc//cc:defs.bzl", "cc_test")
+
+# Hand-written malformed-server coverage for simpleRestJson (issue #48): the
+# alloy conformance suite has no httpMalformedRequestTests, so nothing
+# generated pins the reject paths the way jsonrpc2's generated
+# server_malformed_tests.cc does. This package lives outside generated/
+# because that tree is a golden regenerated byte-for-byte in CI.
+
+cc_test(
+    name = "server_malformed_test",
+    size = "small",
+    srcs = ["server_malformed_test.cc"],
+    deps = [
+        "//protocol-tests/simplerestjson/generated:server",
+        "//runtime:http",
+        "//runtime:json",
+        "@googletest//:gtest_main",
+    ],
+)
+
+# The @pattern-violation message simpleRestJson never asserted: the alloy
+# service models no @pattern, so this drives the roundtrip REST fixture's
+# pattern-constrained SinkId instead.
+cc_test(
+    name = "pattern_violation_test",
+    size = "small",
+    srcs = ["pattern_violation_test.cc"],
+    deps = [
+        "//examples/roundtrip/rest/generated:server",
+        "//runtime:http",
+        "//runtime:json",
+        "@googletest//:gtest_main",
+    ],
+)