Skip to content

REF-036.md

Latest commit

 

History

History
118 lines (96 loc) · 4.96 KB

REF-036.md

File metadata and controls

118 lines (96 loc) · 4.96 KB
Raw
id REF-036
workflow github
github_issue emulebb/emulebb#98
title Adopt GSL contracts for buffer and pointer boundary hardening
status OPEN
priority Minor
category refactor
labels
gsl
buffers
contracts
parser
hardening
dependencies
milestone
created 2026-05-01
source 2026-05-01 modern-library hardening review

Workflow status is tracked in GitHub: emulebb/emulebb#98. This local document is retained as an engineering spec/evidence record.

REF-036 - Adopt GSL contracts for buffer and pointer boundary hardening

Summary

Introduce the Guidelines Support Library (GSL) only where it makes current buffer, pointer, and size contracts explicit at risky boundaries.

This is not a general style migration. The useful part of GSL for this codebase is narrow: gsl::span for pointer-plus-length byte ranges, gsl::not_null for required pointers at API boundaries, and gsl::narrow where lossy numeric conversion would otherwise be silent.

Current State

The app still has parser and persistence surfaces where raw pointers, raw byte arrays, and implicit size contracts are easy to misuse:

  • srchybrid/SafeFile.cpp
    • hash and binary read/write helpers such as ReadHash16(uchar*) and WriteHash16(const uchar*).
  • srchybrid/PartFile.cpp
    • raw BYTE* / uchar* buffers, new BYTE[], hash buffers, and WriteToBuffer(...) style helpers.
  • srchybrid/ArchiveRecovery.h and srchybrid/ArchiveRecovery.cpp
    • packed binary structures, owned byte buffers, and helpers such as calcUInt16(BYTE*) / calcUInt32(BYTE*).
  • Newer REST/test helper seams
    • route helpers and DTO boundaries where typed contract helpers can prevent future raw pointer or size drift.

These surfaces are release-relevant because mistakes tend to become corrupted metadata, truncated binary parsing, invalid archive recovery, or silent release failures rather than obvious compile errors.

Intended Scope

  • Add GSL as a setup-managed dependency only with a first real conversion.
  • Start with one small, tested boundary where the contract is already obvious:
    • fixed-size hash buffers in SafeFile/PartFile helpers, or
    • archive recovery byte readers, or
    • a REST/test helper boundary that already has strong regression coverage.
  • Use gsl::span<const std::byte> or a project-appropriate byte type when the function requires both pointer and length.
  • Use fixed-extent spans where the contract is truly fixed-size, for example a 16-byte hash.
  • Use gsl::not_null<T*> only on boundaries that already require non-null and where call sites can satisfy that contract clearly.
  • Use gsl::narrow only in checked paths where rejecting or asserting lossy conversion is the intended behavior.

Out of Scope

  • Sweeping every pointer in the codebase.
  • Replacing MFC containers or UI APIs with GSL wrappers.
  • Introducing GSL into hot protocol paths without targeted tests.
  • Converting broad call graphs just because a leaf helper was improved.
  • Using GSL as a substitute for real parser validation or bounds checks.

Implementation Notes

  • Pick a boundary that already has tests or where tests can be added in the same slice.
  • Avoid ABI churn on old MFC message handlers and UI callbacks.
  • Prefer wrapper overloads at boundaries over invasive signature changes across many files.
  • Keep old raw-pointer overloads temporarily if they materially reduce risk, but funnel them into the span-based implementation.
  • Where GSL makes a latent contract violation visible, treat that as a separate bug or regression-fix commit unless it is inseparable from the conversion.

Acceptance Criteria

  • GSL is tracked as a setup-managed dependency and is discoverable through the generated workspace dependency contract.
  • The first conversion replaces a real raw pointer/size or fixed-buffer contract with an explicit GSL contract.
  • The converted code keeps current runtime behavior for valid inputs.
  • Invalid size/null/lossy-conversion cases fail deliberately rather than silently.
  • The conversion has targeted test coverage for the touched parser, persistence, or helper boundary.

Test Plan

  • Run setup validation after adding the dependency.
  • Build the app and native tests through the supported emulebb-build entrypoint.
  • For SafeFile/PartFile changes, run part-file persistence, known-file, and hash read/write tests.
  • For archive recovery changes, run archive recovery and malformed-input tests before expanding scope.
  • For REST helper changes, run native web API tests plus the live REST smoke if the route contract changes.
  • Include at least one negative or edge-size test when the conversion changes how invalid input is rejected.

Release Value

This is useful after WIL or alongside a small parser-hardening slice. It is not a release blocker by itself. The value comes from reducing silent buffer and conversion mistakes in the exact areas where broadband release regressions would be expensive to diagnose later.