Skip to content

Fix SimpleOutput leaking annotations of failed branches #926

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jviotti merged 3 commits into from
Jul 16, 2026
Merged

Fix SimpleOutput leaking annotations of failed branches #926

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2a38090
Fix `SimpleOutput` leaking annotations of failed branches
jviotti Jul 15, 2026
5cf3f79
More correctness
jviotti Jul 16, 2026
e5c5c9a
Core
jviotti Jul 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 12 additions & 10 deletions src/output/include/sourcemeta/blaze/output_simple.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,11 @@

#include <sourcemeta/blaze/evaluator.h>

#include <cstddef> // std::size_t
#include <functional> // std::reference_wrapper
// TODO(C++23): Consider std::flat_map/std::flat_set when available in libc++
#include <map> // std::map
#include <ostream> // std::ostream
#include <string> // std::string
#include <utility> // std::pair
#include <utility> // std::move
#include <vector> // std::vector

namespace sourcemeta::blaze {
Expand Down Expand Up @@ -131,16 +130,19 @@ class SOURCEMETA_BLAZE_OUTPUT_EXPORT SimpleOutput {
#if defined(_MSC_VER)
#pragma warning(disable : 4251)
#endif
/// An in-flight branching keyword, along with the number of annotations
/// collected before it started and the error traces it buffers
struct MaskEntry {
sourcemeta::core::WeakPointer evaluate_path;
sourcemeta::core::WeakPointer instance_location;
std::size_t annotations_mark;
std::vector<Entry> buffered_traces;
};

const sourcemeta::core::JSON &instance_;
const sourcemeta::core::WeakPointer base_;
container_type output;
std::vector<
std::pair<sourcemeta::core::WeakPointer, sourcemeta::core::WeakPointer>>
mask;
std::map<
std::pair<sourcemeta::core::WeakPointer, sourcemeta::core::WeakPointer>,
std::vector<Entry>>
masked_traces;
std::vector<MaskEntry> mask;
std::vector<AnnotationEntry> annotations_;
#if defined(_MSC_VER)
#pragma warning(default : 4251)
Expand Down
126 changes: 98 additions & 28 deletions src/output/output_simple.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,32 @@

#include <sourcemeta/blaze/foundation.h>

#include <algorithm> // std::any_of, std::sort
#include <algorithm> // std::any_of, std::equal, std::sort
#include <cassert> // assert
#include <iterator> // std::back_inserter, std::make_move_iterator
#include <iterator> // std::back_inserter, std::make_move_iterator, std::next
#include <ranges> // std::views::reverse
#include <utility> // std::move

namespace {

auto shares_prefix(const sourcemeta::core::WeakPointer &pointer,
const sourcemeta::core::WeakPointer &other,
const std::size_t prefix_size) -> bool {
return pointer.size() >= prefix_size && other.size() >= prefix_size &&
std::equal(pointer.cbegin(),
std::next(pointer.cbegin(),
static_cast<std::ptrdiff_t>(prefix_size)),
other.cbegin());
}

auto starts_with_strict(const sourcemeta::core::WeakPointer &pointer,
const sourcemeta::core::WeakPointer &other) -> bool {
return pointer.size() > other.size() &&
shares_prefix(pointer, other, other.size());
}

} // namespace

namespace sourcemeta::blaze {

SimpleOutput::SimpleOutput(const sourcemeta::core::JSON &instance,
Expand Down Expand Up @@ -46,13 +67,14 @@ auto SimpleOutput::operator()(
const auto &keyword{evaluate_path.back().to_property()};
if (keyword == "anyOf" || keyword == "oneOf" || keyword == "not" ||
keyword == "if" || keyword == "contains") {
this->mask.emplace_back(evaluate_path, instance_location);
this->mask.push_back({.evaluate_path = evaluate_path,
.instance_location = instance_location,
.annotations_mark = this->annotations_.size(),
.buffered_traces = {}});
}
} else if (type == EvaluationType::Post && !this->mask.empty() &&
this->mask.back().first == evaluate_path &&
this->mask.back().second == instance_location) {
const auto mask_key{std::make_pair(evaluate_path, instance_location)};
this->masked_traces.erase(mask_key);
this->mask.back().evaluate_path == evaluate_path &&
this->mask.back().instance_location == instance_location) {
this->mask.pop_back();
}

Expand Down Expand Up @@ -83,27 +105,28 @@ auto SimpleOutput::operator()(
// To ease the output
if (keyword == "anyOf" || keyword == "oneOf" || keyword == "not" ||
keyword == "if" || keyword == "contains") {
this->mask.emplace_back(evaluate_path, instance_location);
this->mask.push_back({.evaluate_path = evaluate_path,
.instance_location = instance_location,
.annotations_mark = this->annotations_.size(),
.buffered_traces = {}});
}
} else if (type == EvaluationType::Post) {
const auto mask_key{std::make_pair(evaluate_path, instance_location)};
const auto mask_it{std::ranges::find(this->mask, mask_key)};
const auto mask_it{std::ranges::find_if(
this->mask, [&](const MaskEntry &mask_entry) -> bool {
return mask_entry.evaluate_path == evaluate_path &&
mask_entry.instance_location == instance_location;
})};
if (mask_it != this->mask.end()) {
// Present unexpected traces only when needed
if (!result && keyword != "not" && keyword != "if") {
auto buffered{this->masked_traces.find(mask_key)};
if (buffered != this->masked_traces.end()) {
#ifdef __cpp_lib_containers_ranges
this->output.append_range(std::move(buffered->second));
this->output.append_range(std::move(mask_it->buffered_traces));
#else
this->output.insert(this->output.end(),
std::make_move_iterator(buffered->second.begin()),
std::make_move_iterator(buffered->second.end()));
this->output.insert(
this->output.end(),
std::make_move_iterator(mask_it->buffered_traces.begin()),
std::make_move_iterator(mask_it->buffered_traces.end()));
#endif
this->masked_traces.erase(buffered);
}
} else {
this->masked_traces.erase(mask_key);
}

this->mask.erase(mask_it);
Expand All @@ -115,19 +138,66 @@ auto SimpleOutput::operator()(
}

if (type == EvaluationType::Post && !this->annotations_.empty()) {
std::erase_if(
this->annotations_, [&](const AnnotationEntry &entry) -> bool {
return entry.evaluate_path.starts_with_initial(evaluate_path) &&
entry.instance_location == instance_location;
});
// A failure inside a branching keyword discards every annotation that
// the failing unit collected, including the ones at instance locations
// below the unit's own instance location. The unit is the disjunction
// branch for `anyOf` and `oneOf`, the application of the subschema to
// the array element for `contains`, and the entire subschema for `if`
// and `not`. Outside of branching keywords, annotations from successful
// sibling subschemas are reported on a best-effort basis, so only
// annotations at the exact instance location of the failure are
// discarded
const MaskEntry *unit{nullptr};
for (const auto &mask_entry : std::views::reverse(this->mask)) {
if (starts_with_strict(evaluate_path, mask_entry.evaluate_path) &&
instance_location.starts_with(mask_entry.instance_location)) {
unit = &mask_entry;
break;
}
}

if (unit) {
const auto &mask_keyword{unit->evaluate_path.back().to_property()};
const auto evaluate_prefix_size{
unit->evaluate_path.size() +
((mask_keyword == "anyOf" || mask_keyword == "oneOf") ? 1 : 0)};
const auto instance_prefix_size{unit->instance_location.size() +
(mask_keyword == "contains" ? 1 : 0)};
// Every annotation the failing unit collected, including the ones
// that the equality check below would match, sits at or beyond the
// mark the unit's branching keyword recorded, so older entries do
// not need to be scanned at all
assert(unit->annotations_mark <= this->annotations_.size());
const auto from{this->annotations_.begin() +
static_cast<std::ptrdiff_t>(unit->annotations_mark)};
this->annotations_.erase(
std::remove_if(
from, this->annotations_.end(),
[&](const AnnotationEntry &entry) -> bool {
return (entry.evaluate_path.starts_with_initial(
evaluate_path) &&
entry.instance_location == instance_location) ||
(shares_prefix(entry.evaluate_path, evaluate_path,

@augmentcode augmentcode Bot Jul 16, 2026 •
edited
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

annotations_ stores effective_evaluate_path (resolved via base_), but this pruning compares entry.evaluate_path against the raw evaluate_path (unresolved). If SimpleOutput is constructed with a matching base_ (e.g. in fail_string_with_matching_base-style usage), this can prevent failed-unit annotation pruning from matching and could reintroduce annotation leaks.

Severity: medium

Fix This in Augment

🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.

Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.
Outdated
Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.
Outdated
evaluate_prefix_size) &&
shares_prefix(entry.instance_location,
instance_location, instance_prefix_size));
}),
this->annotations_.end());
} else {
std::erase_if(
this->annotations_, [&](const AnnotationEntry &entry) -> bool {
return entry.evaluate_path.starts_with_initial(evaluate_path) &&
entry.instance_location == instance_location;
});
}
}

if (keyword == "if") {
return;
} else {
for (const auto &mask_entry : this->mask) {
if (evaluate_path.starts_with(mask_entry.first)) {
this->masked_traces[mask_entry].push_back(
for (auto &mask_entry : this->mask) {
if (evaluate_path.starts_with(mask_entry.evaluate_path)) {
mask_entry.buffered_traces.push_back(
{.message = describe(result, step, evaluate_path, instance_location,
this->instance_, annotation),
.instance_location = instance_location,
Expand Down
91 changes: 91 additions & 0 deletions test/output/output_jsonld_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -391,6 +391,97 @@ TEST(JSONLD_oneof_only_matching_branch_survives) {
EXPECT_JSON_LD_VALUE(schema, instance, expected);
}

TEST(JSONLD_anyof_failed_branch_nested_annotations_dropped) {
const auto schema{sourcemeta::core::parse_json(R"JSON({
"$schema": "https://json-schema.org/draft/2020-12/schema",
"type": "object",
"properties": {
"payment": {
"x-jsonld-id": "https://schema.org/payment",
"anyOf": [
{
"properties": {
"amount": {
"x-jsonld-id": "https://schema.org/wrongPredicate",
"x-jsonld-datatype": "http://www.w3.org/2001/XMLSchema#date"
},
"kind": {
"type": "number",
"minimum": 5,
"maximum": 10,
"multipleOf": 2
}
}
},
{
"properties": {
"amount": { "x-jsonld-id": "https://schema.org/rightPredicate" }
}
}
]
}
}
})JSON")};

const auto instance{sourcemeta::core::parse_json(
R"JSON({ "payment": { "amount": "hi", "kind": 999 } })JSON")};

const auto expected{sourcemeta::core::parse_json(R"JSON([
{
"https://schema.org/payment": [
{
"https://schema.org/rightPredicate": [ { "@value": "hi" } ]
}
]
}
])JSON")};

EXPECT_JSON_LD_VALUE(schema, instance, expected);
}

TEST(JSONLD_anyof_failed_branch_nested_annotations_dropped_cheap_sibling) {
const auto schema{sourcemeta::core::parse_json(R"JSON({
"$schema": "https://json-schema.org/draft/2020-12/schema",
"type": "object",
"properties": {
"payment": {
"x-jsonld-id": "https://schema.org/payment",
"anyOf": [
{
"properties": {
"amount": {
"x-jsonld-id": "https://schema.org/wrongPredicate",
"x-jsonld-datatype": "http://www.w3.org/2001/XMLSchema#date"
},
"kind": { "const": 4 }
}
},
{
"properties": {
"amount": { "x-jsonld-id": "https://schema.org/rightPredicate" }
}
}
]
}
}
})JSON")};

const auto instance{sourcemeta::core::parse_json(
R"JSON({ "payment": { "amount": "hi", "kind": 999 } })JSON")};

const auto expected{sourcemeta::core::parse_json(R"JSON([
{
"https://schema.org/payment": [
{
"https://schema.org/rightPredicate": [ { "@value": "hi" } ]
}
]
}
])JSON")};

EXPECT_JSON_LD_VALUE(schema, instance, expected);
}

TEST(JSONLD_standalone_node_without_edge) {
const auto schema{sourcemeta::core::parse_json(R"JSON({
"$schema": "https://json-schema.org/draft/2020-12/schema",
Expand Down
Loading
Loading