Repository navigation
Link to CLA and document min version of tools-deps - #2
Merged
Merged
Conversation
Contributor
|
CLA link probably makes more sense to link to: https://cla-assistant.io/fluree/ |
ldw1007
approved these changes
Oct 22, 2020
dpetran
added a commit
that referenced
this pull request
May 17, 2024
Three approaches considered: 1) just document that :values needs to appear first 2) discard solutions that don't match the given solution 3) during the parse step, sort the patterns so :values is first I don't like #1, though that's the most expedient. #2 is a bit wasteful as it forces us to generate useless solutions. #3 is doable, and may be a minimal requirement in basic query planning. This implements solution #2.
dpetran
added a commit
that referenced
this pull request
May 31, 2024
Three approaches considered: 1) just document that :values needs to appear first 2) discard solutions that don't match the given solution 3) during the parse step, sort the patterns so :values is first I don't like #1, though that's the most expedient. #2 is a bit wasteful as it forces us to generate useless solutions. #3 is doable, and may be a minimal requirement in basic query planning. This implements solution #2.
dpetran
added a commit
that referenced
this pull request
Jun 12, 2024
Three approaches considered: 1) just document that :values needs to appear first 2) discard solutions that don't match the given solution 3) during the parse step, sort the patterns so :values is first I don't like #1, though that's the most expedient. #2 is a bit wasteful as it forces us to generate useless solutions. #3 is doable, and may be a minimal requirement in basic query planning. This implements solution #2.
JaceRockman
pushed a commit
that referenced
this pull request
Jul 2, 2024
Three approaches considered: 1) just document that :values needs to appear first 2) discard solutions that don't match the given solution 3) during the parse step, sort the patterns so :values is first I don't like #1, though that's the most expedient. #2 is a bit wasteful as it forces us to generate useless solutions. #3 is doable, and may be a minimal requirement in basic query planning. This implements solution #2.
Closed
5 tasks done
5 tasks
bplatz
added a commit
that referenced
this pull request
May 17, 2026
… diagnostics, nits Selected items from the additional review feedback. Each call explained. **#1 — Retry-fragility comment on `take(opts.shapes)`.** No retry exists on the staging path today, but `take()` moves the inline SHACL JSON off the txn, so a future retry would silently skip inline-shapes validation on the second attempt. Hard invariant comment added at the move site; if retry is ever added, defer the take until after `stage_txn` returns successfully. **#3 — `UnsupportedFeature` errors no longer carry an empty `ledger_id`.** `resolve_graph_ref` rejected `f:atT` / `f:trustPolicy` / `f:rollbackGuard` *before* verifying that the ref was actually cross-ledger, so a misrouted same-ledger ref hitting this path would have produced a confusing error with `ledger_id: ""`. Reordered: the same-ledger-ref guard runs first, binding `raw_ledger_ref` once; the unsupported-feature checks now carry the real ledger id in every diagnostic. **#4 — `f:atT` rejection breadth: audited, no gap.** All five `f:GraphRef`-shaped predicates already reject `f:atT` at their same-ledger entry points: - `policy_builder::resolve_policy_source_g_ids` - `tx::resolve_shapes_source_g_ids` (SHACL) - `ontology_imports::resolve_schema_bundle` - `tx::resolve_constraint_source_g_ids_for` - `view::fluree_ext::resolve_local_rules_source_g_id` Plus the cross-ledger `resolver` (now with non-empty ledger_id per #3). Audit-only change; no code modification beyond the doc note. **#2 — Cross-ledger policy target IRI loss is now logged.** `wire_to_restrictions` previously `filter_map`'d unresolvable target IRIs and `for_classes` IRIs silently. A cross-ledger policy could quietly narrow its scope on D if M referenced IRIs D had never registered. Two `tracing::warn!` calls now surface both cases with `restriction_id`, requested count, and resolved count — operators can spot the silent narrowing in logs. Kept the restriction even when targets become empty (rather than dropping the whole restriction): same-ledger via `load_policy_restriction` also produces empty target sets for the same IRI-not-in-snapshot scenario, and dropping the restriction would diverge cross-ledger from same-ledger semantics. The warn log is the parity-preserving signal. `fluree-db-policy` gains a `tracing` dependency for this. **Nits taken:** - `TAtUnavailable` doc-commented as Phase 3 reserved (unreachable today because `UnsupportedFeature { feature: "f:atT" }` fires first). - Empty `uniqueProperties: []` over HTTP is now documented as an intentional "no inline constraints" treatment, not a silent bug. - Cache read/write race in `resolver` documented as benign: losers materialize the same value structurally; single-flight is a future optimization, not a correctness need. - `f:AccessPolicy` IRI promoted from local `const` / duplicated-literal to `fluree_vocab::policy_iris::ACCESS_POLICY`. Two call sites updated; nothing else used the local consts. **Nits skipped per "don't make changes that don't seem needed":** - HTTP-level negative test for malformed `opts.shapes` / `uniqueProperties` (#5) — unit-level coverage exists; HTTP harness setup cost not justified. - Same-ledger `rules_source_g_id` end-to-end test (#6) — already covered by `it_rules_source::rules_source_in_named_graph_is_honored`, which runs an actual query against the named-graph rule. - DynamoDB hard-drop failure handling (#7) — storage layer, unrelated to this PR. - Duplicate pattern match in `shapes_materializer` — cosmetic. - `snapshot.ledger_id` redundancy in inline-shapes / inline- ontology `txn_id` — cosmetic. - `is_last_live_branch` WARN log — unrelated path.
mwatts
pushed a commit
to mwatts/fluree-db
that referenced
this pull request
Jun 12, 2026
Addresses four review findings against the multi-query handler and snapshot application. Bearer ledger scope (High fluree#1): The handler now walks the validated distinct_ledgers set after validation and rejects with 404 (existence-leak avoidance) if any ledger is outside the bearer's read scope. Matches the single-query /query and /query/:ledger behavior exactly: an unsigned bearer that can't read one of the envelope's ledgers gets the whole envelope rejected, not partial results that would reveal whether the ledger exists. Signed requests bypass this check, same as single-query. Identity + default policy-class threading (High fluree#2): The handler builds a MultiQueryIdentityContext containing the effective bearer identity (signed DID wins over bearer identity) and the server-configured default policy class, threaded through to the dispatcher. The dispatcher applies it per JSON-LD sub-query via apply_auth_identity_to_opts — the same code path the single-query JSON-LD handler uses, including the root-identity impersonation semantic. The impersonation check binds to each sub-query's primary ledger (first entry of its from clause), conservative for multi- ledger sub-queries. Headers are now injected into envelope opts BEFORE validation runs, so a client supplying max-fuel or maxConcurrency via fluree-* headers hits the same envelope-level rejections the body opts get. The fluree-* headers (policy-class, policy, policy-values, identity, default-allow, max-fuel) ride on the envelope opts and merge into every sub-query's opts as defaults via merged_opts — sub-query opts still win on key conflict, matching single-query behavior. SPARQL identity threading is explicitly deferred: the existing connection-scoped SPARQL path (/query with SPARQL FROM clauses) also doesn't currently thread identity. v1 multi-query matches that for parity; the docs flag this as a v1 limitation so the two paths can land identity threading together. SPARQL fragment-aware snapshot pinning (Medium fluree#1): apply_snapshot_to_sparql previously skipped any IRI whose value differed from its bare-ledger form via the value_str != bare check, which fired on both real temporal pins AND on named-graph fragments (ledger#txn-meta). So an envelope-pinned SPARQL like FROM <mq:frag#txn-meta> would run against current head instead of the envelope's t. Fixed by gating the skip on actual temporal-marker presence (has_temporal_marker checks for @t: / @iso: / @commit:), and splicing @t:N BEFORE any fragment when rewriting: <ledger#txn-meta> -> <ledger@t:42#txn-meta>. The dataset parser separates fragment from temporal suffix and reattaches the fragment after time-spec resolution, so the spliced form is what it expects. Per-sub-query response size cap (Medium fluree#4): DispatchConfig now carries max_subquery_response_bytes. Each sub-query's serialized result is sized once after dispatch returns; if a single alias's result exceeds the cap it's downgraded to an error outcome (code: "response_too_large") and the data dropped before assembly. This catches a single runaway query before it adds to envelope-wide memory pressure. The envelope-level cap in assemble_response is still the strict guarantee; the per-sub-query cap is the early-exit. The doc now spells out honestly that this is best-effort, not an OOM guard — peak memory is bounded by max_concurrency * max_subquery_response_bytes, not the envelope cap alone — and per-sub-query streaming serialization is queued for v1.1. Tests: - 4 new auth integration tests (multi_query_auth_integration.rs): unauthenticated request when data auth required returns 401, in-scope bearer succeeds, out-of-scope JSON-LD returns 404, out-of-scope SPARQL returns 404. - 1 new fragment-ledger SPARQL integration test confirms a SPARQL FROM <ledger#txn-meta> envelope alias runs successfully (regression against the unpinned-fragment bug). - 2 new unit tests cover the SPARQL fragment splicing (apply_snapshot_to_sparql) for both FROM and FROM NAMED clauses. 47 server integration + 21 policy + 4 multi-query auth + 16 multi-query core + 73 envelope/snapshot/dispatcher unit tests all green.
aaj3f
added a commit
that referenced
this pull request
Jul 2, 2026
Adversarial review of the boundary check in fix #2: a request-supplied catalog_uri / oauth2_token_url that PASSES a string/IP check can still reach an internal host, because reqwest follows redirects by default (a public URL 3xx's to http://169.254.169.254/...) and re-resolves DNS at connect time (a name rebinds public->private after the check). A boundary check alone is bypassable. Move the authoritative enforcement to the client/connect layer, in a new `fluree_db_iceberg::net` module: - hardened_client_builder(): a reqwest client that follows NO redirects and resolves through an SsrfGuardResolver which denylists the resolved IPs and hands the connector only the checked addresses (pinning - no resolve-then- reconnect TOCTOU window, so DNS-rebinding is blocked). Wired into the OAuth2 token client (auth/oauth2.rs) and the REST catalog client (catalog/rest.rs) - the two reqwest sinks. - The S3 sink (io/storage.rs, AWS SDK) validates its endpoint override before use. Because MinIO / LocalStack legitimately run on loopback/private hosts, s3_endpoint gets a NARROWER metadata-only block (still refuses 169.254/16 and unspecified) rather than the full internal denylist. The route boundary keeps a cheap up-front reject (validate_public_url) for literal-IP catalog_uri/oauth2_token_url - which the resolver never sees, since reqwest connects to IP literals directly - covering decimal/octal/hex-encoded IPv4 (the URL parser normalizes these) and bracketed IPv6. The server delegates to the api (guard_iceberg_connection_urls); the IP-classification tests live with the module in the iceberg crate. The denylist covers loopback, RFC1918 private, link-local (incl. the metadata address), CGNAT shared, unspecified/broadcast, and IPv6 loopback/unique-local/ link-local plus v4-mapped. Unauthenticated by default, so enforced regardless of auth mode. Refs fluree/solo#724
aaj3f
added a commit
that referenced
this pull request
Jul 2, 2026
…e_namespace Adversarial re-review completing comment #2. The earlier fix escaped curie() + the body literals, but the render header (`@base <...>` / `@prefix v: <...>`) bypasses curie() and emitted its IRIs raw. The vocab prefix's namespace IS the request-controlled `base_namespace` (routes/iceberg.rs -> iceberg_generate.rs), so a newline in it injected arbitrary Turtle statements. - render.rs: wrap map_document_base + each prefix namespace in escape_iri, so the header IRIs are IRIREF-escaped like the rest (no-op for ordinary values -> golden unchanged). - iceberg_generate.rs: validate base_namespace is an absolute IRI with no control chars / spaces, rejected with a clean 400 (defense in depth). Tests: a base_namespace carrying a newline + a crafted `@prefix evil:` / triple round-trips to exactly one TriplesMap with no raw-newline injection; the api rejects a control-char / scheme-less base_namespace and accepts a valid one. Refs fluree/solo#724
aaj3f
added a commit
that referenced
this pull request
Jul 2, 2026
- Redirect test (comment #2): assert the SSRF-hardened reqwest client (redirect::Policy::none, added earlier) does NOT follow a 302 - the redirect- bounce vector a boundary URL check cannot see. - Two more secret Debug leaks the ConfigValue root-cause did not cover (parallel runtime types holding post-resolve() plaintext): BearerTokenAuth.token (auth/bearer.rs) and VendedCredentials.secret_access_key / session_token (credential/vended.rs) now have manual redacting Debug. - IP classifier completeness: block 0.0.0.0/8 ("this network") and ::/96 IPv4-compatible IPv6 (embeds an IPv4 in the low 32 bits -> checked as that IPv4). Refs fluree/solo#724
aaj3f
added a commit
that referenced
this pull request
Jul 6, 2026
Address the review's findings — all verified real; all make the harness's green trustworthy rather than changing what it covers: - (review #1) the 'no unexpected named graph' guard was dead code: the engine binds GRAPH ?g as a plain literal, so the Iri-only filter in list_named_graphs always produced []. Accept literal and IRI bindings (excluding the alias-named default graph) so the guard survives the eventual engine fix. Verified live: an update leaving a stray graph with no expected graphs now fails with 'Unexpected non-empty named graph'. - (review #2) bail when a manifest yields zero tests — a submodule restructure or manifest-parser regression must not report green. - (review #3) a registered test that dies by timeout/subprocess crash now hard-fails: the register excuses a known wrong answer, not an infra death masking a new hang or panic. - (review #4) UpdateEvaluationTest now rejects an mf:result blank node exposing none of ut:data/ut:graphData/ut:result, instead of degrading to a trivially satisfiable 'expected empty store'. - (review #5) register entries matching no discovered test now fail the suite, so dead entries (typos, upstream renames) cannot accumulate. - notes: tightened the TSV bare-integer heuristic to a single optional leading sign; documented the line-based directive-hoisting assumption; fixed the stale 'CI runs make ci' Makefile comment. Full suite remains green: 36 suites, ~1420 tests, 0 failed, 0 ignored.
aaj3f
added a commit
that referenced
this pull request
Jul 12, 2026
Four additions, one per open review item: - Multi-op atomicity on mid-request failure (review blocking #4): op 1 INSERT DATA stages cleanly, op 2 fails at staging while evaluating over op 1's data (BIND(STRDT(?m, <bad:datatype>)) — proving op 2 ran against the sequential state); the request errors as a unit, 't' is unchanged, and op 1 leaves no trace. - UPDATE BASE resolution at the seam (review blocking #2): the same BASE-carrying document stores and finds identical absolute IRIs on the update and query surfaces, GRAPH names included. - DELETE WHERE label independence at the seam (review blocking #5): label reuse across DELETE WHERE ops executes, each op deleting its own matches. - Indexed EXISTS-with-GRAPH-?g (review blocking #3): the GraphVarCorrelated strategy driven end-to-end on a binary-indexed ledger — including the string-literal back-compat — which no memory-backed suite (W3C harness included) ever exercised. On the #1443 EncodedSid/EncodedLit extraction arms specifically, the audit went one step past the review's finding: no current plan shape delivers a still-encoded binding to either extraction site (probed: plain join, OPTIONAL, FILTER EXISTS, UNION, string variant — upstream operators materialize batches first). The arms stay as defense-in-depth with their comment rewritten to say exactly that (this also retires the stale 'never on a scan hot path' wording the review flagged).
aaj3f
added a commit
that referenced
this pull request
Jul 13, 2026
…1473) Every SPARQL entry method re-lexed the query string: query_with_options built the within-ledger dataset (parse #1) and then lowered it (parse #2); the FROM-default single-ledger route parsed a third time inside the dataset impl to test for a dataset clause. That is 2x for all SPARQL and up to 3x on the FROM path. Parse + validate once, then reuse the AST for both dataset-clause resolution and IR lowering. helpers gains lower_sparql_ast (inject-prefixes-then-lower from an owned AST), inject_default_prefixes, and sparql_ast_has_dataset; parse_sparql_to_ir is now a thin parse-then-lower wrapper. build_within_ledger _dataset_from_ast resolves the clause from an already-parsed AST. The four SPARQL entry methods (query_with_options, query_tracked_with_options, query_tracked_with_r2rml_options, query_view_with_r2rml_options) parse once and thread the owned AST into the dataset path via new prepared entries on query_dataset_with_options / query_dataset_tracked_with_options (their public signatures, with many external callers, stay unchanged). Behavior is identical. The boxed-future boundary that breaks the mutual- recursion Send auto-trait cycle is preserved: only owned data (the AST) is threaded through it, never opaque async recursion. A cfg(test) parse counter plus helpers unit tests assert the single-parse guarantee by construction; the existing dataset/tracked/r2rml suites and the W3C testsuite cover behavioral equivalence. No register change.
aaj3f
added a commit
that referenced
this pull request
Jul 14, 2026
…1473) Every SPARQL entry method re-lexed the query string: query_with_options built the within-ledger dataset (parse #1) and then lowered it (parse #2); the FROM-default single-ledger route parsed a third time inside the dataset impl to test for a dataset clause. That is 2x for all SPARQL and up to 3x on the FROM path. Parse + validate once, then reuse the AST for both dataset-clause resolution and IR lowering. helpers gains lower_sparql_ast (inject-prefixes-then-lower from an owned AST), inject_default_prefixes, and sparql_ast_has_dataset; parse_sparql_to_ir is now a thin parse-then-lower wrapper. build_within_ledger _dataset_from_ast resolves the clause from an already-parsed AST. The four SPARQL entry methods (query_with_options, query_tracked_with_options, query_tracked_with_r2rml_options, query_view_with_r2rml_options) parse once and thread the owned AST into the dataset path via new prepared entries on query_dataset_with_options / query_dataset_tracked_with_options (their public signatures, with many external callers, stay unchanged). Behavior is identical. The boxed-future boundary that breaks the mutual- recursion Send auto-trait cycle is preserved: only owned data (the AST) is threaded through it, never opaque async recursion. A cfg(test) parse counter plus helpers unit tests assert the single-parse guarantee by construction; the existing dataset/tracked/r2rml suites and the W3C testsuite cover behavioral equivalence. No register change.
aaj3f
added a commit
that referenced
this pull request
Jul 17, 2026
…flag slice-1.5) The fused fact⋈dim join declined on ANY folded constant-object constraint (star_constraints) — the blanket O1 guard — so a dim flag like `?prod ex:isCurrent true` forced the whole join to materialize the fact. It is the round-2 deployed timeout for the two headline shapes (#1/#2: OrderLine/SupportTicket ⋈ Product with a dim isCurrent flag). E2 replaces the blanket decline with constraint APPLICATION: - dim-side constraints are enforced while building the FK→GKey map — a dim row failing its flag is skipped, so its join key never enters the map and fact rows probing it drop (inner-join + constraint semantics); - fact-side constraints are enforced per fact row in the fold (next_batch); - resolve_star_constraint_checks resolves each (predicate, constant) to a scalar column PredicateObjectMap and DECLINES (falls back to materialize) any constraint that is a RefObjectMap object or absent — never a silent over-count; - the constraint columns are projected into the respective scans; the equality reuses the normal scan's exact primitives (materialize_object_from_batch + object_column_is_numeric + rdf_term_eq_object_constant_cached, now pub(crate)), so the fused constrained count is byte-parity with the materialized answer. The single-table O1 decline is unchanged (slice-1.75). The COUNT(*) manifest shortcut gains an explicit fact_constraints guard (belt-and-suspenders; the join path always groups, so it never coincided). Hermetics: e2_resolve_star_constraint_checks_scalar_vs_ref (scalar admits, ref / missing declines), e2_row_satisfies_boolean_flag (isCurrent true/false/null under the normal-scan equality). Corpus q065 (Order⋈Customer, dim isCurrent flag, group by segment, FROM path): fused == materialized CONSTRAINED oracle, 0 hash mismatches — the join-path over-count trap. Live: fusion-OFF materialize 52s → fusion-ON E2 0.45s (~115x), same constrained count. fluree-db-query lib 1273 green; corpus 64->65; scoped clippy clean.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1.