From bdce9e2e8c4c9c5d66275ac6c41ae9989c3c4547 Mon Sep 17 00:00:00 2001 From: Juan Cruz Viotti Date: Tue, 13 Oct 2026 11:29:20 -0300 Subject: [PATCH 1/3] Higher line coverage in various modules Signed-off-by: Juan Cruz Viotti --- src/core/crypto/crypto_hkdf_apple.cc | 4 + src/core/crypto/crypto_hkdf_openssl.cc | 4 + src/core/crypto/crypto_hkdf_windows.cc | 8 ++ src/core/crypto/crypto_rsa_oaep_apple.cc | 4 + src/core/crypto/crypto_rsa_oaep_openssl.cc | 4 + src/core/crypto/crypto_rsa_oaep_windows.cc | 4 + src/core/crypto/crypto_sign_apple.cc | 24 +++++ src/core/crypto/crypto_sign_openssl.cc | 16 ++++ src/core/crypto/crypto_sign_windows.cc | 24 +++++ src/core/crypto/crypto_verify_apple.cc | 16 ++++ src/core/crypto/crypto_verify_openssl.cc | 12 +++ src/core/crypto/crypto_verify_windows.cc | 16 ++++ src/core/diff/diff.cc | 6 +- src/core/diff/myers.h | 38 ++++---- src/core/dns/idn_hostname.cc | 11 +-- src/core/gzip/inflate.h | 17 ++-- src/core/http/from_date.cc | 2 +- src/core/http/include/sourcemeta/core/http.h | 2 +- .../include/sourcemeta/core/http_message.h | 28 +++--- src/core/idna/idna.cc | 21 +++-- src/core/jsonpath/jsonpath.cc | 71 +++++++------- src/core/jsonpath/parser.h | 9 +- src/core/jsonpointer/position.cc | 63 +++++++------ .../include/sourcemeta/core/markdown.h | 18 +++- src/core/markdown/markdown.cc | 11 ++- src/core/mcp/headers.cc | 4 + .../mcp/include/sourcemeta/core/mcp_logging.h | 4 + .../include/sourcemeta/core/mcp_protocol.h | 32 +++++++ .../mcp/include/sourcemeta/core/mcp_results.h | 4 + src/core/oidc/oidc_authentication.cc | 9 +- src/core/oidc/oidc_id_token.cc | 17 ++-- .../include/sourcemeta/core/punycode.h | 48 ++++++++-- src/core/punycode/punycode.cc | 53 +++++++---- src/core/regex/permissive.h | 37 +++----- src/core/regex/regex.cc | 54 ++++++----- src/core/semver/semver.cc | 14 +-- src/core/terminal/terminal.cc | 8 +- src/core/time/asctime.cc | 6 +- src/core/time/imf_fixdate.cc | 6 +- src/core/time/include/sourcemeta/core/time.h | 6 +- src/core/time/rfc850_date.cc | 6 +- src/core/uri/parse.cc | 94 ++++++++----------- src/core/uri/setters.cc | 41 ++++---- src/core/uritemplate/helpers.h | 16 ++-- src/core/uritemplate/uritemplate_router.cc | 13 +-- .../uritemplate/uritemplate_router_view.cc | 10 +- src/lang/io/io.cc | 18 ++-- .../sourcemeta/core/parallel_for_each.h | 23 ++--- test/benchmark/CMakeLists.txt | 2 + test/benchmark/benchmark_bad_option.clitest | 12 +++ .../benchmark_output_unwritable.clitest | 9 ++ test/diff/diff_line_myers_test.cc | 22 +++++ test/gzip/gzip_compress_test.cc | 9 ++ test/gzip/gzip_decompress_error_test.cc | 22 +++++ test/idna/idna_is_valid_u_label_test.cc | 10 ++ test/io/io_weakly_canonical_test.cc | 24 +++++ test/jsonpath/jsonpath_parse_test.cc | 21 +++++ test/markdown/markdown_pathological_test.cc | 42 +++++++++ test/oauth/oauth_dpop_test.cc | 60 ++++++++++++ test/oidc/oidc_authentication_test.cc | 19 ++++ test/oidc/oidc_id_token_test.cc | 22 +++++ test/oidc/oidc_metadata_test.cc | 20 ++++ test/oidc/oidc_registration_test.cc | 21 +++++ test/punycode/punycode_decode_test.cc | 39 ++++++++ test/punycode/punycode_encode_test.cc | 44 +++++++++ test/test/clitest/CMakeLists.txt | 1 + .../clitest/error_drop_bad_pattern.clitest | 4 + test/test/clitest/error_drop_bad_pattern.txt | 5 + test/uri/uri_path_test.cc | 15 +++ test/uri/uri_relative_to_test.cc | 9 ++ .../uritemplate_expand_uri_test.cc | 43 +++++++++ test/uritemplate/uritemplate_router_test.cc | 18 ++++ .../uritemplate_router_view_test.cc | 44 ++++++++- 73 files changed, 1121 insertions(+), 372 deletions(-) create mode 100644 test/benchmark/benchmark_bad_option.clitest create mode 100644 test/benchmark/benchmark_output_unwritable.clitest create mode 100644 test/test/clitest/error_drop_bad_pattern.clitest create mode 100644 test/test/clitest/error_drop_bad_pattern.txt diff --git a/src/core/crypto/crypto_hkdf_apple.cc b/src/core/crypto/crypto_hkdf_apple.cc index e1f1689092..881011dbd3 100644 --- a/src/core/crypto/crypto_hkdf_apple.cc +++ b/src/core/crypto/crypto_hkdf_apple.cc @@ -20,7 +20,11 @@ auto to_selector(const sourcemeta::core::KDFHash hash) noexcept -> int { return 2; } + // Clang settles that no other value arrives, so only GCC and MSVC + // need to be told that the end of this function is never reached +#if !defined(__clang__) std::unreachable(); +#endif } auto octets(const std::string_view value) noexcept -> const unsigned char * { diff --git a/src/core/crypto/crypto_hkdf_openssl.cc b/src/core/crypto/crypto_hkdf_openssl.cc index 5a72bad778..65500459a5 100644 --- a/src/core/crypto/crypto_hkdf_openssl.cc +++ b/src/core/crypto/crypto_hkdf_openssl.cc @@ -40,7 +40,11 @@ auto digest_name(const sourcemeta::core::KDFHash hash) noexcept -> const return "SHA512"; } + // Clang settles that no other value arrives, so only GCC and MSVC + // need to be told that the end of this function is never reached +#if !defined(__clang__) std::unreachable(); +#endif } // An empty view may carry a null pointer, which the parameter interface would diff --git a/src/core/crypto/crypto_hkdf_windows.cc b/src/core/crypto/crypto_hkdf_windows.cc index 9d1f7a1fe0..5778fe94ba 100644 --- a/src/core/crypto/crypto_hkdf_windows.cc +++ b/src/core/crypto/crypto_hkdf_windows.cc @@ -26,7 +26,11 @@ auto hash_name(const sourcemeta::core::KDFHash hash) noexcept -> LPCWSTR { return BCRYPT_SHA512_ALGORITHM; } + // Clang settles that no other value arrives, so only GCC and MSVC + // need to be told that the end of this function is never reached +#if !defined(__clang__) std::unreachable(); +#endif } auto hash_name_bytes(const sourcemeta::core::KDFHash hash) noexcept -> ULONG { @@ -39,7 +43,11 @@ auto hash_name_bytes(const sourcemeta::core::KDFHash hash) noexcept -> ULONG { return sizeof(BCRYPT_SHA512_ALGORITHM); } + // Clang settles that no other value arrives, so only GCC and MSVC + // need to be told that the end of this function is never reached +#if !defined(__clang__) std::unreachable(); +#endif } auto as_native(const std::string_view value) noexcept -> PUCHAR { diff --git a/src/core/crypto/crypto_rsa_oaep_apple.cc b/src/core/crypto/crypto_rsa_oaep_apple.cc index e708ab3248..43db7f44a8 100644 --- a/src/core/crypto/crypto_rsa_oaep_apple.cc +++ b/src/core/crypto/crypto_rsa_oaep_apple.cc @@ -29,7 +29,11 @@ auto to_oaep_algorithm(const RSAOAEPHash hash) -> SecKeyAlgorithm { return kSecKeyAlgorithmRSAEncryptionOAEPSHA256; } + // Clang settles that no other value arrives, so only GCC and MSVC + // need to be told that the end of this function is never reached +#if !defined(__clang__) std::unreachable(); +#endif } } // namespace diff --git a/src/core/crypto/crypto_rsa_oaep_openssl.cc b/src/core/crypto/crypto_rsa_oaep_openssl.cc index 7f59cebd5a..4648cc3578 100644 --- a/src/core/crypto/crypto_rsa_oaep_openssl.cc +++ b/src/core/crypto/crypto_rsa_oaep_openssl.cc @@ -22,7 +22,11 @@ auto to_message_digest(const RSAOAEPHash hash) -> const EVP_MD * { return EVP_sha256(); } + // Clang settles that no other value arrives, so only GCC and MSVC + // need to be told that the end of this function is never reached +#if !defined(__clang__) std::unreachable(); +#endif } auto configure(EVP_PKEY_CTX *context, const RSAOAEPHash hash) -> bool { diff --git a/src/core/crypto/crypto_rsa_oaep_windows.cc b/src/core/crypto/crypto_rsa_oaep_windows.cc index 6b26326016..dd5ab01620 100644 --- a/src/core/crypto/crypto_rsa_oaep_windows.cc +++ b/src/core/crypto/crypto_rsa_oaep_windows.cc @@ -24,7 +24,11 @@ auto to_cng_algorithm(const RSAOAEPHash hash) -> LPCWSTR { return BCRYPT_SHA256_ALGORITHM; } + // Clang settles that no other value arrives, so only GCC and MSVC + // need to be told that the end of this function is never reached +#if !defined(__clang__) std::unreachable(); +#endif } auto as_buffer(const std::string_view value) -> PUCHAR { diff --git a/src/core/crypto/crypto_sign_apple.cc b/src/core/crypto/crypto_sign_apple.cc index 1288584a9e..6fdcfbbf7c 100644 --- a/src/core/crypto/crypto_sign_apple.cc +++ b/src/core/crypto/crypto_sign_apple.cc @@ -34,7 +34,11 @@ auto to_sec_key_pkcs1_v15_algorithm( return kSecKeyAlgorithmRSASignatureMessagePKCS1v15SHA512; } + // Clang settles that no other value arrives, so only GCC and MSVC + // need to be told that the end of this function is never reached +#if !defined(__clang__) std::unreachable(); +#endif } // These algorithm variants fix the salt length to the hash function output, @@ -51,7 +55,11 @@ auto to_sec_key_pss_algorithm( return kSecKeyAlgorithmRSASignatureMessagePSSSHA512; } + // Clang settles that no other value arrives, so only GCC and MSVC + // need to be told that the end of this function is never reached +#if !defined(__clang__) std::unreachable(); +#endif } auto to_sec_key_ecdsa_algorithm( @@ -66,7 +74,11 @@ auto to_sec_key_ecdsa_algorithm( return kSecKeyAlgorithmECDSASignatureMessageX962SHA512; } + // Clang settles that no other value arrives, so only GCC and MSVC + // need to be told that the end of this function is never reached +#if !defined(__clang__) std::unreachable(); +#endif } auto make_data(const std::string_view value) -> CFDataRef { @@ -332,7 +344,11 @@ auto make_private_key(const std::string_view pem) -> std::optional { } } + // Clang settles that no other value arrives, so only GCC and MSVC + // need to be told that the end of this function is never reached +#if !defined(__clang__) std::unreachable(); +#endif } auto make_ec_private_key(const EllipticCurve curve, @@ -482,7 +498,11 @@ auto eddsa_sign(const PrivateKey &key, const std::string_view message) return edwards448_sign(internal->edwards_seed, message); } + // Clang settles that no other value arrives, so only GCC and MSVC + // need to be told that the end of this function is never reached +#if !defined(__clang__) std::unreachable(); +#endif } auto derive_public_key(const PrivateKey &key) -> std::optional { @@ -542,7 +562,11 @@ auto derive_public_key(const PrivateKey &key) -> std::optional { } } + // Clang settles that no other value arrives, so only GCC and MSVC + // need to be told that the end of this function is never reached +#if !defined(__clang__) std::unreachable(); +#endif } } // namespace sourcemeta::core diff --git a/src/core/crypto/crypto_sign_openssl.cc b/src/core/crypto/crypto_sign_openssl.cc index 1912651bb4..70b4230207 100644 --- a/src/core/crypto/crypto_sign_openssl.cc +++ b/src/core/crypto/crypto_sign_openssl.cc @@ -35,7 +35,11 @@ auto to_message_digest( return EVP_sha512(); } + // Clang settles that no other value arrives, so only GCC and MSVC + // need to be told that the end of this function is never reached +#if !defined(__clang__) std::unreachable(); +#endif } // Refuse encrypted documents by offering no password, rather than letting the @@ -53,7 +57,11 @@ auto to_group_name(const sourcemeta::core::EllipticCurve curve) noexcept return "P-521"; } + // Clang settles that no other value arrives, so only GCC and MSVC + // need to be told that the end of this function is never reached +#if !defined(__clang__) std::unreachable(); +#endif } auto to_pkey_id(const sourcemeta::core::EdwardsCurve curve) noexcept -> int { @@ -64,7 +72,11 @@ auto to_pkey_id(const sourcemeta::core::EdwardsCurve curve) noexcept -> int { return EVP_PKEY_ED448; } + // Clang settles that no other value arrives, so only GCC and MSVC + // need to be told that the end of this function is never reached +#if !defined(__clang__) std::unreachable(); +#endif } // The signing functions only cover the three NIST prime curves, so a parsed key @@ -511,7 +523,11 @@ auto derive_public_key(const PrivateKey &key) -> std::optional { } } + // Clang settles that no other value arrives, so only GCC and MSVC + // need to be told that the end of this function is never reached +#if !defined(__clang__) std::unreachable(); +#endif } } // namespace sourcemeta::core diff --git a/src/core/crypto/crypto_sign_windows.cc b/src/core/crypto/crypto_sign_windows.cc index 70658f3981..6b28915cb0 100644 --- a/src/core/crypto/crypto_sign_windows.cc +++ b/src/core/crypto/crypto_sign_windows.cc @@ -40,7 +40,11 @@ auto to_cng_algorithm( return BCRYPT_SHA512_ALGORITHM; } + // Clang settles that no other value arrives, so only GCC and MSVC + // need to be told that the end of this function is never reached +#if !defined(__clang__) std::unreachable(); +#endif } auto to_ecdsa_algorithm(const sourcemeta::core::EllipticCurve curve) noexcept @@ -54,7 +58,11 @@ auto to_ecdsa_algorithm(const sourcemeta::core::EllipticCurve curve) noexcept return BCRYPT_ECDSA_P521_ALGORITHM; } + // Clang settles that no other value arrives, so only GCC and MSVC + // need to be told that the end of this function is never reached +#if !defined(__clang__) std::unreachable(); +#endif } auto to_ecc_private_magic(const sourcemeta::core::EllipticCurve curve) noexcept @@ -68,7 +76,11 @@ auto to_ecc_private_magic(const sourcemeta::core::EllipticCurve curve) noexcept return BCRYPT_ECDSA_PRIVATE_P521_MAGIC; } + // Clang settles that no other value arrives, so only GCC and MSVC + // need to be told that the end of this function is never reached +#if !defined(__clang__) std::unreachable(); +#endif } struct KeyPair { @@ -414,7 +426,11 @@ auto make_private_key(const std::string_view pem) -> std::optional { } } + // Clang settles that no other value arrives, so only GCC and MSVC + // need to be told that the end of this function is never reached +#if !defined(__clang__) std::unreachable(); +#endif } auto make_ec_private_key(const EllipticCurve curve, @@ -548,7 +564,11 @@ auto eddsa_sign(const PrivateKey &key, const std::string_view message) return edwards448_sign(internal->edwards_seed, message); } + // Clang settles that no other value arrives, so only GCC and MSVC + // need to be told that the end of this function is never reached +#if !defined(__clang__) std::unreachable(); +#endif } auto derive_public_key(const PrivateKey &key) -> std::optional { @@ -607,7 +627,11 @@ auto derive_public_key(const PrivateKey &key) -> std::optional { } } + // Clang settles that no other value arrives, so only GCC and MSVC + // need to be told that the end of this function is never reached +#if !defined(__clang__) std::unreachable(); +#endif } } // namespace sourcemeta::core diff --git a/src/core/crypto/crypto_verify_apple.cc b/src/core/crypto/crypto_verify_apple.cc index 44d71be8da..16777bf470 100644 --- a/src/core/crypto/crypto_verify_apple.cc +++ b/src/core/crypto/crypto_verify_apple.cc @@ -33,7 +33,11 @@ auto to_sec_key_pkcs1_v15_algorithm( return kSecKeyAlgorithmRSASignatureMessagePKCS1v15SHA512; } + // Clang settles that no other value arrives, so only GCC and MSVC + // need to be told that the end of this function is never reached +#if !defined(__clang__) std::unreachable(); +#endif } // These algorithm variants fix the salt length to the hash function output, @@ -50,7 +54,11 @@ auto to_sec_key_pss_algorithm( return kSecKeyAlgorithmRSASignatureMessagePSSSHA512; } + // Clang settles that no other value arrives, so only GCC and MSVC + // need to be told that the end of this function is never reached +#if !defined(__clang__) std::unreachable(); +#endif } auto to_sec_key_ecdsa_algorithm( @@ -65,7 +73,11 @@ auto to_sec_key_ecdsa_algorithm( return kSecKeyAlgorithmECDSASignatureMessageX962SHA512; } + // Clang settles that no other value arrives, so only GCC and MSVC + // need to be told that the end of this function is never reached +#if !defined(__clang__) std::unreachable(); +#endif } auto make_data(const std::string_view value) -> CFDataRef { @@ -355,7 +367,11 @@ auto eddsa_verify(const PublicKey &key, const std::string_view message, return edwards448_verify(internal->edwards_point, message, signature); } + // Clang settles that no other value arrives, so only GCC and MSVC + // need to be told that the end of this function is never reached +#if !defined(__clang__) std::unreachable(); +#endif } auto rsa_public_components(const PublicKey &key) diff --git a/src/core/crypto/crypto_verify_openssl.cc b/src/core/crypto/crypto_verify_openssl.cc index bd71e91962..d3b84cc7f4 100644 --- a/src/core/crypto/crypto_verify_openssl.cc +++ b/src/core/crypto/crypto_verify_openssl.cc @@ -33,7 +33,11 @@ auto to_message_digest( return EVP_sha512(); } + // Clang settles that no other value arrives, so only GCC and MSVC + // need to be told that the end of this function is never reached +#if !defined(__clang__) std::unreachable(); +#endif } auto to_group_name(const sourcemeta::core::EllipticCurve curve) noexcept @@ -47,7 +51,11 @@ auto to_group_name(const sourcemeta::core::EllipticCurve curve) noexcept return "P-521"; } + // Clang settles that no other value arrives, so only GCC and MSVC + // need to be told that the end of this function is never reached +#if !defined(__clang__) std::unreachable(); +#endif } auto to_pkey_id(const sourcemeta::core::EdwardsCurve curve) noexcept -> int { @@ -58,7 +66,11 @@ auto to_pkey_id(const sourcemeta::core::EdwardsCurve curve) noexcept -> int { return EVP_PKEY_ED448; } + // Clang settles that no other value arrives, so only GCC and MSVC + // need to be told that the end of this function is never reached +#if !defined(__clang__) std::unreachable(); +#endif } auto native_rsa_key(const std::string_view modulus, diff --git a/src/core/crypto/crypto_verify_windows.cc b/src/core/crypto/crypto_verify_windows.cc index 7fe0804347..f09a7b2b07 100644 --- a/src/core/crypto/crypto_verify_windows.cc +++ b/src/core/crypto/crypto_verify_windows.cc @@ -38,7 +38,11 @@ auto to_cng_algorithm( return BCRYPT_SHA512_ALGORITHM; } + // Clang settles that no other value arrives, so only GCC and MSVC + // need to be told that the end of this function is never reached +#if !defined(__clang__) std::unreachable(); +#endif } auto to_ecdsa_algorithm(const sourcemeta::core::EllipticCurve curve) noexcept @@ -52,7 +56,11 @@ auto to_ecdsa_algorithm(const sourcemeta::core::EllipticCurve curve) noexcept return BCRYPT_ECDSA_P521_ALGORITHM; } + // Clang settles that no other value arrives, so only GCC and MSVC + // need to be told that the end of this function is never reached +#if !defined(__clang__) std::unreachable(); +#endif } auto to_ecc_public_magic(const sourcemeta::core::EllipticCurve curve) noexcept @@ -66,7 +74,11 @@ auto to_ecc_public_magic(const sourcemeta::core::EllipticCurve curve) noexcept return BCRYPT_ECDSA_PUBLIC_P521_MAGIC; } + // Clang settles that no other value arrives, so only GCC and MSVC + // need to be told that the end of this function is never reached +#if !defined(__clang__) std::unreachable(); +#endif } struct KeyPair { @@ -355,7 +367,11 @@ auto eddsa_verify(const PublicKey &key, const std::string_view message, return edwards448_verify(internal->edwards_point, message, signature); } + // Clang settles that no other value arrives, so only GCC and MSVC + // need to be told that the end of this function is never reached +#if !defined(__clang__) std::unreachable(); +#endif } auto rsa_public_components(const PublicKey &key) diff --git a/src/core/diff/diff.cc b/src/core/diff/diff.cc index 081bd669b7..1368125b76 100644 --- a/src/core/diff/diff.cc +++ b/src/core/diff/diff.cc @@ -224,9 +224,10 @@ auto diff(const std::string_view original, const std::string_view modified, std::vector original_changed(original_size, false); std::vector modified_changed(modified_size, false); + internal::MyersWorkspace workspace; + switch (algorithm) { - case Diff::Algorithm::Myers: { - internal::MyersWorkspace workspace; + case Diff::Algorithm::Myers: // A search only needs its workspace once both sides have something left, // as any other case resolves to a plain insertion or deletion if (original_end > prefix && modified_end > prefix) { @@ -240,7 +241,6 @@ auto diff(const std::string_view original, const std::string_view modified, modified_identities, prefix, modified_end, original_changed, modified_changed, workspace); break; - } } compact_changes(result.original, result.original_ends_with_newline, diff --git a/src/core/diff/myers.h b/src/core/diff/myers.h index 5881f1adbb..d997b121b0 100644 --- a/src/core/diff/myers.h +++ b/src/core/diff/myers.h @@ -3,7 +3,6 @@ #include // assert #include // std::size_t, std::ptrdiff_t -#include // std::unreachable #include // std::vector namespace sourcemeta::core::internal { @@ -46,8 +45,13 @@ inline auto myers_middle_snake(const std::vector &original, workspace.forward[static_cast(offset + 1)] = 0; workspace.backward[static_cast(offset + 1)] = 0; - const auto limit{(original_size + modified_size + 1) / 2}; - for (std::ptrdiff_t depth{0}; depth <= limit; ++depth) { + [[maybe_unused]] const auto limit{(original_size + modified_size + 1) / 2}; + // A middle snake always lands within the bounds the search was given, so one + // of the returns below is always taken. Spelling the search as a loop with no + // exit of its own is what says as much to every compiler, which otherwise has + // to be told separately that the end of the function is never reached + for (std::ptrdiff_t depth{0};; ++depth) { + assert(depth <= limit); for (std::ptrdiff_t diagonal{-depth}; diagonal <= depth; diagonal += 2) { const auto lower{ workspace.forward[static_cast(offset + diagonal - 1)]}; @@ -121,13 +125,6 @@ inline auto myers_middle_snake(const std::vector &original, } } } - - // A middle snake always lands within the bounds the search was given, so the - // loop above returns rather than running out. Unlike a function that ends - // after an exhaustive switch, the search is a loop whose returns are all - // conditional, so every compiler has to be told the end is never reached - assert(false); - std::unreachable(); } inline auto myers_compare(const std::vector &original, @@ -137,17 +134,16 @@ inline auto myers_compare(const std::vector &original, std::vector &original_changed, std::vector &modified_changed, MyersWorkspace &workspace) -> void { - while (original_start < original_end && modified_start < modified_end && - original[original_start] == modified[modified_start]) { - original_start += 1; - modified_start += 1; - } - - while (original_start < original_end && modified_start < modified_end && - original[original_end - 1] == modified[modified_end - 1]) { - original_end -= 1; - modified_end -= 1; - } + // The lines the two sides share at the head and at the tail are taken off + // before any of this is reached, which is what sizes the identity tables and + // the workspace, so no run of equal lines is left at either end to slide + // past here. A sub-problem begins and ends where a slide of the search + // stopped, and a slide stops exactly where the two sides differ, so the same + // holds of what the recursion hands back to itself + assert(original_start == original_end || modified_start == modified_end || + original[original_start] != modified[modified_start]); + assert(original_start == original_end || modified_start == modified_end || + original[original_end - 1] != modified[modified_end - 1]); if (original_start == original_end) { for (auto index{modified_start}; index < modified_end; ++index) { diff --git a/src/core/dns/idn_hostname.cc b/src/core/dns/idn_hostname.cc index 2dea961ca2..672bbad514 100644 --- a/src/core/dns/idn_hostname.cc +++ b/src/core/dns/idn_hostname.cc @@ -61,12 +61,11 @@ auto validate_idn_labels(const std::u32string_view codepoints) -> bool { // the corresponding A-label. std::size_t a_label_octets{label.size()}; if (*kind == IDNALabelKind::ULabel) { - try { - const auto body{utf32_to_punycode(decoded)}; - a_label_octets = 4 + body.size(); - } catch (const PunycodeError &) { - return false; - } + // Classifying the label held it to the octet count a label may carry, + // which leaves it far shorter than what the position counters of the + // encoding could ever run past, so the encoding always answers + const auto body{utf32_to_punycode(decoded)}; + a_label_octets = 4 + body.size(); } else if (*kind == IDNALabelKind::Ascii) { // RFC 5891 §4.2.3.1: a label must not contain "--" in the third and // fourth positions unless it is a valid A-label (an "xn--" ACE label, diff --git a/src/core/gzip/inflate.h b/src/core/gzip/inflate.h index 7f4aa589ac..9951d30323 100644 --- a/src/core/gzip/inflate.h +++ b/src/core/gzip/inflate.h @@ -664,10 +664,7 @@ class InflateDecoder { break; case State::StoredLength: - if (!this->read_stored_length(buffers)) { - return this->leave(buffers, InflateStatus::NeedInput); - } - + this->read_stored_length(buffers); break; case State::StoredData: { const auto progress{this->copy_stored_data(buffers)}; @@ -1100,13 +1097,12 @@ class InflateDecoder { } } - auto read_stored_length(InflateBuffers &buffers) -> bool { + // A block header is only read once the whole margin is in hand, and reading + // it gives back more input than it takes, so these four bytes always stand + // behind it unless the source ended in the middle of them + auto read_stored_length(InflateBuffers &buffers) -> void { if (available_input(buffers) < 4) { - if (buffers.input_final) { - unexpected_end(); - } - - return false; + unexpected_end(); } const auto length{static_cast(buffers.input_next[0]) | @@ -1121,7 +1117,6 @@ class InflateDecoder { buffers.input_next += 4; this->stored_remaining_ = length; this->state_ = State::StoredData; - return true; } auto copy_stored_data(InflateBuffers &buffers) -> InflateStatus { diff --git a/src/core/http/from_date.cc b/src/core/http/from_date.cc index c2ff17f105..af74a0dca6 100644 --- a/src/core/http/from_date.cc +++ b/src/core/http/from_date.cc @@ -7,7 +7,7 @@ namespace sourcemeta::core { -auto http_from_date(const std::string_view value) noexcept +auto http_from_date(const std::string_view value) -> std::optional { if (const auto result{from_imf_fixdate(value)}; result.has_value()) { return result; diff --git a/src/core/http/include/sourcemeta/core/http.h b/src/core/http/include/sourcemeta/core/http.h index 2155722306..81f4100596 100644 --- a/src/core/http/include/sourcemeta/core/http.h +++ b/src/core/http/include/sourcemeta/core/http.h @@ -234,7 +234,7 @@ auto http_negotiate_encoding( /// "Sun Nov 6 08:49:37 1994").has_value()); /// ``` SOURCEMETA_CORE_HTTP_EXPORT -auto http_from_date(const std::string_view value) noexcept +auto http_from_date(const std::string_view value) -> std::optional; /// @ingroup http diff --git a/src/core/http/include/sourcemeta/core/http_message.h b/src/core/http/include/sourcemeta/core/http_message.h index 4168e3bae7..7a69f52158 100644 --- a/src/core/http/include/sourcemeta/core/http_message.h +++ b/src/core/http/include/sourcemeta/core/http_message.h @@ -397,26 +397,24 @@ inline auto http_serialize_headers(const Headers &headers) -> std::string { continue; } - // RFC 9112 §5.1 notes that "a single SP preceding the field line - // value is preferred for consistent readability by humans" + // A field value either carries its own bytes or already is the bytes, so + // which of the two it is settles only what gets read out of it here + std::string_view bytes; if constexpr (requires { value.bytes(); }) { - if (http_field_line_has_forbidden_byte(value.bytes())) { - continue; - } - - result += name; - result += ": "; - result += value.bytes(); + bytes = value.bytes(); } else { - if (http_field_line_has_forbidden_byte(value)) { - continue; - } + bytes = value; + } - result += name; - result += ": "; - result += value; + if (http_field_line_has_forbidden_byte(bytes)) { + continue; } + // RFC 9112 §5.1 notes that "a single SP preceding the field line + // value is preferred for consistent readability by humans" + result += name; + result += ": "; + result += bytes; result += "\r\n"; } diff --git a/src/core/idna/idna.cc b/src/core/idna/idna.cc index 3abb2d051a..304a4ecfeb 100644 --- a/src/core/idna/idna.cc +++ b/src/core/idna/idna.cc @@ -5,6 +5,7 @@ #include #include // std::ranges::lower_bound +#include // assert #include // std::size_t #include // std::optional, std::nullopt #include // std::string, std::u32string @@ -67,11 +68,10 @@ auto validate_a_label_body(const std::string_view encoded, // RFC 5891 §4.2.1: "verify that the A-label produced by the step in // Section 4.4 matches the one provided as input", so re-encoding the decoded // U-label must yield the lowercased bytes we decoded from - try { - return utf32_to_punycode(decoded) == body; - } catch (const PunycodeError &) { - return false; - } + // The A-label this decoded came from is held to the octet count a label may + // carry, which leaves what is re-encoded far shorter than what the position + // counters of the encoding could run past, so the encoding always answers + return utf32_to_punycode(decoded) == body; } } // namespace @@ -408,10 +408,15 @@ auto idna_passes_bidi_rule(const std::u32string_view label) noexcept -> bool { } // RFC 5893 §2 conditions 3 and 6: the label must end with a specific - // class, optionally followed by NSM characters. - for (std::size_t index = label.size(); index-- > 0;) { + // class, optionally followed by NSM characters. The first character was held + // to L, R, or AL above and none of those is an NSM, so walking back from the + // end always reaches a character that answers, which is why the walk is + // spelled without an exit of its own + for (std::size_t index{label.size()};;) { + index -= 1; const auto class_value{bidi_class(label[index])}; if (class_value == BidiClass::NonspacingMark) { + assert(index > 0); continue; } if (is_rtl_label) { @@ -423,8 +428,6 @@ auto idna_passes_bidi_rule(const std::u32string_view label) noexcept -> bool { return class_value == BidiClass::LeftToRight || class_value == BidiClass::EuropeanNumber; } - - return false; } auto idna_is_valid_a_label(const std::string_view label) -> bool { diff --git a/src/core/jsonpath/jsonpath.cc b/src/core/jsonpath/jsonpath.cc index 9bab19fa35..b6c762d7d4 100644 --- a/src/core/jsonpath/jsonpath.cc +++ b/src/core/jsonpath/jsonpath.cc @@ -307,9 +307,9 @@ auto filter_query_visit_iterative( const auto *const selectors_end{selector + segment.selectors.size()}; for (; selector != selectors_end; ++selector) { switch (static_cast(selector->index())) { - case JSONPath::SelectorKind::Name: { - const auto *entry{std::get_if(selector)}; + case JSONPath::SelectorKind::Name: if (type == JSON::Type::Object) { + const auto *entry{std::get_if(selector)}; const auto *child{current.try_at(entry->name, entry->hash)}; if (child != nullptr) { pool.push_back({.node = child, .cursor = item.cursor + 1}); @@ -317,7 +317,6 @@ auto filter_query_visit_iterative( } break; - } case JSONPath::SelectorKind::Wildcard: if (type == JSON::Type::Object) { const auto &object{current.as_object()}; @@ -335,9 +334,9 @@ auto filter_query_visit_iterative( } break; - case JSONPath::SelectorKind::Index: { - const auto *entry{std::get_if(selector)}; + case JSONPath::SelectorKind::Index: if (type == JSON::Type::Array) { + const auto *entry{std::get_if(selector)}; const auto &array{current.as_array()}; const auto size{static_cast(array.size())}; const auto index{entry->index < 0 ? entry->index + size @@ -350,7 +349,6 @@ auto filter_query_visit_iterative( } break; - } case JSONPath::SelectorKind::Slice: { const auto *entry{std::get_if(selector)}; if (type == JSON::Type::Array) { @@ -863,6 +861,27 @@ inline auto filter_compare(const JSONPath::FilterComparison &comparison, #endif } +// A test either runs a query and answers whether it reached anything or calls +// a function that answers on its own, and what it comes to may be negated +// either way +inline auto filter_test_matches(const JSONPath::FilterTest &test, + const JSON &candidate, const JSON &root) + -> bool { + bool result{false}; + if (test.subject.index() == 0) { + const auto &query{*std::get_if(&test.subject)}; + result = !filter_query_visit(query.segments, 0, + query.relative ? candidate : root, root, + [](const JSON &) -> bool { return false; }); + } else { + result = evaluate_logical_function( + *std::get_if(&test.subject), candidate, + root); + } + + return test.negated ? !result : result; +} + inline auto filter_matches(const JSONPath::FilterExpression &expression, const JSON &candidate, const JSON &root) -> bool { switch ( @@ -871,22 +890,10 @@ inline auto filter_matches(const JSONPath::FilterExpression &expression, return filter_compare( *std::get_if(&expression.value), candidate, root); - case JSONPath::FilterExpressionKind::Test: { - const auto &test{*std::get_if(&expression.value)}; - bool result{false}; - if (test.subject.index() == 0) { - const auto &query{*std::get_if(&test.subject)}; - result = !filter_query_visit( - query.segments, 0, query.relative ? candidate : root, root, - [](const JSON &) -> bool { return false; }); - } else { - result = evaluate_logical_function( - *std::get_if(&test.subject), - candidate, root); - } - - return test.negated ? !result : result; - } + case JSONPath::FilterExpressionKind::Test: + return filter_test_matches( + *std::get_if(&expression.value), candidate, + root); case JSONPath::FilterExpressionKind::Conjunction: { const auto &conjunction{ *std::get_if(&expression.value)}; @@ -1005,9 +1012,9 @@ inline auto evaluate_query(const std::vector &segments, const auto *const selectors_end{selector + segment.selectors.size()}; for (; selector != selectors_end; ++selector) { switch (static_cast(selector->index())) { - case JSONPath::SelectorKind::Name: { - const auto *entry{std::get_if(selector)}; + case JSONPath::SelectorKind::Name: if (type == JSON::Type::Object) { + const auto *entry{std::get_if(selector)}; const auto *child{current.try_at(entry->name, entry->hash)}; if (child != nullptr) { stack.push_visit_property(child, item.cursor + 1, child_depth, @@ -1016,7 +1023,6 @@ inline auto evaluate_query(const std::vector &segments, } break; - } case JSONPath::SelectorKind::Wildcard: if (type == JSON::Type::Object) { const auto &object{current.as_object()}; @@ -1037,9 +1043,9 @@ inline auto evaluate_query(const std::vector &segments, } break; - case JSONPath::SelectorKind::Index: { - const auto *entry{std::get_if(selector)}; + case JSONPath::SelectorKind::Index: if (type == JSON::Type::Array) { + const auto *entry{std::get_if(selector)}; const auto &array{current.as_array()}; const auto size{static_cast(array.size())}; const auto index{entry->index < 0 ? entry->index + size @@ -1053,7 +1059,6 @@ inline auto evaluate_query(const std::vector &segments, } break; - } case JSONPath::SelectorKind::Slice: { const auto *entry{std::get_if(selector)}; if (type == JSON::Type::Array) { @@ -1190,9 +1195,9 @@ inline auto evaluate_selectors(const std::vector &segments, const auto *const selectors_end{selector + segment.selectors.size()}; for (; selector != selectors_end; ++selector) { switch (static_cast(selector->index())) { - case JSONPath::SelectorKind::Name: { - const auto *entry{std::get_if(selector)}; + case JSONPath::SelectorKind::Name: if (type == JSON::Type::Object) { + const auto *entry{std::get_if(selector)}; const auto *child{current.try_at(entry->name, entry->hash)}; if (child != nullptr) { state.frames.push_property(entry->name, entry->hash); @@ -1202,7 +1207,6 @@ inline auto evaluate_selectors(const std::vector &segments, } break; - } case JSONPath::SelectorKind::Wildcard: if (type == JSON::Type::Object) { const auto &object{current.as_object()}; @@ -1225,9 +1229,9 @@ inline auto evaluate_selectors(const std::vector &segments, } break; - case JSONPath::SelectorKind::Index: { - const auto *entry{std::get_if(selector)}; + case JSONPath::SelectorKind::Index: if (type == JSON::Type::Array) { + const auto *entry{std::get_if(selector)}; const auto &array{current.as_array()}; const auto size{static_cast(array.size())}; const auto index{entry->index < 0 ? entry->index + size @@ -1243,7 +1247,6 @@ inline auto evaluate_selectors(const std::vector &segments, } break; - } case JSONPath::SelectorKind::Slice: { const auto *entry{std::get_if(selector)}; if (type == JSON::Type::Array) { diff --git a/src/core/jsonpath/parser.h b/src/core/jsonpath/parser.h index 1cd5e7aec8..832ec39622 100644 --- a/src/core/jsonpath/parser.h +++ b/src/core/jsonpath/parser.h @@ -886,11 +886,10 @@ class JSONPathParser { return JSON{exact.value()}; } - try { - return JSON{Decimal{text}}; - } catch (const DecimalParseError &) { - this->fail(); - } + // The range check above holds the literal to what the double precision + // format reaches, which sits far inside the exponent a decimal carries, so + // reading it as one always answers + return JSON{Decimal{text}}; } // filter-query = rel-query / jsonpath-query diff --git a/src/core/jsonpointer/position.cc b/src/core/jsonpointer/position.cc index f27ad6caf8..23035f5735 100644 --- a/src/core/jsonpointer/position.cc +++ b/src/core/jsonpointer/position.cc @@ -8,6 +8,32 @@ #include // std::uint64_t #include // std::optional #include // std::string_view +#include // std::vector + +namespace { + +// A property step and an index step differ in nothing but which of the two +// child maps of a node the step is remembered in, so what a step does with +// what it finds there is spelled out once here. The map is reached through the +// node each time, as taking in a new node may move the ones already held +template +auto descend_into(std::vector &trie, Children Node::*children, + const Key &key, std::size_t ¤t_node) -> void { + const auto match{(trie[current_node].*children).find(key)}; + if (match != (trie[current_node].*children).end()) { + current_node = match->second; + return; + } + + const auto node_index{trie.size()}; + trie.push_back({.position = std::nullopt, + .index_children = {}, + .property_children = {}}); + (trie[current_node].*children).emplace(key, node_index); + current_node = node_index; +} + +} // namespace namespace sourcemeta::core { @@ -32,40 +58,15 @@ auto PointerPositionTracker::ensure_index() const -> void { std::make_pair(event.line, event.column)); switch (event.context) { - case JSON::ParseContext::Property: { + case JSON::ParseContext::Property: assert(event.property != nullptr); - const std::string_view key{*event.property}; - auto iterator{ - this->trie_[current_node].property_children.find(key)}; - if (iterator == this->trie_[current_node].property_children.end()) { - const auto node_index{this->trie_.size()}; - this->trie_.push_back({.position = std::nullopt, - .index_children = {}, - .property_children = {}}); - this->trie_[current_node].property_children.emplace(key, - node_index); - current_node = node_index; - } else { - current_node = iterator->second; - } + descend_into(this->trie_, &TrieNode::property_children, + std::string_view{*event.property}, current_node); break; - } - case JSON::ParseContext::Index: { - auto iterator{ - this->trie_[current_node].index_children.find(event.index)}; - if (iterator == this->trie_[current_node].index_children.end()) { - const auto node_index{this->trie_.size()}; - this->trie_.push_back({.position = std::nullopt, - .index_children = {}, - .property_children = {}}); - this->trie_[current_node].index_children.emplace(event.index, - node_index); - current_node = node_index; - } else { - current_node = iterator->second; - } + case JSON::ParseContext::Index: + descend_into(this->trie_, &TrieNode::index_children, event.index, + current_node); break; - } case JSON::ParseContext::Root: break; } diff --git a/src/core/markdown/include/sourcemeta/core/markdown.h b/src/core/markdown/include/sourcemeta/core/markdown.h index 6901d12eb7..20ad935e03 100644 --- a/src/core/markdown/include/sourcemeta/core/markdown.h +++ b/src/core/markdown/include/sourcemeta/core/markdown.h @@ -9,6 +9,9 @@ #include // NOLINTEND(misc-include-cleaner) +#include // std::size_t +#include // std::uint32_t +#include // std::numeric_limits #include // std::string #include // std::string_view @@ -24,6 +27,13 @@ namespace sourcemeta::core { +/// @ingroup markdown +/// The most input bytes accepted when a caller names no bound of its own, +/// which is what the offsets the parser addresses its input through can reach, +/// so a caller may name a smaller bound but never a larger one. +inline constexpr std::size_t MARKDOWN_MAXIMUM_INPUT_BYTES{ + std::numeric_limits::max()}; + /// @ingroup markdown /// Convert a Markdown string to an HTML fragment using GitHub Flavored /// Markdown (GFM) with all standard extensions enabled (tables, autolinks, @@ -35,7 +45,8 @@ namespace sourcemeta::core { /// output far larger than the input, so the conversion throws `MarkdownError` /// once the rendered link references expand past the larger of 16 times the /// input size and 1,048,576 bytes, or once a table inserts more than 524,288 -/// empty cells. For example: +/// empty cells. A bound smaller than the default refuses a larger input +/// outright. For example: /// /// ```cpp /// #include @@ -46,8 +57,9 @@ namespace sourcemeta::core { /// assert(result == "

