From c9f73ec9c9c25a859a2821724c980c5c3e7ee7f3 Mon Sep 17 00:00:00 2001 From: JasMehta08 Date: Mon, 20 Jul 2026 14:08:58 +0530 Subject: [PATCH 1/2] [ntuple] Add checksum to S3 JSON anchor --- tree/ntuple/src/RPageStorageS3.cxx | 28 ++++++-- tree/ntuple/test/ntuple_storage_s3.cxx | 93 ++++++++++++++++++++++++-- 2 files changed, 113 insertions(+), 8 deletions(-) diff --git a/tree/ntuple/src/RPageStorageS3.cxx b/tree/ntuple/src/RPageStorageS3.cxx index 763516c3df5ae..ecaecfa591c64 100644 --- a/tree/ntuple/src/RPageStorageS3.cxx +++ b/tree/ntuple/src/RPageStorageS3.cxx @@ -21,6 +21,7 @@ #include #include +#include #include #include @@ -31,8 +32,7 @@ using ROOT::Internal::MakeUninitArray; using ROOT::Internal::RNTupleCompressor; -/// Field-by-field equality check across all 14 anchor members. -/// Used to verify round-trip correctness in tests. +/// Field-by-field equality check across all data members. bool ROOT::Experimental::Internal::RNTupleAnchorS3::operator==(const RNTupleAnchorS3 &other) const { return fVersionAnchor == other.fVersionAnchor && fVersionEpoch == other.fVersionEpoch && @@ -45,8 +45,8 @@ bool ROOT::Experimental::Internal::RNTupleAnchorS3::operator==(const RNTupleAnch } /// Serialize the anchor to a pretty-printed JSON string (2-space indent). -/// nlohmann/json handles type conversion, string escaping, and uint64 precision. -/// The output is suitable for direct upload to S3 as the anchor object. +/// The checksum is computed over the compact canonical form of the data fields; +/// the stored JSON uses pretty-printing for readability. std::string ROOT::Experimental::Internal::RNTupleAnchorS3::ToJSON() const { nlohmann::json jsonAnchor; @@ -64,6 +64,9 @@ std::string ROOT::Experimental::Internal::RNTupleAnchorS3::ToJSON() const jsonAnchor["footerOffset"] = fFooterOffset; jsonAnchor["nBytesFooter"] = fNBytesFooter; jsonAnchor["lenFooter"] = fLenFooter; + + auto canonical = jsonAnchor.dump(-1); + jsonAnchor["checksum"] = XXH3_64bits(canonical.data(), canonical.size()); return jsonAnchor.dump(2); } @@ -110,6 +113,23 @@ ROOT::Experimental::Internal::RNTupleAnchorS3::CreateFromJSON(const std::string return R__FAIL("missing or invalid field in S3 anchor: " + std::string(e.what())); } + if (!jsonAnchor.contains("checksum")) + return R__FAIL("missing 'checksum' field in S3 anchor"); + + std::uint64_t storedChecksum; + try { + storedChecksum = jsonAnchor.at("checksum").get(); + } catch (const nlohmann::json::exception &e) { + return R__FAIL("invalid 'checksum' field in S3 anchor: " + std::string(e.what())); + } + + jsonAnchor.erase("checksum"); + auto canonical = jsonAnchor.dump(-1); + auto computedChecksum = XXH3_64bits(canonical.data(), canonical.size()); + + if (storedChecksum != computedChecksum) + return R__FAIL("S3 anchor checksum mismatch"); + return anchor; } diff --git a/tree/ntuple/test/ntuple_storage_s3.cxx b/tree/ntuple/test/ntuple_storage_s3.cxx index a7359f0764138..3a55d1dbf2acc 100644 --- a/tree/ntuple/test/ntuple_storage_s3.cxx +++ b/tree/ntuple/test/ntuple_storage_s3.cxx @@ -122,7 +122,7 @@ TEST(RNTupleAnchorS3, MalformedJson) EXPECT_FALSE(bool(result)); } -TEST(RNTupleAnchorS3, ExtraFieldsIgnored) +TEST(RNTupleAnchorS3, ExtraFieldsDetectedByChecksum) { RNTupleAnchorS3 orig; orig.fUrlTemplate = "${baseurl}/${objid}"; @@ -134,13 +134,13 @@ TEST(RNTupleAnchorS3, ExtraFieldsIgnored) orig.fLenFooter = 600; auto json = orig.ToJSON(); - // Inject an unknown field before the closing brace + // Inject an unknown field; the checksum no longer matches because the reader hashes + // all non-checksum fields, including unknown ones added by tampering. auto pos = json.rfind('}'); json.insert(pos, ",\n \"future_field\": 999"); auto result = RNTupleAnchorS3::CreateFromJSON(json); - ASSERT_TRUE(bool(result)) << result.GetError()->GetReport(); - EXPECT_EQ(orig, result.Inspect()); + EXPECT_FALSE(bool(result)); } TEST(RNTupleAnchorS3, LargeObjectIds) @@ -368,6 +368,90 @@ TEST(RNTupleAnchorS3, MaxUint64Values) EXPECT_EQ(UINT64_MAX, parsed.fLenFooter); } +// ==================== Checksum Tests ==================== + +TEST(RNTupleAnchorS3, ChecksumMismatch) +{ + RNTupleAnchorS3 anchor; + anchor.fHeaderObjId = 42; + anchor.fNBytesHeader = 100; + anchor.fLenHeader = 200; + + auto json = anchor.ToJSON(); + + // Corrupt a data field while keeping the old checksum + auto pos = json.find("\"nBytesHeader\": 100"); + ASSERT_NE(std::string::npos, pos); + json.replace(pos, std::strlen("\"nBytesHeader\": 100"), "\"nBytesHeader\": 999"); + + auto result = RNTupleAnchorS3::CreateFromJSON(json); + EXPECT_FALSE(bool(result)); +} + +TEST(RNTupleAnchorS3, MissingChecksumRejected) +{ + // An anchor without a checksum field must be rejected. + std::string json = R"({ + "anchorVersion": 0, + "formatVersionEpoch": 0, + "formatVersionMajor": 1, + "formatVersionMinor": 0, + "formatVersionPatch": 0, + "urlTemplate": "${baseurl}/${objid}", + "headerObjId": 0, + "headerOffset": 0, + "nBytesHeader": 0, + "lenHeader": 0, + "footerObjId": 0, + "footerOffset": 0, + "nBytesFooter": 0, + "lenFooter": 0 + })"; + + auto result = RNTupleAnchorS3::CreateFromJSON(json); + EXPECT_FALSE(bool(result)); +} + +TEST(RNTupleAnchorS3, ChecksumDeterministic) +{ + RNTupleAnchorS3 anchor; + anchor.fHeaderObjId = 1; + anchor.fFooterObjId = 5; + anchor.fNBytesHeader = 80; + anchor.fLenHeader = 100; + anchor.fNBytesFooter = 150; + anchor.fLenFooter = 200; + + auto json1 = anchor.ToJSON(); + auto json2 = anchor.ToJSON(); + EXPECT_EQ(json1, json2) << "ToJSON must produce identical output for the same data"; + + auto result = RNTupleAnchorS3::CreateFromJSON(json1); + ASSERT_TRUE(bool(result)) << result.GetError()->GetReport(); + auto json3 = result.Inspect().ToJSON(); + EXPECT_EQ(json1, json3); +} + +TEST(RNTupleAnchorS3, WrongChecksumType) +{ + RNTupleAnchorS3 anchor; + auto json = anchor.ToJSON(); + + // Replace the numeric checksum value with a string + auto pos = json.find("\"checksum\":"); + ASSERT_NE(std::string::npos, pos); + auto valStart = json.find(':', pos) + 1; + while (valStart < json.size() && json[valStart] == ' ') + ++valStart; + auto valEnd = valStart; + while (valEnd < json.size() && json[valEnd] != '\n' && json[valEnd] != ',') + ++valEnd; + json.replace(valStart, valEnd - valStart, " \"not_a_number\""); + + auto result = RNTupleAnchorS3::CreateFromJSON(json); + EXPECT_FALSE(bool(result)); +} + // ==================== ParseS3Url Tests ==================== using ROOT::Experimental::Internal::ParseS3Url; @@ -576,6 +660,7 @@ TEST(RPageSinkS3Wire, WriteIssuesExpectedPuts) // The anchor body is the JSON document the reader bootstraps from. EXPECT_NE(std::string::npos, requests.back().fBody.find("\"footerObjId\"")); EXPECT_NE(std::string::npos, requests.back().fBody.find("\"urlTemplate\"")); + EXPECT_NE(std::string::npos, requests.back().fBody.find("\"checksum\"")); } TEST(RPageSinkS3Wire, PutErrorThrows) From e38221d5a4fcf7e83e39d481b41b24b035f05a51 Mon Sep 17 00:00:00 2001 From: JasMehta08 Date: Mon, 20 Jul 2026 14:09:11 +0530 Subject: [PATCH 2/2] [ntuple] Add clone template field to S3 anchor --- tree/ntuple/inc/ROOT/RPageStorageS3.hxx | 3 +++ tree/ntuple/src/RPageStorageS3.cxx | 26 ++++++++++++++++++------- tree/ntuple/test/ntuple_storage_s3.cxx | 6 ++++++ 3 files changed, 28 insertions(+), 7 deletions(-) diff --git a/tree/ntuple/inc/ROOT/RPageStorageS3.hxx b/tree/ntuple/inc/ROOT/RPageStorageS3.hxx index 84eb4f11df0d9..7a82d201299a5 100644 --- a/tree/ntuple/inc/ROOT/RPageStorageS3.hxx +++ b/tree/ntuple/inc/ROOT/RPageStorageS3.hxx @@ -51,6 +51,9 @@ struct RNTupleAnchorS3 { /// ${objid} with the numeric object ID. Defaults to the scheme this writer uses; the reader /// overrides it from the stored anchor. std::string fUrlTemplate = "${baseurl}/${objid}"; + /// Pattern for resolving clone (attribute-set) names to base URLs. + /// ${baseurl} is replaced with the anchor URL, ${name} with the clone name. + std::string fCloneTemplate = "${baseurl}/_clone/${name}"; /// Object ID and byte offset of the compressed header within the S3 object std::uint64_t fHeaderObjId = 0; std::uint64_t fHeaderOffset = 0; diff --git a/tree/ntuple/src/RPageStorageS3.cxx b/tree/ntuple/src/RPageStorageS3.cxx index ecaecfa591c64..da4afb9546b27 100644 --- a/tree/ntuple/src/RPageStorageS3.cxx +++ b/tree/ntuple/src/RPageStorageS3.cxx @@ -38,10 +38,11 @@ bool ROOT::Experimental::Internal::RNTupleAnchorS3::operator==(const RNTupleAnch return fVersionAnchor == other.fVersionAnchor && fVersionEpoch == other.fVersionEpoch && fVersionMajor == other.fVersionMajor && fVersionMinor == other.fVersionMinor && fVersionPatch == other.fVersionPatch && fUrlTemplate == other.fUrlTemplate && - fHeaderObjId == other.fHeaderObjId && fHeaderOffset == other.fHeaderOffset && - fNBytesHeader == other.fNBytesHeader && fLenHeader == other.fLenHeader && - fFooterObjId == other.fFooterObjId && fFooterOffset == other.fFooterOffset && - fNBytesFooter == other.fNBytesFooter && fLenFooter == other.fLenFooter; + fCloneTemplate == other.fCloneTemplate && fHeaderObjId == other.fHeaderObjId && + fHeaderOffset == other.fHeaderOffset && fNBytesHeader == other.fNBytesHeader && + fLenHeader == other.fLenHeader && fFooterObjId == other.fFooterObjId && + fFooterOffset == other.fFooterOffset && fNBytesFooter == other.fNBytesFooter && + fLenFooter == other.fLenFooter; } /// Serialize the anchor to a pretty-printed JSON string (2-space indent). @@ -56,6 +57,7 @@ std::string ROOT::Experimental::Internal::RNTupleAnchorS3::ToJSON() const jsonAnchor["formatVersionMinor"] = fVersionMinor; jsonAnchor["formatVersionPatch"] = fVersionPatch; jsonAnchor["urlTemplate"] = fUrlTemplate; + jsonAnchor["cloneTemplate"] = fCloneTemplate; jsonAnchor["headerObjId"] = fHeaderObjId; jsonAnchor["headerOffset"] = fHeaderOffset; jsonAnchor["nBytesHeader"] = fNBytesHeader; @@ -101,6 +103,7 @@ ROOT::Experimental::Internal::RNTupleAnchorS3::CreateFromJSON(const std::string anchor.fVersionMinor = jsonAnchor.at("formatVersionMinor").get(); anchor.fVersionPatch = jsonAnchor.at("formatVersionPatch").get(); anchor.fUrlTemplate = jsonAnchor.at("urlTemplate").get(); + anchor.fCloneTemplate = jsonAnchor.at("cloneTemplate").get(); anchor.fHeaderObjId = jsonAnchor.at("headerObjId").get(); anchor.fHeaderOffset = jsonAnchor.at("headerOffset").get(); anchor.fNBytesHeader = jsonAnchor.at("nBytesHeader").get(); @@ -320,8 +323,17 @@ std::unique_ptr ROOT::Experimental::Internal::RPageSinkS3::CloneAsHidden(std::string_view name, const ROOT::RNTupleWriteOptions &opts) const { - // The hidden (attribute-set) ntuple is stored under a reserved "_clone" sub-prefix so its objects and - // anchor can never collide with the main ntuple's numeric object keys ($baseurl/0, $baseurl/1, ...). - std::string cloneBaseUrl = fBaseUrl + "/_clone/" + std::string(name); + // Resolve the clone template so the hidden ntuple's objects and anchor live under a sub-prefix + // that cannot collide with the main ntuple's numeric object keys. + std::string cloneBaseUrl = fAnchor.fCloneTemplate; + + auto pos = cloneBaseUrl.find("${baseurl}"); + if (pos != std::string::npos) + cloneBaseUrl.replace(pos, std::strlen("${baseurl}"), fBaseUrl); + + pos = cloneBaseUrl.find("${name}"); + if (pos != std::string::npos) + cloneBaseUrl.replace(pos, std::strlen("${name}"), name); + return std::unique_ptr(new RPageSinkS3(name, cloneBaseUrl, opts, RFromBaseUrl{})); } diff --git a/tree/ntuple/test/ntuple_storage_s3.cxx b/tree/ntuple/test/ntuple_storage_s3.cxx index 3a55d1dbf2acc..855df0d87d6af 100644 --- a/tree/ntuple/test/ntuple_storage_s3.cxx +++ b/tree/ntuple/test/ntuple_storage_s3.cxx @@ -31,6 +31,7 @@ TEST(RNTupleAnchorS3, RoundTrip) orig.fVersionMinor = 2; orig.fVersionPatch = 0; orig.fUrlTemplate = "https://bucket.s3.us-east-1.amazonaws.com/data/${objid}"; + orig.fCloneTemplate = "${baseurl}/clones/${name}"; orig.fHeaderObjId = 1; orig.fHeaderOffset = 0; orig.fNBytesHeader = 1200; @@ -54,6 +55,7 @@ TEST(RNTupleAnchorS3, RoundTrip) EXPECT_EQ(2u, parsed.fVersionMinor); EXPECT_EQ(0u, parsed.fVersionPatch); EXPECT_EQ("https://bucket.s3.us-east-1.amazonaws.com/data/${objid}", parsed.fUrlTemplate); + EXPECT_EQ("${baseurl}/clones/${name}", parsed.fCloneTemplate); EXPECT_EQ(1u, parsed.fHeaderObjId); EXPECT_EQ(0u, parsed.fHeaderOffset); EXPECT_EQ(1200u, parsed.fNBytesHeader); @@ -242,6 +244,10 @@ TEST(RNTupleAnchorS3, Equality) b.fHeaderObjId = 99; EXPECT_FALSE(a == b); + + b = a; + b.fCloneTemplate = "${baseurl}/other/${name}"; + EXPECT_FALSE(a == b); } TEST(RNTupleAnchorS3, ToJSONProducesValidJson)