Hello world

\n"); /// ``` SOURCEMETA_CORE_MARKDOWN_EXPORT -auto markdown_to_html(const std::string_view input, const bool safe = true) - -> std::string; +auto markdown_to_html(const std::string_view input, const bool safe = true, + std::size_t maximum_input_bytes = + MARKDOWN_MAXIMUM_INPUT_BYTES) -> std::string; } // namespace sourcemeta::core diff --git a/src/core/markdown/markdown.cc b/src/core/markdown/markdown.cc index ae2584cb74..d4ef004d2d 100644 --- a/src/core/markdown/markdown.cc +++ b/src/core/markdown/markdown.cc @@ -9,6 +9,7 @@ #include "postprocess.h" #include "render.h" +#include // std::min #include // std::size_t #include // std::uint32_t #include // std::numeric_limits @@ -60,8 +61,8 @@ auto replace_invalid_characters(const std::string_view input, namespace sourcemeta::core { -auto markdown_to_html(const std::string_view input, const bool safe) - -> std::string { +auto markdown_to_html(const std::string_view input, const bool safe, + const std::size_t maximum_input_bytes) -> std::string { thread_local std::optional state; if (!state.has_value()) { state.emplace(); @@ -80,8 +81,10 @@ auto markdown_to_html(const std::string_view input, const bool safe) source = converter.repaired; } - // The parser addresses the input through 32-bit offsets - if (source.size() > std::numeric_limits::max()) { + // The parser addresses the input through 32-bit offsets, so what a caller + // names only ever tightens the reach those offsets have + if (source.size() > + std::min(maximum_input_bytes, MARKDOWN_MAXIMUM_INPUT_BYTES)) { throw MarkdownError{"The input is larger than the supported size bound"}; } diff --git a/src/core/mcp/headers.cc b/src/core/mcp/headers.cc index 7ac84cef2f..b1e53ed67d 100644 --- a/src/core/mcp/headers.cc +++ b/src/core/mcp/headers.cc @@ -285,7 +285,11 @@ auto mcp_make_error_request_meta( version, identifier, JSONRPC_CODE_INVALID_PARAMS, "Invalid params: clientInfo description must be a string"); } + // Clang settles that no other value arrives, so only GCC and MSVC + // need to be told that the end of this function is never reached +#if !defined(__clang__) std::unreachable(); +#endif } auto mcp_request_name_from_body(const sourcemeta::core::JSON &envelope) diff --git a/src/core/mcp/include/sourcemeta/core/mcp_logging.h b/src/core/mcp/include/sourcemeta/core/mcp_logging.h index 89b77f9efe..707dc2b637 100644 --- a/src/core/mcp/include/sourcemeta/core/mcp_logging.h +++ b/src/core/mcp/include/sourcemeta/core/mcp_logging.h @@ -75,7 +75,11 @@ constexpr auto mcp_log_level_string(const MCPLogLevel level) noexcept case MCPLogLevel::Emergency: return "emergency"; } + // Clang settles that no other value arrives, so only GCC and MSVC + // need to be told that the end of this function is never reached +#if !defined(__clang__) std::unreachable(); +#endif } /// @ingroup mcp diff --git a/src/core/mcp/include/sourcemeta/core/mcp_protocol.h b/src/core/mcp/include/sourcemeta/core/mcp_protocol.h index fc628e644c..def5acdd94 100644 --- a/src/core/mcp/include/sourcemeta/core/mcp_protocol.h +++ b/src/core/mcp/include/sourcemeta/core/mcp_protocol.h @@ -397,7 +397,11 @@ mcp_protocol_version_string(const MCPProtocolVersion version) noexcept case MCPProtocolVersion::V_2026_07_28: return MCP_PROTOCOL_VERSION_2026_07_28; } + // Clang settles that no other value arrives, so only GCC and MSVC + // need to be told that the end of this function is never reached +#if !defined(__clang__) std::unreachable(); +#endif } /// @ingroup mcp @@ -903,7 +907,11 @@ constexpr auto mcp_is_request_method(const MCPProtocolVersion version, method == MCP_METHOD_SUBSCRIPTIONS_LISTEN || method == MCP_METHOD_TOOLS_CALL || method == MCP_METHOD_TOOLS_LIST; } + // Clang settles that no other value arrives, so only GCC and MSVC + // need to be told that the end of this function is never reached +#if !defined(__clang__) std::unreachable(); +#endif } /// @ingroup mcp @@ -945,7 +953,11 @@ mcp_is_server_request_method(const MCPProtocolVersion version, case MCPProtocolVersion::V_2026_07_28: return false; } + // Clang settles that no other value arrives, so only GCC and MSVC + // need to be told that the end of this function is never reached +#if !defined(__clang__) std::unreachable(); +#endif } /// @ingroup mcp @@ -1003,7 +1015,11 @@ mcp_is_notification_method(const MCPProtocolVersion version, method == MCP_METHOD_NOTIFICATIONS_SUBSCRIPTIONS_ACKNOWLEDGED || method == MCP_METHOD_NOTIFICATIONS_TOOLS_LIST_CHANGED; } + // Clang settles that no other value arrives, so only GCC and MSVC + // need to be told that the end of this function is never reached +#if !defined(__clang__) std::unreachable(); +#endif } /// @ingroup mcp @@ -1037,7 +1053,11 @@ mcp_is_input_request_method(const MCPProtocolVersion version, method == MCP_METHOD_ROOTS_LIST || method == MCP_METHOD_SAMPLING_CREATE_MESSAGE; } + // Clang settles that no other value arrives, so only GCC and MSVC + // need to be told that the end of this function is never reached +#if !defined(__clang__) std::unreachable(); +#endif } /// @ingroup mcp @@ -1161,7 +1181,11 @@ constexpr auto mcp_supports_method(const MCPProtocolVersion version, method == MCP_METHOD_SUBSCRIPTIONS_LISTEN || method == MCP_METHOD_TOOLS_CALL || method == MCP_METHOD_TOOLS_LIST; } + // Clang settles that no other value arrives, so only GCC and MSVC + // need to be told that the end of this function is never reached +#if !defined(__clang__) std::unreachable(); +#endif } /// @ingroup mcp @@ -1245,7 +1269,11 @@ mcp_is_client_notification_method(MCPProtocolVersion version, case MCPProtocolVersion::V_2026_07_28: return method == "notifications/cancelled"; } + // Clang settles that no other value arrives, so only GCC and MSVC + // need to be told that the end of this function is never reached +#if !defined(__clang__) std::unreachable(); +#endif } /// @ingroup mcp @@ -1299,7 +1327,11 @@ mcp_is_server_notification_method(MCPProtocolVersion version, method == "notifications/tools/list_changed" || method == "notifications/message"; } + // Clang settles that no other value arrives, so only GCC and MSVC + // need to be told that the end of this function is never reached +#if !defined(__clang__) std::unreachable(); +#endif } } // namespace sourcemeta::core diff --git a/src/core/mcp/include/sourcemeta/core/mcp_results.h b/src/core/mcp/include/sourcemeta/core/mcp_results.h index d32daf5305..114ceb639d 100644 --- a/src/core/mcp/include/sourcemeta/core/mcp_results.h +++ b/src/core/mcp/include/sourcemeta/core/mcp_results.h @@ -53,7 +53,11 @@ constexpr auto mcp_cache_scope_string(const MCPCacheScope scope) noexcept case MCPCacheScope::Private: return "private"; } + // Clang settles that no other value arrives, so only GCC and MSVC + // need to be told that the end of this function is never reached +#if !defined(__clang__) std::unreachable(); +#endif } /// @ingroup mcp diff --git a/src/core/oidc/oidc_authentication.cc b/src/core/oidc/oidc_authentication.cc index 93ff58afd3..9519ba1a60 100644 --- a/src/core/oidc/oidc_authentication.cc +++ b/src/core/oidc/oidc_authentication.cc @@ -5,6 +5,7 @@ #include #include // std::array +#include // assert #include // std::size_t #include // std::less #include // std::optional, std::nullopt @@ -81,10 +82,10 @@ auto drop_offline_access(std::string_view &scope, std::string &storage) if (in_arena) { offset = static_cast(scope.data() - arena); } else { - if (storage.capacity() - storage.size() < scope.size()) { - return; - } - + // The parser reserved the whole query up front and a borrowed value took + // none of that room, while everything decoded into the arena is at most as + // long as the query it came from, so what is appended here always fits + assert(storage.capacity() - storage.size() >= scope.size()); offset = storage.size(); storage.resize(storage.size() + scope.size()); } diff --git a/src/core/oidc/oidc_id_token.cc b/src/core/oidc/oidc_id_token.cc index 1640202e0a..b9988ff62e 100644 --- a/src/core/oidc/oidc_id_token.cc +++ b/src/core/oidc/oidc_id_token.cc @@ -10,7 +10,6 @@ #include // std::int64_t #include // std::optional, std::nullopt #include // std::span -#include // std::out_of_range #include // std::string #include // std::string_view @@ -170,16 +169,12 @@ auto oidc_id_token_checks(const JWT &token, const std::string_view issuer, std::optional authentication_time; const auto *auth_time{payload.try_at("auth_time"sv, HASH_AUTH_TIME)}; if (auth_time != nullptr && auth_time->is_number()) { - // A NumericDate may be non-integer (RFC 7519 Section 2), and a value beyond - // the range of a double cannot stand for a usable timestamp, so it is - // treated as absent rather than aborting, matching how the JWT layer reads - // the registered time claims - try { - authentication_time = from_unix_timestamp( - std::chrono::duration{auth_time->as_real()}); - } catch (const std::out_of_range &) { - authentication_time = std::nullopt; - } + // A NumericDate may be non-integer (RFC 7519 Section 2), and a value the + // clock cannot stand for comes back as no time at all, which reads as an + // absent claim rather than aborting, matching how the JWT layer reads the + // registered time claims + authentication_time = from_unix_timestamp( + std::chrono::duration{auth_time->as_real()}); } if (options.maximum_authentication_age.has_value()) { diff --git a/src/core/punycode/include/sourcemeta/core/punycode.h b/src/core/punycode/include/sourcemeta/core/punycode.h index b996e60e81..60b7597075 100644 --- a/src/core/punycode/include/sourcemeta/core/punycode.h +++ b/src/core/punycode/include/sourcemeta/core/punycode.h @@ -9,7 +9,10 @@ #include // NOLINTEND(misc-include-cleaner) +#include // std::size_t +#include // std::uint32_t #include // std::istream +#include // std::numeric_limits #include // std::ostream #include // std::string, std::u32string #include // std::string_view, std::u32string_view @@ -27,6 +30,21 @@ namespace sourcemeta::core { // See https://www.rfc-editor.org/rfc/rfc3492 +/// @ingroup punycode +/// The most code points an encode accepts when a caller names no bound of its +/// own. RFC 3492 Section 6.4 offers bounding the input as one way of keeping +/// the position counters from wrapping, which is the ceiling this stands at, so +/// a caller may name a smaller bound but never a larger one. +inline constexpr std::size_t PUNYCODE_MAXIMUM_CODE_POINTS{ + std::numeric_limits::max()}; + +/// @ingroup punycode +/// The most bytes a decode accepts when a caller names no bound of its own, +/// which stands at the same ceiling the encode bound does and may likewise only +/// be tightened. +inline constexpr std::size_t PUNYCODE_MAXIMUM_ENCODED_BYTES{ + std::numeric_limits::max()}; + /// @ingroup punycode /// Encode Unicode code points (UTF-32) to Punycode. For example: /// @@ -45,8 +63,11 @@ namespace sourcemeta::core { /// because the C++ standard library does not define the required locale facets /// (`std::ctype`) for `std::basic_istream` and /// `std::basic_ostream` to function properly. +/// A bound smaller than the default refuses a larger input outright. SOURCEMETA_CORE_PUNYCODE_EXPORT -auto utf32_to_punycode(std::u32string_view input) -> std::string; +auto utf32_to_punycode(std::u32string_view input, + std::size_t maximum_code_points = + PUNYCODE_MAXIMUM_CODE_POINTS) -> std::string; /// @ingroup punycode /// Encode UTF-8 to Punycode using streams. For example: @@ -61,8 +82,11 @@ auto utf32_to_punycode(std::u32string_view input) -> std::string; /// sourcemeta::core::utf8_to_punycode(input, output); /// assert(output.str() == "Mnchen-3ya"); /// ``` +/// A bound smaller than the default refuses a larger input outright. SOURCEMETA_CORE_PUNYCODE_EXPORT -auto utf8_to_punycode(std::istream &input, std::ostream &output) -> void; +auto utf8_to_punycode( + std::istream &input, std::ostream &output, + std::size_t maximum_code_points = PUNYCODE_MAXIMUM_CODE_POINTS) -> void; /// @ingroup punycode /// Encode UTF-8 to Punycode. For example: @@ -74,8 +98,11 @@ auto utf8_to_punycode(std::istream &input, std::ostream &output) -> void; /// assert(sourcemeta::core::utf8_to_punycode("M\xC3\xBCnchen") == /// "Mnchen-3ya"); /// ``` +/// A bound smaller than the default refuses a larger input outright. SOURCEMETA_CORE_PUNYCODE_EXPORT -auto utf8_to_punycode(const std::string_view input) -> std::string; +auto utf8_to_punycode(const std::string_view input, + std::size_t maximum_code_points = + PUNYCODE_MAXIMUM_CODE_POINTS) -> std::string; /// @ingroup punycode /// Decode Punycode to Unicode code points (UTF-32). For example: @@ -96,8 +123,11 @@ auto utf8_to_punycode(const std::string_view input) -> std::string; /// because the C++ standard library does not define the required locale facets /// (`std::ctype`) for `std::basic_istream` and /// `std::basic_ostream` to function properly. +/// A bound smaller than the default refuses a larger input outright. SOURCEMETA_CORE_PUNYCODE_EXPORT -auto punycode_to_utf32(const std::string_view input) -> std::u32string; +auto punycode_to_utf32(const std::string_view input, + std::size_t maximum_encoded_bytes = + PUNYCODE_MAXIMUM_ENCODED_BYTES) -> std::u32string; /// @ingroup punycode /// Decode Punycode to UTF-8 using streams. For example: @@ -112,8 +142,11 @@ auto punycode_to_utf32(const std::string_view input) -> std::u32string; /// sourcemeta::core::punycode_to_utf8(input, output); /// assert(output.str() == "M\xC3\xBCnchen"); /// ``` +/// A bound smaller than the default refuses a larger input outright. SOURCEMETA_CORE_PUNYCODE_EXPORT -auto punycode_to_utf8(std::istream &input, std::ostream &output) -> void; +auto punycode_to_utf8( + std::istream &input, std::ostream &output, + std::size_t maximum_encoded_bytes = PUNYCODE_MAXIMUM_ENCODED_BYTES) -> void; /// @ingroup punycode /// Decode Punycode to UTF-8. For example: @@ -125,8 +158,11 @@ auto punycode_to_utf8(std::istream &input, std::ostream &output) -> void; /// assert(sourcemeta::core::punycode_to_utf8("Mnchen-3ya") == /// "M\xC3\xBCnchen"); /// ``` +/// A bound smaller than the default refuses a larger input outright. SOURCEMETA_CORE_PUNYCODE_EXPORT -auto punycode_to_utf8(const std::string_view input) -> std::string; +auto punycode_to_utf8(const std::string_view input, + std::size_t maximum_encoded_bytes = + PUNYCODE_MAXIMUM_ENCODED_BYTES) -> std::string; } // namespace sourcemeta::core diff --git a/src/core/punycode/punycode.cc b/src/core/punycode/punycode.cc index 63059eb4dd..39e47d8867 100644 --- a/src/core/punycode/punycode.cc +++ b/src/core/punycode/punycode.cc @@ -2,8 +2,9 @@ #include #include -#include // std::ranges::sort +#include // std::min, std::ranges::sort #include // assert +#include // std::size_t #include // std::uint32_t, std::uint64_t #include // std::numeric_limits #include // std::vector @@ -82,11 +83,14 @@ static constexpr auto is_basic(const char32_t code_point) -> bool { } static auto punycode_encode(const std::u32string_view codepoints, - std::string &output) -> void { + std::string &output, + const std::size_t maximum_code_points) -> void { // RFC 3492 Section 6.4 offers bounding the input as one way of keeping the // counters from wrapping, alongside the detection the arithmetic below - // carries out, so this bound is a choice made here rather than a requirement - if (codepoints.size() > std::numeric_limits::max()) { + // carries out. What a caller names only ever tightens the ceiling the + // counters stand at, as a wider input than that has nowhere to be addressed + if (codepoints.size() > + std::min(maximum_code_points, PUNYCODE_MAXIMUM_CODE_POINTS)) { throw PunycodeError("Input is too large"); } @@ -181,12 +185,14 @@ static auto punycode_encode(const std::u32string_view codepoints, } static auto punycode_decode(const std::string_view encoded, - std::u32string &decoded) -> void { + std::u32string &decoded, + const std::size_t maximum_encoded_bytes) -> void { // RFC 3492 Section 6.4 offers bounding the input as one way of keeping the // position counter from wrapping, alongside the detection the arithmetic - // below carries out, so this bound is a choice made here rather than a - // requirement - if (encoded.size() > std::numeric_limits::max()) { + // below carries out. What a caller names only ever tightens the ceiling the + // counter stands at, as a wider input than that has nowhere to be addressed + if (encoded.size() > + std::min(maximum_encoded_bytes, PUNYCODE_MAXIMUM_ENCODED_BYTES)) { throw PunycodeError("Input is too large"); } @@ -283,30 +289,35 @@ static auto punycode_decode(const std::string_view encoded, } } -auto utf32_to_punycode(std::u32string_view input) -> std::string { +auto utf32_to_punycode(std::u32string_view input, + const std::size_t maximum_code_points) -> std::string { std::string result; - punycode_encode(input, result); + punycode_encode(input, result, maximum_code_points); return result; } -auto punycode_to_utf32(const std::string_view input) -> std::u32string { +auto punycode_to_utf32(const std::string_view input, + const std::size_t maximum_encoded_bytes) + -> std::u32string { std::u32string result; - punycode_decode(input, result); + punycode_decode(input, result, maximum_encoded_bytes); return result; } -auto utf8_to_punycode(std::istream &input, std::ostream &output) -> void { +auto utf8_to_punycode(std::istream &input, std::ostream &output, + const std::size_t maximum_code_points) -> void { const auto codepoints = utf8_to_utf32(input); if (!codepoints.has_value()) { throw PunycodeError("Invalid UTF-8 input"); } std::string result; - punycode_encode(codepoints.value(), result); + punycode_encode(codepoints.value(), result, maximum_code_points); output << result; } -auto punycode_to_utf8(std::istream &input, std::ostream &output) -> void { +auto punycode_to_utf8(std::istream &input, std::ostream &output, + const std::size_t maximum_encoded_bytes) -> void { std::string encoded; char character{0}; while (input.get(character)) { @@ -314,26 +325,28 @@ auto punycode_to_utf8(std::istream &input, std::ostream &output) -> void { } std::u32string decoded; - punycode_decode(encoded, decoded); + punycode_decode(encoded, decoded, maximum_encoded_bytes); for (const auto code_point : decoded) { codepoint_to_utf8(code_point, output); } } -auto utf8_to_punycode(const std::string_view input) -> std::string { +auto utf8_to_punycode(const std::string_view input, + const std::size_t maximum_code_points) -> std::string { const auto codepoints = utf8_to_utf32(input); if (!codepoints.has_value()) { throw PunycodeError("Invalid UTF-8 input"); } std::string result; - punycode_encode(codepoints.value(), result); + punycode_encode(codepoints.value(), result, maximum_code_points); return result; } -auto punycode_to_utf8(const std::string_view input) -> std::string { +auto punycode_to_utf8(const std::string_view input, + const std::size_t maximum_encoded_bytes) -> std::string { std::u32string decoded; - punycode_decode(input, decoded); + punycode_decode(input, decoded, maximum_encoded_bytes); std::string output; output.reserve(decoded.size()); for (const auto code_point : decoded) { diff --git a/src/core/regex/permissive.h b/src/core/regex/permissive.h index fe3515357b..ecadb823fb 100644 --- a/src/core/regex/permissive.h +++ b/src/core/regex/permissive.h @@ -10,7 +10,7 @@ #include // std::optional #include // std::string #include // std::string_view -#include // std::move, std::pair, std::unreachable +#include // std::move, std::pair namespace sourcemeta::core { @@ -111,30 +111,21 @@ constexpr auto NON_DIGIT_CLASS{(~DIGIT_CLASS).reset(0)}; constexpr auto NON_WORD_CLASS{(~WORD_CLASS).reset(0)}; constexpr auto NON_SPACE_CLASS{(~SPACE_CLASS).reset(0)}; +// Each shorthand names a class in its lower case and the complement of that +// class in its upper case, so which class is meant and whether it is negated +// are read apart rather than spelled out once per pairing. The caller matches +// the shorthand against the set of them before asking, which is what leaves +// the last pairing to answer for whatever is not one of the other two inline auto set_shorthand_class(std::bitset<128> &characters, const char shorthand) -> void { - switch (shorthand) { - case 'd': - characters |= DIGIT_CLASS; - return; - case 'D': - characters |= NON_DIGIT_CLASS; - return; - case 'w': - characters |= WORD_CLASS; - return; - case 'W': - characters |= NON_WORD_CLASS; - return; - case 's': - characters |= SPACE_CLASS; - return; - case 'S': - characters |= NON_SPACE_CLASS; - return; - default: - // The caller matches the shorthand against the set of them before asking - std::unreachable(); + const auto lowered{static_cast(shorthand | 0x20)}; + const auto negated{shorthand != lowered}; + if (lowered == 'd') { + characters |= negated ? NON_DIGIT_CLASS : DIGIT_CLASS; + } else if (lowered == 'w') { + characters |= negated ? NON_WORD_CLASS : WORD_CLASS; + } else { + characters |= negated ? NON_SPACE_CLASS : SPACE_CLASS; } } diff --git a/src/core/regex/regex.cc b/src/core/regex/regex.cc index 5b8835ae09..9f604e8f43 100644 --- a/src/core/regex/regex.cc +++ b/src/core/regex/regex.cc @@ -38,6 +38,32 @@ auto compile_pcre2(const std::string &pattern, const std::uint32_t options) return RegexTypePCRE2{std::shared_ptr(pcre2_regex)}; } +auto matches_range(const RegexTypeRange &range, const std::string_view value) + -> bool { + // ECMA-262 "." matches a single code point, not a single byte, so the + // bounds are compared against the number of code points + return utf8_codepoint_within(value, range.first, range.second); +} + +auto matches_pcre2(const RegexTypePCRE2 ®ex, const std::string_view value) + -> bool { + auto *pcre2_code_ptr{static_cast(regex.code.get())}; + // Allocated once per thread and reused across calls to avoid + // allocating every time. It is intentionally never freed, as + // releasing it on thread exit would make matching unsafe during the + // destruction of other objects with thread storage duration + thread_local pcre2_match_data *match_data{ + pcre2_match_data_create(1, nullptr)}; + // A null context selects the built-in default limits, which guarantee + // that pathological patterns applied to adversarial inputs terminate. + // Note that exhausting any matching resource, like the machine stack + // of the just-in-time fast path on long values, counts as a failure + const int match_result{ + pcre2_match(pcre2_code_ptr, reinterpret_cast(value.data()), + value.size(), 0, PCRE2_NO_UTF_CHECK, match_data, nullptr)}; + return match_result >= 0; +} + } // namespace auto to_regex(const std::string_view pattern, const RegexDialect dialect, @@ -131,30 +157,10 @@ auto matches(const Regex ®ex, const std::string_view value) -> bool { return value.starts_with(*std::get_if(®ex)); case RegexIndex::NonEmpty: return !value.empty(); - case RegexIndex::Range: { - // ECMA-262 "." matches a single code point, not a single byte, so the - // bounds are compared against the number of code points - const RegexTypeRange *range{std::get_if(®ex)}; - return utf8_codepoint_within(value, range->first, range->second); - } - case RegexIndex::PCRE2: { - const RegexTypePCRE2 *pcre2_regex{std::get_if(®ex)}; - auto *pcre2_code_ptr{static_cast(pcre2_regex->code.get())}; - // Allocated once per thread and reused across calls to avoid - // allocating every time. It is intentionally never freed, as - // releasing it on thread exit would make matching unsafe during the - // destruction of other objects with thread storage duration - thread_local pcre2_match_data *match_data{ - pcre2_match_data_create(1, nullptr)}; - // A null context selects the built-in default limits, which guarantee - // that pathological patterns applied to adversarial inputs terminate. - // Note that exhausting any matching resource, like the machine stack - // of the just-in-time fast path on long values, counts as a failure - const int match_result{pcre2_match( - pcre2_code_ptr, reinterpret_cast(value.data()), - value.size(), 0, PCRE2_NO_UTF_CHECK, match_data, nullptr)}; - return match_result >= 0; - } + case RegexIndex::Range: + return matches_range(*std::get_if(®ex), value); + case RegexIndex::PCRE2: + return matches_pcre2(*std::get_if(®ex), value); case RegexIndex::Noop: return true; } diff --git a/src/core/semver/semver.cc b/src/core/semver/semver.cc index 373e1ddc34..1dc3cb019c 100644 --- a/src/core/semver/semver.cc +++ b/src/core/semver/semver.cc @@ -367,15 +367,15 @@ auto parse_semver(const std::string_view input, std::uint64_t &major, } } - if (position != input.size()) { - if constexpr (ShouldThrow) { - throw sourcemeta::core::SemVerParseError(position + 1); - } else { - return false; - } + if (position == input.size()) { + return true; } - return true; + if constexpr (ShouldThrow) { + throw sourcemeta::core::SemVerParseError(position + 1); + } else { + return false; + } } } // namespace diff --git a/src/core/terminal/terminal.cc b/src/core/terminal/terminal.cc index 5d633b5bc0..3d55b6efaf 100644 --- a/src/core/terminal/terminal.cc +++ b/src/core/terminal/terminal.cc @@ -70,9 +70,13 @@ auto terminal_color_enabled(TerminalStream stream) noexcept -> bool { return true; case TerminalColorPolicy::Disabled: return false; - default: - std::unreachable(); } + + // Clang settles that no other value arrives, so only GCC and MSVC + // need to be told that the end of this function is never reached +#if !defined(__clang__) + std::unreachable(); +#endif } auto terminal_sgr_reset() noexcept -> std::string_view { diff --git a/src/core/time/asctime.cc b/src/core/time/asctime.cc index acc0452c81..ce1eb18c5a 100644 --- a/src/core/time/asctime.cc +++ b/src/core/time/asctime.cc @@ -35,8 +35,8 @@ auto to_asctime(const std::chrono::system_clock::time_point time) return stream.str(); } -auto from_asctime(const std::string_view value) noexcept - -> std::optional try { +auto from_asctime(const std::string_view value) + -> std::optional { if (value.size() != 24) { return std::nullopt; } @@ -79,8 +79,6 @@ auto from_asctime(const std::string_view value) noexcept return std::nullopt; } return broken_down_time_to_time_point(parts); -} catch (...) { - return std::nullopt; } } // namespace sourcemeta::core diff --git a/src/core/time/imf_fixdate.cc b/src/core/time/imf_fixdate.cc index 73470b0a4f..0bffefc8ba 100644 --- a/src/core/time/imf_fixdate.cc +++ b/src/core/time/imf_fixdate.cc @@ -33,8 +33,8 @@ auto to_imf_fixdate(const std::chrono::system_clock::time_point time) return stream.str(); } -auto from_imf_fixdate(const std::string_view value) noexcept - -> std::optional try { +auto from_imf_fixdate(const std::string_view value) + -> std::optional { if (value.size() != 29) { return std::nullopt; } @@ -73,8 +73,6 @@ auto from_imf_fixdate(const std::string_view value) noexcept return std::nullopt; } return broken_down_time_to_time_point(parts); -} catch (...) { - return std::nullopt; } } // namespace sourcemeta::core diff --git a/src/core/time/include/sourcemeta/core/time.h b/src/core/time/include/sourcemeta/core/time.h index 6ca16831e5..e875f951e0 100644 --- a/src/core/time/include/sourcemeta/core/time.h +++ b/src/core/time/include/sourcemeta/core/time.h @@ -57,7 +57,7 @@ auto to_imf_fixdate(const std::chrono::system_clock::time_point time) /// assert(point.has_value()); /// ``` SOURCEMETA_CORE_TIME_EXPORT -auto from_imf_fixdate(const std::string_view value) noexcept +auto from_imf_fixdate(const std::string_view value) -> std::optional; /// @ingroup time @@ -93,7 +93,7 @@ auto to_rfc850_date(const std::chrono::system_clock::time_point time) /// assert(point.has_value()); /// ``` SOURCEMETA_CORE_TIME_EXPORT -auto from_rfc850_date(const std::string_view value) noexcept +auto from_rfc850_date(const std::string_view value) -> std::optional; /// @ingroup time @@ -130,7 +130,7 @@ auto to_asctime(const std::chrono::system_clock::time_point time) /// assert(point.has_value()); /// ``` SOURCEMETA_CORE_TIME_EXPORT -auto from_asctime(const std::string_view value) noexcept +auto from_asctime(const std::string_view value) -> std::optional; /// @ingroup time diff --git a/src/core/time/rfc850_date.cc b/src/core/time/rfc850_date.cc index bcf4e12cd5..6ce10e0f5d 100644 --- a/src/core/time/rfc850_date.cc +++ b/src/core/time/rfc850_date.cc @@ -41,8 +41,8 @@ auto to_rfc850_date(const std::chrono::system_clock::time_point time) return stream.str(); } -auto from_rfc850_date(const std::string_view value) noexcept - -> std::optional try { +auto from_rfc850_date(const std::string_view value) + -> std::optional { const auto comma_position{value.find(',')}; if (comma_position == std::string_view::npos) { return std::nullopt; @@ -99,8 +99,6 @@ auto from_rfc850_date(const std::string_view value) noexcept return std::nullopt; } return broken_down_time_to_time_point(parts); -} catch (...) { - return std::nullopt; } } // namespace sourcemeta::core diff --git a/src/core/uri/parse.cc b/src/core/uri/parse.cc index 2a5a2c912b..f98fb56292 100644 --- a/src/core/uri/parse.cc +++ b/src/core/uri/parse.cc @@ -81,17 +81,43 @@ auto accept_iri_extension(const std::string_view input, } } +// Every reader below turns down what it was handed in the same two ways, +// depending on whether it was asked to check the input or to take it apart, so +// what a refusal answers with is spelled out once here +template +auto parse_refused() + -> std::conditional_t> { + if constexpr (CheckOnly) { + return false; + } else { + return std::nullopt; + } +} + +// A reader that has found what it was after answers with the text it took or +// with the bare fact that it was there, and steps past the delimiter either +// way, so what being taken looks like is spelled out once here +template +auto parse_taken(const std::string_view input, + const std::string_view::size_type start, + std::string_view::size_type &position) + -> std::conditional_t> { + const auto taken{input.substr(start, position - start)}; + position += 1; + if constexpr (CheckOnly) { + return true; + } else { + return std::string{taken}; + } +} + template auto parse_scheme(const std::string_view input, std::string_view::size_type &position) -> std::conditional_t> { if (position >= input.size() || !std::isalpha(static_cast(input[position]))) { - if constexpr (CheckOnly) { - return false; - } else { - return std::nullopt; - } + return parse_refused(); } const auto start = position; @@ -102,22 +128,11 @@ auto parse_scheme(const std::string_view input, } if (position < input.size() && input[position] == URI_COLON) { - if constexpr (CheckOnly) { - position += 1; - return true; - } else { - std::string scheme{input.substr(start, position - start)}; - position += 1; - return scheme; - } + return parse_taken(input, start, position); } position = start; - if constexpr (CheckOnly) { - return false; - } else { - return std::nullopt; - } + return parse_refused(); } template @@ -126,11 +141,7 @@ auto parse_port(const std::string_view input, -> std::conditional_t> { if (position >= input.size() || !std::isdigit(static_cast(input[position]))) { - if constexpr (CheckOnly) { - return false; - } else { - return std::nullopt; - } + return parse_refused(); } const auto start = position; @@ -289,12 +300,7 @@ auto parse_host(const std::string_view input, if (input[position] == URI_OPEN_BRACKET) { ip_literal = true; - if constexpr (CheckOnly) { - parse_ipv6(input, position); - return; - } else { - return parse_ipv6(input, position); - } + return parse_ipv6(input, position); } const auto start = position; @@ -364,11 +370,7 @@ auto parse_userinfo(const std::string_view input, } position = start; - if constexpr (CheckOnly) { - return false; - } else { - return std::nullopt; - } + return parse_refused(); } template @@ -376,20 +378,12 @@ auto parse_path(const std::string_view input, std::string_view::size_type &position) -> std::conditional_t> { if (position >= input.size()) { - if constexpr (CheckOnly) { - return false; - } else { - return std::nullopt; - } + return parse_refused(); } const auto first_char = input[position]; if (first_char == URI_QUESTION || first_char == URI_HASH) { - if constexpr (CheckOnly) { - return false; - } else { - return std::nullopt; - } + return parse_refused(); } const auto start = position; @@ -422,11 +416,7 @@ auto parse_query(const std::string_view input, std::string_view::size_type &position) -> std::conditional_t> { if (position >= input.size() || input[position] != URI_QUESTION) { - if constexpr (CheckOnly) { - return false; - } else { - return std::nullopt; - } + return parse_refused(); } position += 1; @@ -462,11 +452,7 @@ auto parse_fragment(const std::string_view input, std::string_view::size_type &position) -> std::conditional_t> { if (position >= input.size() || input[position] != URI_HASH) { - if constexpr (CheckOnly) { - return false; - } else { - return std::nullopt; - } + return parse_refused(); } position += 1; diff --git a/src/core/uri/setters.cc b/src/core/uri/setters.cc index 7f77e4097f..c04348a0ea 100644 --- a/src/core/uri/setters.cc +++ b/src/core/uri/setters.cc @@ -81,6 +81,18 @@ auto validate_raw_path(const std::string_view path) -> void { namespace sourcemeta::core { +namespace { + +// A path hangs off a root when what it is set on names one, which a scheme +// that addresses hierarchically does, and which an authority does on its own +auto needs_root_slash(const URI &uri) -> bool { + const auto is_hierarchical{!uri.is_urn() && !uri.is_tag() && + !uri.is_mailto() && uri.scheme().has_value()}; + return is_hierarchical || uri.host().has_value(); +} + +} // namespace + auto URI::path(const std::string &path) -> URI & { if (path.empty()) { this->path_ = std::nullopt; @@ -93,12 +105,7 @@ auto URI::path(const std::string &path) -> URI & { validate_raw_path(path); - // Determine if this URI needs a leading slash - // (URIs with scheme/authority need leading slash, except URNs/tags/mailto) - const auto needs_leading_slash = - (!this->is_urn() && !this->is_tag() && !this->is_mailto() && - this->scheme_.has_value()) || - this->host_.has_value(); + const auto needs_leading_slash{needs_root_slash(*this)}; this->path_ = apply_leading_slash_transform(std::optional{path}, needs_leading_slash); @@ -117,12 +124,7 @@ auto URI::path(std::string &&path) -> URI & { validate_raw_path(path); - // Determine if this URI needs a leading slash - // (URIs with scheme/authority need leading slash, except URNs/tags/mailto) - const auto needs_leading_slash = - (!this->is_urn() && !this->is_tag() && !this->is_mailto() && - this->scheme_.has_value()) || - this->host_.has_value(); + const auto needs_leading_slash{needs_root_slash(*this)}; this->path_ = apply_leading_slash_transform( std::optional{std::move(path)}, needs_leading_slash); @@ -224,10 +226,7 @@ auto URI::append_path(std::string_view path) -> URI & { "Cannot append a URI as a path that contains an invalid character"}; } - const auto needs_leading_slash = - (!this->is_urn() && !this->is_tag() && !this->is_mailto() && - this->scheme_.has_value()) || - this->host_.has_value(); + const auto needs_leading_slash{needs_root_slash(*this)}; merge_reference_path(this->path_, needs_leading_slash, path); return *this; } @@ -256,10 +255,7 @@ auto URI::append_path(const URI &reference) -> URI & { return *this; } - const auto needs_leading_slash = - (!this->is_urn() && !this->is_tag() && !this->is_mailto() && - this->scheme_.has_value()) || - this->host_.has_value(); + const auto needs_leading_slash{needs_root_slash(*this)}; merge_reference_path(this->path_, needs_leading_slash, reference.path_.value()); return *this; @@ -272,10 +268,7 @@ auto URI::append_path(URI &&reference) -> URI & { return *this; } - const auto needs_leading_slash = - (!this->is_urn() && !this->is_tag() && !this->is_mailto() && - this->scheme_.has_value()) || - this->host_.has_value(); + const auto needs_leading_slash{needs_root_slash(*this)}; std::string reference_path{std::move(reference.path_.value())}; reference.path_ = std::nullopt; merge_reference_path(this->path_, needs_leading_slash, reference_path); diff --git a/src/core/uritemplate/helpers.h b/src/core/uritemplate/helpers.h index ee3096c2f1..152261d284 100644 --- a/src/core/uritemplate/helpers.h +++ b/src/core/uritemplate/helpers.h @@ -348,9 +348,10 @@ auto parse_expression(const std::string_view input) -> std::conditional_t, URITemplateParseResult> { if constexpr (std::is_same_v) { - if (input.empty() || input[0] == '{') { - return std::nullopt; - } + // A literal is read last of all, after everything that opens an expression + // has either taken what stands here or refused the template outright, and + // the walk that asks only ever hands over what it has left to read + assert(!input.empty() && input[0] != '{'); std::size_t position = 0; while (position < input.size()) { @@ -405,10 +406,11 @@ auto parse_expression(const std::string_view input) if (input.size() < 2) { throw URITemplateParseError(1); } - // Not a simple variable if it has an operator - if (is_operator(input[1])) { - return std::nullopt; - } + + // RFC 6570 Section 2.2 has an operator name which expansion stands here, + // and every one of those is read before this is, so what reaches here + // never carries one + assert(!is_operator(input[1])); var_start = 1; } diff --git a/src/core/uritemplate/uritemplate_router.cc b/src/core/uritemplate/uritemplate_router.cc index 72ca9b81b1..b528ecd121 100644 --- a/src/core/uritemplate/uritemplate_router.cc +++ b/src/core/uritemplate/uritemplate_router.cc @@ -729,7 +729,11 @@ auto URITemplateRouter::match(const std::string_view path, ++position; } - if ((current != nullptr) && current->identifier == 0 && current->variable && + // Every way out of the walk above either answers on its own or settles on a + // node, which is what stepping past that node's own literals relies on too + assert(current != nullptr); + + if (current->identifier == 0 && current->variable && current->variable->type == NodeType::OptionalExpansion) { assert(variable_index <= std::numeric_limits::max()); @@ -739,11 +743,8 @@ auto URITemplateRouter::match(const std::string_view path, current->variable->context); } - return (current != nullptr) - ? finalize_match(this->otherwise_, current->identifier, - current->context) - : finalize_match(this->otherwise_, this->root_.identifier, - this->root_.context); + return finalize_match(this->otherwise_, current->identifier, + current->context); } } // namespace sourcemeta::core diff --git a/src/core/uritemplate/uritemplate_router_view.cc b/src/core/uritemplate/uritemplate_router_view.cc index a06320ee71..3ba4254f88 100644 --- a/src/core/uritemplate/uritemplate_router_view.cc +++ b/src/core/uritemplate/uritemplate_router_view.cc @@ -878,13 +878,11 @@ auto URITemplateRouterView::arguments( } switch (type_tag) { - case ARGUMENT_TYPE_STRING: { - const std::string_view string_value{ - reinterpret_cast(this->data_ + cursor), - value_length}; - callback(name, string_value); + case ARGUMENT_TYPE_STRING: + callback(name, std::string_view{reinterpret_cast( + this->data_ + cursor), + value_length}); break; - } case ARGUMENT_TYPE_INTEGER: { if (value_length != 8) { diff --git a/src/lang/io/io.cc b/src/lang/io/io.cc index bde3c58c35..2bdca437e2 100644 --- a/src/lang/io/io.cc +++ b/src/lang/io/io.cc @@ -146,14 +146,18 @@ auto weakly_canonical(const std::filesystem::path &path) const auto absolute_path{ path.is_absolute() ? path : std::filesystem::absolute(path)}; - // On Linux, FIFO files (like /dev/fd/XX due to process substitution) - // cannot be made canonical - // See https://github.com/sourcemeta/jsonschema/issues/252 - if (std::filesystem::is_fifo(absolute_path)) { - return normalize(absolute_path); - } - try { + // On Linux, FIFO files (like /dev/fd/XX due to process substitution) + // cannot be made canonical + // See https://github.com/sourcemeta/jsonschema/issues/252 + // + // Asking what the path is can fail for the same reasons canonicalising it + // can, so it is asked from inside here rather than ahead of it, where a + // failure would travel on without the naming below ever seeing it + if (std::filesystem::is_fifo(absolute_path)) { + return normalize(absolute_path); + } + return normalize(std::filesystem::weakly_canonical(absolute_path)); } catch (const std::filesystem::filesystem_error &error) { if (error.code() == std::errc::no_such_file_or_directory) { diff --git a/src/lang/parallel/include/sourcemeta/core/parallel_for_each.h b/src/lang/parallel/include/sourcemeta/core/parallel_for_each.h index 65b59c38b4..4ed2bd93cb 100644 --- a/src/lang/parallel/include/sourcemeta/core/parallel_for_each.h +++ b/src/lang/parallel/include/sourcemeta/core/parallel_for_each.h @@ -7,7 +7,7 @@ #include // std::exception_ptr, std::current_exception, std::rethrow_exception #include // std::function #include // std::input_iterator, std::iter_reference_t -#include // std::mutex, std::scoped_lock +#include // std::mutex, std::scoped_lock, std::unique_lock #include // std::queue #include // std::runtime_error #include // std::thread @@ -132,17 +132,18 @@ auto parallel_for_each( &exception_mutex, effective_parallelism, total] { try { while (true) { - Iterator iterator; - std::size_t cursor{0}; - { - std::scoped_lock lock{queue_mutex}; - if (tasks.empty()) { - return; - } - iterator = tasks.front(); - cursor = total - tasks.size() + 1; - tasks.pop(); + // The lock is given up before the work runs, as holding it across the + // callback would leave the other workers waiting on the queue for as + // long as this one takes + std::unique_lock lock{queue_mutex}; + if (tasks.empty()) { + return; } + + const auto iterator{tasks.front()}; + const auto cursor{total - tasks.size() + 1}; + tasks.pop(); + lock.unlock(); effective_callback(*iterator, effective_parallelism, cursor); } } catch (...) { diff --git a/test/benchmark/CMakeLists.txt b/test/benchmark/CMakeLists.txt index 994d232250..b3d6e8437d 100644 --- a/test/benchmark/CMakeLists.txt +++ b/test/benchmark/CMakeLists.txt @@ -27,5 +27,7 @@ add_benchmark_test(help sourcemeta_core_benchmark_success) add_benchmark_test(filter sourcemeta_core_benchmark_success) add_benchmark_test(filter_none sourcemeta_core_benchmark_success) add_benchmark_test(output sourcemeta_core_benchmark_success) +add_benchmark_test(bad_option sourcemeta_core_benchmark_success) +add_benchmark_test(output_unwritable sourcemeta_core_benchmark_success) add_benchmark_test(empty sourcemeta_core_benchmark_empty) add_benchmark_test(stray sourcemeta_core_benchmark_stray) diff --git a/test/benchmark/benchmark_bad_option.clitest b/test/benchmark/benchmark_bad_option.clitest new file mode 100644 index 0000000000..5612383eb5 --- /dev/null +++ b/test/benchmark/benchmark_bad_option.clitest @@ -0,0 +1,12 @@ +RUN --bogus STDIN /dev/null IN . INTO result.txt EXPECTING 1 +WRITE expected.txt UNTIL EXPECTED +1> Usage: sourcemeta_core_benchmark_success [options] +1> +1> Run the registered benchmarks. +1> +1> Options: +1> -f, --filter Only run benchmarks whose name contains +1> -o, --output Also write the results as JSON to +1> -h, --help Show this message +EXPECTED +COMPARE result.txt AGAINST expected.txt diff --git a/test/benchmark/benchmark_output_unwritable.clitest b/test/benchmark/benchmark_output_unwritable.clitest new file mode 100644 index 0000000000..28565772fd --- /dev/null +++ b/test/benchmark/benchmark_output_unwritable.clitest @@ -0,0 +1,9 @@ +RUN --filter nothingmatchesthis --output . STDIN /dev/null IN . INTO result.txt EXPECTING 1 +REPLACE MATCHING "Running on [0-9]+ processors" WITH "Running on N processors" IN result.txt +REPLACE MATCHING "error: [^\n]+" WITH "error: " IN result.txt +WRITE expected.txt UNTIL EXPECTED +1> Running on N processors +2> error: +2> at file path . +EXPECTED +COMPARE result.txt AGAINST expected.txt diff --git a/test/diff/diff_line_myers_test.cc b/test/diff/diff_line_myers_test.cc index 1edffc131d..c360c6932e 100644 --- a/test/diff/diff_line_myers_test.cc +++ b/test/diff/diff_line_myers_test.cc @@ -1218,3 +1218,25 @@ TEST(deleting_one_of_two_equal_lines_shifts_the_hunk_to_the_last) { "-b\n" " c\n"); } + +// A run of changed lines that begins with the same line as the one just past +// it leaves the hunk free to sit either side of that line, so the run is slid +// along until it cannot move, which is what lines a deletion up with the last +// of the repeats it could have taken +TEST(deleted_run_slides_along_a_repeated_line) { + const auto result{ + sourcemeta::core::diff("p\nS\nq\nS\nr\nS\ns\nS\nt\n", "p\nS\nr\nS\nt\n", + sourcemeta::core::Diff::Mode::Line, + sourcemeta::core::Diff::Algorithm::Myers)}; + EXPECT_EQ(render(result, 1), "--- a\n" + "+++ b\n" + "@@ -2,8 +2,4 @@\n" + " S\n" + "-q\n" + "-S\n" + " r\n" + " S\n" + "-s\n" + "-S\n" + " t\n"); +} diff --git a/test/gzip/gzip_compress_test.cc b/test/gzip/gzip_compress_test.cc index e910fd682f..9f8cd7215b 100644 --- a/test/gzip/gzip_compress_test.cc +++ b/test/gzip/gzip_compress_test.cc @@ -615,3 +615,12 @@ TEST(compress_long_repeat_at_a_short_non_overlapping_distance_round_trips) { EXPECT_GZIP_DECOMPRESS(compressed, input); EXPECT_LT(compressed.size(), input.size()); } + +// A match found one position before the end of the search window is held back +// in case the next position does better, and the window closes before that +// next position is reached, so the held match is what the block ends on +TEST(compress_match_deferred_as_the_search_window_closes_round_trips) { + const std::string input{"MATCHfghijklmnopqrstuvwxyMATCHabc"}; + const auto compressed{compress(input, 9)}; + EXPECT_GZIP_DECOMPRESS(compressed, input); +} diff --git a/test/gzip/gzip_decompress_error_test.cc b/test/gzip/gzip_decompress_error_test.cc index 8295645638..fc67cfd6e1 100644 --- a/test/gzip/gzip_decompress_error_test.cc +++ b/test/gzip/gzip_decompress_error_test.cc @@ -1023,3 +1023,25 @@ TEST(missing_stored_block_length) { 0x00, 0x00, 0x00, 0xff, 0x01}; EXPECT_GZIP_DECOMPRESS_ERROR(input, "Unexpected end of source stream"); } + +// RFC 1951 section 3.2.7 sanctions an incomplete code that holds a single +// one-bit codeword, which leaves the other half of the table standing for no +// symbol at all, so a code-length read landing there names nothing +TEST(dynamic_block_code_length_outside_an_incomplete_table) { + const std::vector input{ + 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xff, + 0x05, 0x00, 0x02, 0x20, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}; + EXPECT_GZIP_DECOMPRESS_ERROR(input, "Invalid Huffman code"); +} + +// The same code read off the padding that stands in for input that never +// arrived says only that the source ended, since the bits it turned on were +// never the author's to begin with +TEST(dynamic_block_code_length_read_past_the_end) { + const std::vector input{0x1f, 0x8b, 0x08, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0xff, + 0x05, 0x00, 0x02, 0x20}; + EXPECT_GZIP_DECOMPRESS_ERROR(input, "Unexpected end of source stream"); +} diff --git a/test/idna/idna_is_valid_u_label_test.cc b/test/idna/idna_is_valid_u_label_test.cc index 6aacf64922..7fbe73c49f 100644 --- a/test/idna/idna_is_valid_u_label_test.cc +++ b/test/idna/idna_is_valid_u_label_test.cc @@ -195,3 +195,13 @@ TEST(non_ascii_label_with_a_label_over_63_octets_rejected) { const std::u32string label(60, U'\u00E9'); EXPECT_FALSE(sourcemeta::core::idna_is_valid_u_label(label)); } + +// RFC 5891 §4.2.4 holds a label to 63 octets in A-label form, which is +// measured by encoding it, so a label written far past that length is turned +// down by the encoding running out of room to count with rather than by the +// comparison that would have followed +TEST(u_label_longer_than_the_encoding_can_count) { + std::u32string label(33000, U'a'); + label.push_back(U'\x20000'); + EXPECT_FALSE(sourcemeta::core::idna_is_valid_u_label(label)); +} diff --git a/test/io/io_weakly_canonical_test.cc b/test/io/io_weakly_canonical_test.cc index 0aca6c9f6f..7f905e3ac7 100644 --- a/test/io/io_weakly_canonical_test.cc +++ b/test/io/io_weakly_canonical_test.cc @@ -154,3 +154,27 @@ TEST(posix_relative_fifo) { } #endif + +// Creating symlinks on Windows requires elevated privileges, which CI +// runners typically lack. +#if !defined(_WIN32) +// A symlink pointing at itself resolves to a loop, which is neither a missing +// file nor anything else this maps, so it travels on as it came +TEST(unmapped_error_surfaces_as_filesystem_error) { + const auto loop_path{std::filesystem::path{BUILD_DIRECTORY} / + "sourcemeta_core_io_weakly_canonical_loop"}; + std::filesystem::remove(loop_path); + std::filesystem::create_symlink(loop_path, loop_path); + + try { + sourcemeta::core::weakly_canonical(loop_path); + FAIL(); + } catch (const sourcemeta::core::IOFileNotFoundError &) { + FAIL(); + } catch (const std::filesystem::filesystem_error &error) { + EXPECT_EQ(error.path1(), loop_path); + } + + std::filesystem::remove(loop_path); +} +#endif diff --git a/test/jsonpath/jsonpath_parse_test.cc b/test/jsonpath/jsonpath_parse_test.cc index 8af30d6e1b..fbc9beea42 100644 --- a/test/jsonpath/jsonpath_parse_test.cc +++ b/test/jsonpath/jsonpath_parse_test.cc @@ -569,3 +569,24 @@ TEST(parse_rejects_an_integer_literal_too_large_to_hold) { TEST(parse_rejects_an_integer_literal_below_the_allowed_range) { EXPECT_JSONPATH_PARSE_ERROR("$[?@.a==-9007199254740992]", 26); } + +// RFC 9535 Section 2.4.1 writes a function expression as the function name +// followed by its arguments in parentheses, so a name standing on its own is +// not one however well the name itself reads +TEST(jsonpath_parse_function_name_without_arguments) { + EXPECT_JSONPATH_PARSE_ERROR("$[?length]", 10); +} + +// RFC 9535 Section 2.1 restricts a query to I-JSON, which RFC 7493 Section 2.2 +// keeps within the range the double precision format reaches, so a literal +// written past that range names no value a query may carry +TEST(jsonpath_parse_number_literal_past_every_representable_value) { + EXPECT_JSONPATH_PARSE_ERROR("$[?@.a == 1e99999999999999999999]", 33); +} + +// RFC 9535 Section 2.4.1 writes a function expression as the function name +// followed by its arguments in parentheses, so a name standing alone is not +// one, which a logical-not leaves nowhere else to be read as +TEST(jsonpath_parse_function_name_without_parentheses) { + EXPECT_JSONPATH_PARSE_ERROR("$[?!length]", 11); +} diff --git a/test/markdown/markdown_pathological_test.cc b/test/markdown/markdown_pathological_test.cc index 3ae7222153..8c5e6e36b6 100644 --- a/test/markdown/markdown_pathological_test.cc +++ b/test/markdown/markdown_pathological_test.cc @@ -354,3 +354,45 @@ TEST(unclosed_inline_declarations) { sourcemeta::core::markdown_to_html("x" + repeat("x" + repeat("<!A ", 4999) + "<!A

\n"); } + +// The parser addresses its input through 32-bit offsets, which is a reach no +// caller can widen, but a caller with less to spend than that may hold a +// conversion to a smaller input than the one it was handed +TEST(input_past_the_bound_the_caller_allows) { + try { + sourcemeta::core::markdown_to_html("Hello **world**", true, 8); + FAIL(); + } catch (const sourcemeta::core::MarkdownError &error) { + EXPECT_STREQ(error.what(), + "The input is larger than the supported size bound"); + } + + EXPECT_EQ(sourcemeta::core::markdown_to_html("Hello **world**", true, 15), + "

Hello world

\n"); +} + +TEST(input_past_the_bound_the_caller_allows_when_unsafe) { + try { + sourcemeta::core::markdown_to_html("x", false, 4); + FAIL(); + } catch (const sourcemeta::core::MarkdownError &error) { + EXPECT_STREQ(error.what(), + "The input is larger than the supported size bound"); + } + + EXPECT_EQ(sourcemeta::core::markdown_to_html("x", false, 8), + "

x

\n"); +} + +// The replacement a malformed byte sequence goes through stands in for it +// before the bound is read, so what the bound counts is what the parser is +// handed rather than what the caller wrote +TEST(input_past_the_bound_the_caller_allows_after_replacement) { + try { + sourcemeta::core::markdown_to_html("a\xFF", true, 1); + FAIL(); + } catch (const sourcemeta::core::MarkdownError &error) { + EXPECT_STREQ(error.what(), + "The input is larger than the supported size bound"); + } +} diff --git a/test/oauth/oauth_dpop_test.cc b/test/oauth/oauth_dpop_test.cc index 816e896ffb..37047193f7 100644 --- a/test/oauth/oauth_dpop_test.cc +++ b/test/oauth/oauth_dpop_test.cc @@ -1330,3 +1330,63 @@ TEST(replay_store_zero_capacity_falls_back_to_the_default) { EXPECT_FALSE(store.check_and_insert("id", "https://server.example.com/token", FIXED_TIME, std::chrono::seconds{300})); } + +// RFC 9449 Section 4.2 signs the target URI, which RFC 9110 Section 4.2.1 +// gives an authority, so a URI that names a scheme and no authority at all +// names no target to bind the proof to however well it parses +TEST(proofer_rejects_a_target_without_an_authority) { + auto key{sourcemeta::core::JWKPrivate::from( + sourcemeta::core::parse_json(EC_PRIVATE_JWK))}; + EXPECT_TRUE(key.has_value()); + sourcemeta::core::OAuthDPoPProofer proofer{ + std::move(key.value()), sourcemeta::core::JWSAlgorithm::ES256}; + std::string proof; + EXPECT_FALSE(proofer.proof("https://server.example.com", "POST", + "urn:example:token", "", FIXED_TIME, proof)); + EXPECT_TRUE(proof.empty()); +} + +// RFC 9449 Section 4.3 check 7 turns down a proof whose embedded key carries +// private material, which leaves a key that is an object holding none of it +// still to be read as a key before anything can be bound to it +TEST(proof_thumbprint_rejects_a_jwk_header_that_is_not_a_key) { + const std::string proof{ + "eyJ0eXAiOiJkcG9wK2p3dCIsImFsZyI6IkVTMjU2IiwiandrIjp7Imt0eSI6Im9vcHMifX0" + ".eyJodHUiOiJodHRwczovL3JzLmV4YW1wbGUuY29tL3giLCJodG0iOiJQT1NUIiwianRpIjo" + "i" + "YWJjIiwiaWF0IjoxNzAwMDAwMDAwfQ.c2ln"}; + EXPECT_FALSE( + sourcemeta::core::oauth_dpop_proof_thumbprint(proof).has_value()); +} + +// RFC 9449 Section 4.2 binds the proof to the public half of the key that +// signed it, which a symmetric key has none of, so naming an asymmetric +// algorithm over one leaves nothing to embed even though the algorithm passes +TEST(proofer_rejects_a_key_with_no_public_half) { + auto key{sourcemeta::core::JWKPrivate::from( + sourcemeta::core::parse_json(OCT_JWK))}; + EXPECT_TRUE(key.has_value()); + sourcemeta::core::OAuthDPoPProofer proofer{ + std::move(key.value()), sourcemeta::core::JWSAlgorithm::ES256}; + std::string proof; + EXPECT_FALSE(proofer.proof("https://server.example.com", "POST", + "https://server.example.com/token", "", FIXED_TIME, + proof)); + EXPECT_TRUE(proof.empty()); +} + +// An algorithm the signing key cannot answer for leaves nothing to sign the +// proof with, which is settled where the signature is taken rather than when +// the two were put together +TEST(proofer_rejects_an_algorithm_the_key_cannot_sign_with) { + auto key{sourcemeta::core::JWKPrivate::from( + sourcemeta::core::parse_json(EC_PRIVATE_JWK))}; + EXPECT_TRUE(key.has_value()); + sourcemeta::core::OAuthDPoPProofer proofer{ + std::move(key.value()), sourcemeta::core::JWSAlgorithm::RS256}; + std::string proof; + EXPECT_FALSE(proofer.proof("https://server.example.com", "POST", + "https://server.example.com/token", "", FIXED_TIME, + proof)); + EXPECT_TRUE(proof.empty()); +} diff --git a/test/oidc/oidc_authentication_test.cc b/test/oidc/oidc_authentication_test.cc index 010e6d4301..739f17a89a 100644 --- a/test/oidc/oidc_authentication_test.cc +++ b/test/oidc/oidc_authentication_test.cc @@ -697,3 +697,22 @@ TEST(parse_drops_offline_access_scope_with_prompt_none) { storage, request, sourcemeta::core::OIDCProfile::Legacy)); EXPECT_EQ(request.scope, "openid"); } + +// OpenID Connect Core 1.0 Section 3.1.2.1 makes the client identifier and the +// redirection URI REQUIRED, so the convenience has nothing to build a URL out +// of when either is missing, whatever else it fills in for the caller +TEST(authorization_url_convenience_without_a_client_identifier) { + EXPECT_FALSE( + sourcemeta::core::oidc_authorization_url( + "https://server.example/authorize", "", "https://client.example/cb", + "xyz", "E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM", "n-0S6_WzA2Mj") + .has_value()); +} + +TEST(authorization_url_convenience_without_a_redirection_uri) { + EXPECT_FALSE(sourcemeta::core::oidc_authorization_url( + "https://server.example/authorize", "s6BhdRkqt3", "", "xyz", + "E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM", + "n-0S6_WzA2Mj") + .has_value()); +} diff --git a/test/oidc/oidc_id_token_test.cc b/test/oidc/oidc_id_token_test.cc index 99f4d71f48..f8bd19bf93 100644 --- a/test/oidc/oidc_id_token_test.cc +++ b/test/oidc/oidc_id_token_test.cc @@ -1434,3 +1434,25 @@ TEST(validate_rejects_a_required_code_hash_that_does_not_match) { "client-id", REFERENCE_NOW, options)}; EXPECT_FALSE(identity.has_value()); } + +// RFC 7519 Section 2 lets a NumericDate be non-integer, so a value past what a +// timestamp can stand for is read as no authentication time at all rather than +// aborting, which a freshness window then has nothing to measure against +TEST(validate_rejects_an_auth_time_beyond_the_representable_range) { + const auto compact{sign_id_token(R"JSON({ + "iss": "https://issuer.example", + "sub": "user-1", + "aud": "client-id", + "auth_time": 1e300, + "iat": 1699996400, + "exp": 2000000000 + })JSON")}; + const auto token{sourcemeta::core::JWT::from(compact)}; + EXPECT_TRUE(token.has_value()); + sourcemeta::core::OIDCValidationOptions options; + options.maximum_authentication_age = std::chrono::hours{2}; + const auto rejected{sourcemeta::core::oidc_validate_id_token( + token.value(), oct_key_set(), ALLOWED_HS256, "https://issuer.example", + "client-id", REFERENCE_NOW, options)}; + EXPECT_FALSE(rejected.has_value()); +} diff --git a/test/oidc/oidc_metadata_test.cc b/test/oidc/oidc_metadata_test.cc index d9a22d6514..aa69f30430 100644 --- a/test/oidc/oidc_metadata_test.cc +++ b/test/oidc/oidc_metadata_test.cc @@ -914,3 +914,23 @@ TEST(make_emits_the_request_parameter_capability_when_supported) { EXPECT_TRUE(document.has_value()); EXPECT_TRUE(document.value().at("request_parameter_supported").to_boolean()); } + +// RFC 8414 Section 2 makes the issuer a REQUIRED issuer identifier, which the +// provider document is built on top of, so an issuer the layer below refuses +// leaves nothing for this one to add to +TEST(make_refuses_an_issuer_the_underlying_document_rejects) { + const std::array response_types{{"code"}}; + const std::array subject_types{{"public"}}; + const std::array id_token_algs{{"RS256"}}; + sourcemeta::core::OIDCProviderMetadataConfig config; + config.base.issuer = "https://server.example/path?query=1"; + config.base.authorization_endpoint = "https://server.example/authorize"; + config.base.token_endpoint = "https://server.example/token"; + config.base.jwks_uri = "https://server.example/jwks"; + config.base.response_types_supported = response_types; + config.subject_types_supported = subject_types; + config.id_token_signing_alg_values_supported = id_token_algs; + + EXPECT_FALSE( + sourcemeta::core::oidc_make_provider_metadata(config).has_value()); +} diff --git a/test/oidc/oidc_registration_test.cc b/test/oidc/oidc_registration_test.cc index f836d616ab..dc8a0e901a 100644 --- a/test/oidc/oidc_registration_test.cc +++ b/test/oidc/oidc_registration_test.cc @@ -575,3 +575,24 @@ TEST(accepts_the_largest_representable_default_max_age) { EXPECT_EQ(metadata.value().default_max_age().value(), std::chrono::seconds::max()); } + +// RFC 7591 Section 2 describes client metadata as a JSON object, so a document +// that is anything else is turned down by the registration layer this one +// builds on rather than by any rule of its own +TEST(from_rejects_a_document_that_is_not_an_object) { + auto document{sourcemeta::core::parse_json("[]")}; + EXPECT_FALSE(sourcemeta::core::OIDCClientMetadata::from(std::move(document)) + .has_value()); +} + +// RFC 7591 Section 2: "The `jwks_uri` and `jwks` parameters MUST NOT both be +// present in the same request or response" +TEST(from_rejects_both_a_key_set_and_a_key_set_uri) { + auto document{sourcemeta::core::parse_json(R"JSON({ + "redirect_uris": [ "https://client.example/cb" ], + "jwks_uri": "https://client.example/jwks.json", + "jwks": { "keys": [] } + })JSON")}; + EXPECT_FALSE(sourcemeta::core::OIDCClientMetadata::from(std::move(document)) + .has_value()); +} diff --git a/test/punycode/punycode_decode_test.cc b/test/punycode/punycode_decode_test.cc index 29c3051e55..c9f325e669 100644 --- a/test/punycode/punycode_decode_test.cc +++ b/test/punycode/punycode_decode_test.cc @@ -431,3 +431,42 @@ TEST(error_digit_just_past_the_decimal_run) { EXPECT_STREQ(error.what(), "Invalid digit"); } } + +// The bound a decode answers to counts the bytes it was handed rather than the +// code points they stand for, as the position counter walks the encoded form +TEST(decode_refuses_more_bytes_than_the_caller_allows) { + try { + [[maybe_unused]] const auto result{ + sourcemeta::core::punycode_to_utf32("Mnchen-3ya", 9)}; + FAIL(); + } catch (const sourcemeta::core::PunycodeError &error) { + EXPECT_STREQ(error.what(), "Input is too large"); + } +} + +TEST(decode_accepts_exactly_as_many_bytes_as_the_caller_allows) { + EXPECT_EQ(sourcemeta::core::punycode_to_utf8("Mnchen-3ya", 10), + "M\xC3\xBCnchen"); +} + +TEST(decode_to_utf8_refuses_more_bytes_than_the_caller_allows) { + try { + [[maybe_unused]] const auto result{ + sourcemeta::core::punycode_to_utf8("Mnchen-3ya", 9)}; + FAIL(); + } catch (const sourcemeta::core::PunycodeError &error) { + EXPECT_STREQ(error.what(), "Input is too large"); + } +} + +TEST(decode_to_utf8_stream_refuses_more_bytes_than_the_caller_allows) { + std::istringstream input{"Mnchen-3ya"}; + std::ostringstream output; + + try { + sourcemeta::core::punycode_to_utf8(input, output, 9); + FAIL(); + } catch (const sourcemeta::core::PunycodeError &error) { + EXPECT_STREQ(error.what(), "Input is too large"); + } +} diff --git a/test/punycode/punycode_encode_test.cc b/test/punycode/punycode_encode_test.cc index e04907622e..8a8eacf4ff 100644 --- a/test/punycode/punycode_encode_test.cc +++ b/test/punycode/punycode_encode_test.cc @@ -414,3 +414,47 @@ TEST(error_encode_overflow) { EXPECT_STREQ(error.what(), "Encode overflow"); } } + +// RFC 3492 Section 6.4 leaves bounding the input to the implementation, so a +// caller may hold an encode to fewer code points than the position counters +// could address +TEST(encode_refuses_more_code_points_than_the_caller_allows) { + const std::u32string input{0x004D, 0x00FC, 0x006E, 0x0063, + 0x0068, 0x0065, 0x006E}; + + try { + [[maybe_unused]] const auto result{ + sourcemeta::core::utf32_to_punycode(input, 6)}; + FAIL(); + } catch (const sourcemeta::core::PunycodeError &error) { + EXPECT_STREQ(error.what(), "Input is too large"); + } +} + +TEST(encode_accepts_exactly_as_many_code_points_as_the_caller_allows) { + const std::u32string input{0x004D, 0x00FC, 0x006E, 0x0063, + 0x0068, 0x0065, 0x006E}; + EXPECT_EQ(sourcemeta::core::utf32_to_punycode(input, 7), "Mnchen-3ya"); +} + +TEST(encode_from_utf8_refuses_more_code_points_than_the_caller_allows) { + try { + [[maybe_unused]] const auto result{ + sourcemeta::core::utf8_to_punycode("M\xC3\xBCnchen", 6)}; + FAIL(); + } catch (const sourcemeta::core::PunycodeError &error) { + EXPECT_STREQ(error.what(), "Input is too large"); + } +} + +TEST(encode_from_utf8_stream_refuses_more_code_points_than_the_caller_allows) { + std::istringstream input{"M\xC3\xBCnchen"}; + std::ostringstream output; + + try { + sourcemeta::core::utf8_to_punycode(input, output, 6); + FAIL(); + } catch (const sourcemeta::core::PunycodeError &error) { + EXPECT_STREQ(error.what(), "Input is too large"); + } +} diff --git a/test/test/clitest/CMakeLists.txt b/test/test/clitest/CMakeLists.txt index 71434b6628..0f3469512b 100644 --- a/test/test/clitest/CMakeLists.txt +++ b/test/test/clitest/CMakeLists.txt @@ -84,6 +84,7 @@ add_clitest_test(write_without_until 1) add_clitest_run_test(error_bad_pattern 1) +add_clitest_run_test(error_drop_bad_pattern 1) add_clitest_run_test(error_checksum_algorithm_is_not_sha256 1) add_clitest_run_test(error_checksum_binding_preposition 1) add_clitest_run_test(error_checksum_usage 1) diff --git a/test/test/clitest/error_drop_bad_pattern.clitest b/test/test/clitest/error_drop_bad_pattern.clitest new file mode 100644 index 0000000000..a13bebe866 --- /dev/null +++ b/test/test/clitest/error_drop_bad_pattern.clitest @@ -0,0 +1,4 @@ +WRITE payload.txt UNTIL END +content +END +DROP LINES MATCHING '[' IN payload.txt diff --git a/test/test/clitest/error_drop_bad_pattern.txt b/test/test/clitest/error_drop_bad_pattern.txt new file mode 100644 index 0000000000..3dda64e78f --- /dev/null +++ b/test/test/clitest/error_drop_bad_pattern.txt @@ -0,0 +1,5 @@ +TAP version 14 +1..1 +not ok 1 - error_drop_bad_pattern.clitest +# error_drop_bad_pattern.clitest:4: bad pattern: [ +# 0 passed, 1 failed diff --git a/test/uri/uri_path_test.cc b/test/uri/uri_path_test.cc index 788d4629b0..073e955b0f 100644 --- a/test/uri/uri_path_test.cc +++ b/test/uri/uri_path_test.cc @@ -1161,3 +1161,18 @@ TEST(append_path_rvalue_reference_on_a_relative_reference) { uri.append_path(std::move(reference)); EXPECT_EQ(uri.recompose(), "base/extra"); } + +// A reference with an authority but no scheme still names a root for its path +// to hang off, so what decides the leading slash is the authority rather than +// the scheme that is missing +TEST(append_path_to_an_authority_without_a_scheme) { + sourcemeta::core::URI uri{"//example.com"}; + uri.append_path("test"); + EXPECT_EQ(uri.recompose(), "//example.com/test"); +} + +TEST(append_path_to_a_urn) { + sourcemeta::core::URI uri{"urn:example:a"}; + uri.append_path("b"); + EXPECT_EQ(uri.recompose(), "urn:example:a/b"); +} diff --git a/test/uri/uri_relative_to_test.cc b/test/uri/uri_relative_to_test.cc index 3fe8f53c47..a8237d971f 100644 --- a/test/uri/uri_relative_to_test.cc +++ b/test/uri/uri_relative_to_test.cc @@ -650,3 +650,12 @@ TEST(ip_literal_base_and_registered_name_target_stay_absolute) { uri.relative_to(base); EXPECT_EQ(uri.recompose(), "https://v1.x/a/c"); } + +// A URN carries its hierarchy inside the path rather than under a slashed +// root, so walking up from its parent runs out of separators before reaching +// one, which is where the walk gives up rather than stepping past the start +TEST(relative_to_a_urn_whose_parent_has_no_further_separator) { + sourcemeta::core::URI uri{"urn:other:x"}; + uri.relative_to(sourcemeta::core::URI{"urn:example:a/b"}); + EXPECT_EQ(uri.recompose(), "urn:other:x"); +} diff --git a/test/uritemplate/uritemplate_expand_uri_test.cc b/test/uritemplate/uritemplate_expand_uri_test.cc index fa8209fa08..95ff291f9d 100644 --- a/test/uritemplate/uritemplate_expand_uri_test.cc +++ b/test/uritemplate/uritemplate_expand_uri_test.cc @@ -945,3 +945,46 @@ TEST(reserved_expansion_of_a_percent_sign_with_no_hexadecimal) { EXPECT_EQ(result, "a%25zb"); } + +// RFC 6570 Section 3.2.9 writes an exploded named expansion as a repetition of +// "name=value", so a member that is the empty string still contributes its +// name and the separator that would have introduced a value +TEST(list_explode_query_with_an_empty_member) { + const sourcemeta::core::URITemplate uri_template{"{?list*}"}; + std::size_t call_count = 0; + const auto result = uri_template.expand( + [&call_count]( + const std::string_view name) -> sourcemeta::core::URITemplateValue { + if (name == "list") { + call_count++; + if (call_count == 1) { + return std::make_tuple(std::string_view{""}, std::nullopt, true); + } + return std::make_tuple(std::string_view{"x"}, std::nullopt, false); + } + return std::nullopt; + }); + + EXPECT_EQ(result, "?list=&list=x"); +} + +// A caller answering that more values follow and then handing over none has +// nothing left to append, so the expansion stops where it was rather than +// writing a separator for a value that never arrives +TEST(list_explode_query_promising_more_than_it_gives) { + const sourcemeta::core::URITemplate uri_template{"{?list*}"}; + std::size_t call_count = 0; + const auto result = uri_template.expand( + [&call_count]( + const std::string_view name) -> sourcemeta::core::URITemplateValue { + if (name == "list") { + call_count++; + if (call_count == 1) { + return std::make_tuple(std::string_view{"a"}, std::nullopt, true); + } + } + return std::nullopt; + }); + + EXPECT_EQ(result, "?list=a"); +} diff --git a/test/uritemplate/uritemplate_router_test.cc b/test/uritemplate/uritemplate_router_test.cc index 88ec9052b4..54ba660643 100644 --- a/test/uritemplate/uritemplate_router_test.cc +++ b/test/uritemplate/uritemplate_router_test.cc @@ -3284,3 +3284,21 @@ TEST(a_route_of_a_single_variable) { EXPECT_TRUE(router.describes("/anything")); EXPECT_FALSE(router.describes("/anything/else")); } + +// A segment that opens with a literal, turns into an expression and then +// carries on in literal text again is reported whole, so the span widens past +// the expression to wherever the segment actually ends +TEST(add_rejects_a_literal_variable_literal_segment) { + sourcemeta::core::URITemplateRouter router; + try { + router.add("/a{x}b", "op_bad", 1); + FAIL(); + } catch ( + const sourcemeta::core::URITemplateRouterInvalidSegmentError &error) { + EXPECT_STREQ(error.what(), + "Path segment cannot mix literals and variables"); + EXPECT_EQ(error.segment(), "a{x}b"); + } catch (...) { + FAIL(); + } +} diff --git a/test/uritemplate/uritemplate_router_view_test.cc b/test/uritemplate/uritemplate_router_view_test.cc index fbdc0f76c2..93045a56a0 100644 --- a/test/uritemplate/uritemplate_router_view_test.cc +++ b/test/uritemplate/uritemplate_router_view_test.cc @@ -6,7 +6,7 @@ #include // std::array #include // INT64_MIN, INT64_MAX #include // std::uint8_t, std::int64_t -#include // std::memset +#include // std::memset, std::memcpy #include // std::filesystem::path, std::filesystem::remove, std::filesystem::temp_directory_path #include // std::ifstream #include // std::istreambuf_iterator @@ -5625,3 +5625,45 @@ TEST_F(URITemplateRouterViewTest, save_to_a_destination_with_no_room_throws) { } } #endif + +// A node naming its capture outside the string table has no name to hand back, +// so the walk gives up where it reaches that node rather than reading past the +// table it was handed +TEST_F(URITemplateRouterViewTest, + match_rejects_an_optional_expansion_naming_a_string_past_the_table) { + { + sourcemeta::core::URITemplateRouter router; + router.add("/list{/path*}", "op_1", 1); + sourcemeta::core::URITemplateRouterView::save(router, this->path_); + } + + std::vector blob; + { + std::ifstream file{this->path_, std::ios::binary | std::ios::ate}; + const auto size = static_cast(file.tellg()); + file.seekg(0, std::ios::beg); + blob.resize(size); + file.read(reinterpret_cast(blob.data()), + static_cast(size)); + } + + // The nodes follow the 48 byte header one after another, each 32 bytes wide, + // carrying the kind they are at offset 20 and the string they name at 0 + std::uint32_t node_count{0}; + std::memcpy(&node_count, blob.data() + 8, sizeof(node_count)); + std::size_t optional_expansion{0}; + for (std::uint32_t index = 0; index < node_count; ++index) { + if (blob.at(48 + (index * 32) + 20) == 4) { + optional_expansion = 48 + (index * 32); + } + } + + EXPECT_TRUE(optional_expansion != 0); + const std::uint32_t past_the_table{0xFFFFFFFF}; + std::memcpy(blob.data() + optional_expansion, &past_the_table, + sizeof(past_the_table)); + + const sourcemeta::core::URITemplateRouterView view{blob.data(), blob.size()}; + EXPECT_ROUTER_MATCH(view, "/list", 0, 0, captures); + EXPECT_TRUE(captures.empty()); +} From 401bffb36c41f81773896e2fbcf1eb1b921d734e Mon Sep 17 00:00:00 2001 From: Juan Cruz Viotti Date: Tue, 13 Oct 2026 15:38:33 -0300 Subject: [PATCH 2/3] Fix Signed-off-by: Juan Cruz Viotti --- src/core/diff/diff.cc | 42 ++++++++++++------- src/core/diff/myers.h | 26 +++++++----- .../include/sourcemeta/core/http_message.h | 8 +++- src/core/jsonpath/parser.h | 12 ++++-- .../include/sourcemeta/core/markdown.h | 4 +- src/core/oidc/oidc_id_token.cc | 18 +++++--- .../include/sourcemeta/core/punycode.h | 18 +++++--- src/core/punycode/punycode.cc | 7 ++++ test/diff/diff_line_myers_test.cc | 31 ++++++++++++++ test/jsonpath/jsonpath_parse_test.cc | 7 ++++ test/markdown/markdown_pathological_test.cc | 6 ++- test/oidc/oidc_id_token_test.cc | 10 ++--- .../uritemplate_expand_uri_test.cc | 2 +- 13 files changed, 141 insertions(+), 50 deletions(-) diff --git a/src/core/diff/diff.cc b/src/core/diff/diff.cc index 1368125b76..756ba64295 100644 --- a/src/core/diff/diff.cc +++ b/src/core/diff/diff.cc @@ -69,6 +69,31 @@ auto to_identities( } } +// The search is given its workspace here rather than alongside the result, so +// that the two buffers it sizes to the span of the change are given up before +// anything is made of what the search found +auto compare_with_myers(const std::vector &original_identities, + const std::vector &modified_identities, + const std::size_t prefix, + const std::size_t original_end, + const std::size_t modified_end, + std::vector &original_changed, + std::vector &modified_changed) -> void { + sourcemeta::core::internal::MyersWorkspace workspace; + // A search only needs its workspace once both sides have something left, as + // any other case resolves to a plain insertion or deletion + if (original_end > prefix && modified_end > prefix) { + const auto span{(original_end - prefix) + (modified_end - prefix)}; + workspace.forward.assign((2 * span) + 3, 0); + workspace.backward.assign((2 * span) + 3, 0); + workspace.offset = static_cast(span + 1); + } + + sourcemeta::core::internal::myers_compare( + original_identities, prefix, original_end, modified_identities, prefix, + modified_end, original_changed, modified_changed, workspace); +} + // Slide every run of changed lines as far towards the end of the input as the // surrounding lines allow, which is what makes paired changes line up auto compact_changes(const std::vector &lines, @@ -224,22 +249,11 @@ auto diff(const std::string_view original, const std::string_view modified, std::vector original_changed(original_size, false); std::vector modified_changed(modified_size, false); - internal::MyersWorkspace workspace; - switch (algorithm) { case Diff::Algorithm::Myers: - // A search only needs its workspace once both sides have something left, - // as any other case resolves to a plain insertion or deletion - if (original_end > prefix && modified_end > prefix) { - const auto span{(original_end - prefix) + (modified_end - prefix)}; - workspace.forward.assign((2 * span) + 3, 0); - workspace.backward.assign((2 * span) + 3, 0); - workspace.offset = static_cast(span + 1); - } - - internal::myers_compare(original_identities, prefix, original_end, - modified_identities, prefix, modified_end, - original_changed, modified_changed, workspace); + compare_with_myers(original_identities, modified_identities, prefix, + original_end, modified_end, original_changed, + modified_changed); break; } diff --git a/src/core/diff/myers.h b/src/core/diff/myers.h index d997b121b0..c1ba7f3632 100644 --- a/src/core/diff/myers.h +++ b/src/core/diff/myers.h @@ -134,16 +134,22 @@ inline auto myers_compare(const std::vector &original, std::vector &original_changed, std::vector &modified_changed, MyersWorkspace &workspace) -> void { - // The lines the two sides share at the head and at the tail are taken off - // before any of this is reached, which is what sizes the identity tables and - // the workspace, so no run of equal lines is left at either end to slide - // past here. A sub-problem begins and ends where a slide of the search - // stopped, and a slide stops exactly where the two sides differ, so the same - // holds of what the recursion hands back to itself - assert(original_start == original_end || modified_start == modified_end || - original[original_start] != modified[modified_start]); - assert(original_start == original_end || modified_start == modified_end || - original[original_end - 1] != modified[modified_end - 1]); + // The caller takes off the lines the two sides share at the head and at the + // tail before the search begins, but a middle snake can still leave a + // sub-problem whose own ends match: a forward search never looks behind + // where its snake starts, and a backward search never looks past where its + // snake ends, so each recursion trims again for itself + while (original_start < original_end && modified_start < modified_end && + original[original_start] == modified[modified_start]) { + original_start += 1; + modified_start += 1; + } + + while (original_start < original_end && modified_start < modified_end && + original[original_end - 1] == modified[modified_end - 1]) { + original_end -= 1; + modified_end -= 1; + } if (original_start == original_end) { for (auto index{modified_start}; index < modified_end; ++index) { diff --git a/src/core/http/include/sourcemeta/core/http_message.h b/src/core/http/include/sourcemeta/core/http_message.h index 7a69f52158..de641e3270 100644 --- a/src/core/http/include/sourcemeta/core/http_message.h +++ b/src/core/http/include/sourcemeta/core/http_message.h @@ -9,6 +9,7 @@ #include // std::nullopt, std::optional #include // std::string #include // std::string_view +#include // std::is_same_v #include // std::move namespace sourcemeta::core { @@ -398,9 +399,14 @@ inline auto http_serialize_headers(const Headers &headers) -> std::string { } // A field value either carries its own bytes or already is the bytes, so - // which of the two it is settles only what gets read out of it here + // which of the two it is settles only what gets read out of it here. A + // value that carries them hands back a view of storage it keeps, which is + // what lets the view below outlive the call that produced it std::string_view bytes; if constexpr (requires { value.bytes(); }) { + static_assert(std::is_same_v, + "A field value must hand back a view of bytes it keeps, " + "as an owning result would leave nothing to write out"); bytes = value.bytes(); } else { bytes = value; diff --git a/src/core/jsonpath/parser.h b/src/core/jsonpath/parser.h index 832ec39622..d3b5ff001a 100644 --- a/src/core/jsonpath/parser.h +++ b/src/core/jsonpath/parser.h @@ -886,10 +886,14 @@ class JSONPathParser { return JSON{exact.value()}; } - // The range check above holds the literal to what the double precision - // format reaches, which sits far inside the exponent a decimal carries, so - // reading it as one always answers - return JSON{Decimal{text}}; + // The range check above is on the value, which a literal of zero passes + // however it spells its exponent, so what is read here can still name a + // scale no decimal carries + try { + return JSON{Decimal{text}}; + } catch (const DecimalParseError &) { + this->fail(); + } } // filter-query = rel-query / jsonpath-query diff --git a/src/core/markdown/include/sourcemeta/core/markdown.h b/src/core/markdown/include/sourcemeta/core/markdown.h index 20ad935e03..2dd374aa63 100644 --- a/src/core/markdown/include/sourcemeta/core/markdown.h +++ b/src/core/markdown/include/sourcemeta/core/markdown.h @@ -46,7 +46,9 @@ inline constexpr std::size_t MARKDOWN_MAXIMUM_INPUT_BYTES{ /// once the rendered link references expand past the larger of 16 times the /// input size and 1,048,576 bytes, or once a table inserts more than 524,288 /// empty cells. A bound smaller than the default refuses a larger input -/// outright. For example: +/// outright, counted after a NUL or a malformed byte sequence has been +/// replaced, so a repaired input may exceed a bound the written one meets. +/// For example: /// /// ```cpp /// #include diff --git a/src/core/oidc/oidc_id_token.cc b/src/core/oidc/oidc_id_token.cc index b9988ff62e..a7abd0bf7d 100644 --- a/src/core/oidc/oidc_id_token.cc +++ b/src/core/oidc/oidc_id_token.cc @@ -8,8 +8,9 @@ #include // std::chrono::system_clock, std::chrono::seconds, std::chrono::duration, std::chrono::duration_cast #include // std::int64_t -#include // std::optional, std::nullopt +#include // std::optional #include // std::span +#include // std::out_of_range, std::nullopt #include // std::string #include // std::string_view @@ -169,12 +170,17 @@ auto oidc_id_token_checks(const JWT &token, const std::string_view issuer, std::optional authentication_time; const auto *auth_time{payload.try_at("auth_time"sv, HASH_AUTH_TIME)}; if (auth_time != nullptr && auth_time->is_number()) { - // A NumericDate may be non-integer (RFC 7519 Section 2), and a value the - // clock cannot stand for comes back as no time at all, which reads as an - // absent claim rather than aborting, matching how the JWT layer reads the + // A NumericDate may be non-integer (RFC 7519 Section 2). A value past what + // a double stands for refuses to be read as one at all, and one the clock + // cannot stand for comes back as no time, so either way the claim reads as + // absent rather than aborting, matching how the JWT layer reads the // registered time claims - authentication_time = from_unix_timestamp( - std::chrono::duration{auth_time->as_real()}); + try { + authentication_time = from_unix_timestamp( + std::chrono::duration{auth_time->as_real()}); + } catch (const std::out_of_range &) { + authentication_time = std::nullopt; + } } if (options.maximum_authentication_age.has_value()) { diff --git a/src/core/punycode/include/sourcemeta/core/punycode.h b/src/core/punycode/include/sourcemeta/core/punycode.h index 60b7597075..d6169e16fe 100644 --- a/src/core/punycode/include/sourcemeta/core/punycode.h +++ b/src/core/punycode/include/sourcemeta/core/punycode.h @@ -63,7 +63,8 @@ inline constexpr std::size_t PUNYCODE_MAXIMUM_ENCODED_BYTES{ /// because the C++ standard library does not define the required locale facets /// (`std::ctype`) for `std::basic_istream` and /// `std::basic_ostream` to function properly. -/// A bound smaller than the default refuses a larger input outright. +/// +/// A bound smaller than the default refuses a larger input. SOURCEMETA_CORE_PUNYCODE_EXPORT auto utf32_to_punycode(std::u32string_view input, std::size_t maximum_code_points = @@ -82,7 +83,8 @@ auto utf32_to_punycode(std::u32string_view input, /// sourcemeta::core::utf8_to_punycode(input, output); /// assert(output.str() == "Mnchen-3ya"); /// ``` -/// A bound smaller than the default refuses a larger input outright. +/// +/// A bound smaller than the default refuses a larger input. SOURCEMETA_CORE_PUNYCODE_EXPORT auto utf8_to_punycode( std::istream &input, std::ostream &output, @@ -98,7 +100,8 @@ auto utf8_to_punycode( /// assert(sourcemeta::core::utf8_to_punycode("M\xC3\xBCnchen") == /// "Mnchen-3ya"); /// ``` -/// A bound smaller than the default refuses a larger input outright. +/// +/// A bound smaller than the default refuses a larger input. SOURCEMETA_CORE_PUNYCODE_EXPORT auto utf8_to_punycode(const std::string_view input, std::size_t maximum_code_points = @@ -123,7 +126,8 @@ auto utf8_to_punycode(const std::string_view input, /// because the C++ standard library does not define the required locale facets /// (`std::ctype`) for `std::basic_istream` and /// `std::basic_ostream` to function properly. -/// A bound smaller than the default refuses a larger input outright. +/// +/// A bound smaller than the default refuses a larger input. SOURCEMETA_CORE_PUNYCODE_EXPORT auto punycode_to_utf32(const std::string_view input, std::size_t maximum_encoded_bytes = @@ -142,7 +146,8 @@ auto punycode_to_utf32(const std::string_view input, /// sourcemeta::core::punycode_to_utf8(input, output); /// assert(output.str() == "M\xC3\xBCnchen"); /// ``` -/// A bound smaller than the default refuses a larger input outright. +/// +/// A bound smaller than the default refuses a larger input. SOURCEMETA_CORE_PUNYCODE_EXPORT auto punycode_to_utf8( std::istream &input, std::ostream &output, @@ -158,7 +163,8 @@ auto punycode_to_utf8( /// assert(sourcemeta::core::punycode_to_utf8("Mnchen-3ya") == /// "M\xC3\xBCnchen"); /// ``` -/// A bound smaller than the default refuses a larger input outright. +/// +/// A bound smaller than the default refuses a larger input. SOURCEMETA_CORE_PUNYCODE_EXPORT auto punycode_to_utf8(const std::string_view input, std::size_t maximum_encoded_bytes = diff --git a/src/core/punycode/punycode.cc b/src/core/punycode/punycode.cc index 39e47d8867..2723fab75c 100644 --- a/src/core/punycode/punycode.cc +++ b/src/core/punycode/punycode.cc @@ -318,10 +318,17 @@ auto utf8_to_punycode(std::istream &input, std::ostream &output, auto punycode_to_utf8(std::istream &input, std::ostream &output, const std::size_t maximum_encoded_bytes) -> void { + // The bound is held to while the stream is read rather than once it has all + // arrived, so that what a caller refuses is never taken in to begin with + const auto bound{ + std::min(maximum_encoded_bytes, PUNYCODE_MAXIMUM_ENCODED_BYTES)}; std::string encoded; char character{0}; while (input.get(character)) { encoded.push_back(character); + if (encoded.size() > bound) { + throw PunycodeError("Input is too large"); + } } std::u32string decoded; diff --git a/test/diff/diff_line_myers_test.cc b/test/diff/diff_line_myers_test.cc index c360c6932e..9dea4dceac 100644 --- a/test/diff/diff_line_myers_test.cc +++ b/test/diff/diff_line_myers_test.cc @@ -1240,3 +1240,34 @@ TEST(deleted_run_slides_along_a_repeated_line) { "-S\n" " t\n"); } + +// A middle snake can leave a sub-problem whose own head or tail still matches, +// since a forward search never looks behind where its snake starts and a +// backward search never looks past where its snake ends, so each recursion has +// to trim for itself rather than trusting what the caller trimmed +TEST(recursion_left_with_a_shared_head) { + const auto result{sourcemeta::core::diff( + "a\n", "b\na\na\nb\n", sourcemeta::core::Diff::Mode::Line, + sourcemeta::core::Diff::Algorithm::Myers)}; + EXPECT_EQ(render(result, 1), "--- a\n" + "+++ b\n" + "@@ -1 +1,4 @@\n" + "+b\n" + "+a\n" + " a\n" + "+b\n"); +} + +TEST(recursion_left_with_a_shared_tail) { + const auto result{sourcemeta::core::diff( + "a\na\nb\n", "c\na\nb\nd\n", sourcemeta::core::Diff::Mode::Line, + sourcemeta::core::Diff::Algorithm::Myers)}; + EXPECT_EQ(render(result, 1), "--- a\n" + "+++ b\n" + "@@ -1,3 +1,4 @@\n" + "+c\n" + " a\n" + "-a\n" + " b\n" + "+d\n"); +} diff --git a/test/jsonpath/jsonpath_parse_test.cc b/test/jsonpath/jsonpath_parse_test.cc index fbc9beea42..a6074e35af 100644 --- a/test/jsonpath/jsonpath_parse_test.cc +++ b/test/jsonpath/jsonpath_parse_test.cc @@ -590,3 +590,10 @@ TEST(jsonpath_parse_number_literal_past_every_representable_value) { TEST(jsonpath_parse_function_name_without_parentheses) { EXPECT_JSONPATH_PARSE_ERROR("$[?!length]", 11); } + +// The I-JSON range above is a bound on the value, which a literal of zero +// passes however large an exponent it is spelled with, leaving a scale that no +// decimal can carry for the reading itself to turn down +TEST(jsonpath_parse_zero_with_an_exponent_past_every_scale) { + EXPECT_JSONPATH_PARSE_ERROR("$[?@.a == 0e2147483648]", 23); +} diff --git a/test/markdown/markdown_pathological_test.cc b/test/markdown/markdown_pathological_test.cc index 8c5e6e36b6..288c967c7d 100644 --- a/test/markdown/markdown_pathological_test.cc +++ b/test/markdown/markdown_pathological_test.cc @@ -386,10 +386,12 @@ TEST(input_past_the_bound_the_caller_allows_when_unsafe) { // The replacement a malformed byte sequence goes through stands in for it // before the bound is read, so what the bound counts is what the parser is -// handed rather than what the caller wrote +// handed rather than what the caller wrote. Two bytes arrive and four are +// parsed, so a bound of two passes on what was written and fails on what the +// repair made of it TEST(input_past_the_bound_the_caller_allows_after_replacement) { try { - sourcemeta::core::markdown_to_html("a\xFF", true, 1); + sourcemeta::core::markdown_to_html("a\xFF", true, 2); FAIL(); } catch (const sourcemeta::core::MarkdownError &error) { EXPECT_STREQ(error.what(), diff --git a/test/oidc/oidc_id_token_test.cc b/test/oidc/oidc_id_token_test.cc index f8bd19bf93..c9ef2b7703 100644 --- a/test/oidc/oidc_id_token_test.cc +++ b/test/oidc/oidc_id_token_test.cc @@ -1435,15 +1435,15 @@ TEST(validate_rejects_a_required_code_hash_that_does_not_match) { EXPECT_FALSE(identity.has_value()); } -// RFC 7519 Section 2 lets a NumericDate be non-integer, so a value past what a -// timestamp can stand for is read as no authentication time at all rather than -// aborting, which a freshness window then has nothing to measure against -TEST(validate_rejects_an_auth_time_beyond_the_representable_range) { +// A value past what a double stands for cannot be read as one in the first +// place, which is a different refusal from a value that reads but names a time +// the clock cannot hold +TEST(validate_treats_an_auth_time_past_the_double_range_as_absent) { const auto compact{sign_id_token(R"JSON({ "iss": "https://issuer.example", "sub": "user-1", "aud": "client-id", - "auth_time": 1e300, + "auth_time": 1e400, "iat": 1699996400, "exp": 2000000000 })JSON")}; diff --git a/test/uritemplate/uritemplate_expand_uri_test.cc b/test/uritemplate/uritemplate_expand_uri_test.cc index 95ff291f9d..c751d3b6eb 100644 --- a/test/uritemplate/uritemplate_expand_uri_test.cc +++ b/test/uritemplate/uritemplate_expand_uri_test.cc @@ -946,7 +946,7 @@ TEST(reserved_expansion_of_a_percent_sign_with_no_hexadecimal) { EXPECT_EQ(result, "a%25zb"); } -// RFC 6570 Section 3.2.9 writes an exploded named expansion as a repetition of +// RFC 6570 Section 3.2.8 writes an exploded named expansion as a repetition of // "name=value", so a member that is the empty string still contributes its // name and the separator that would have introduced a value TEST(list_explode_query_with_an_empty_member) { From 0e95b2067a80c41d678f278dcfe807174b947404 Mon Sep 17 00:00:00 2001 From: Juan Cruz Viotti Date: Tue, 13 Oct 2026 16:07:50 -0300 Subject: [PATCH 3/3] More Signed-off-by: Juan Cruz Viotti --- src/core/markdown/markdown.cc | 2 -- src/core/oidc/oidc_id_token.cc | 4 ++-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/core/markdown/markdown.cc b/src/core/markdown/markdown.cc index d4ef004d2d..37e1340192 100644 --- a/src/core/markdown/markdown.cc +++ b/src/core/markdown/markdown.cc @@ -11,8 +11,6 @@ #include // std::min #include // std::size_t -#include // std::uint32_t -#include // std::numeric_limits #include // std::optional #include // std::string #include // std::string_view diff --git a/src/core/oidc/oidc_id_token.cc b/src/core/oidc/oidc_id_token.cc index a7abd0bf7d..01a18db5da 100644 --- a/src/core/oidc/oidc_id_token.cc +++ b/src/core/oidc/oidc_id_token.cc @@ -8,9 +8,9 @@ #include // std::chrono::system_clock, std::chrono::seconds, std::chrono::duration, std::chrono::duration_cast #include // std::int64_t -#include // std::optional +#include // std::optional, std::nullopt #include // std::span -#include // std::out_of_range, std::nullopt +#include // std::out_of_range #include // std::string #include // std::string_view