Diagnose references that lead nowhere and report extra YAML documents

src/core/jsonschema/bundle.cc

@@ -665,6 +665,14 @@ auto embed_references(
       depth == 0 ? default_base : std::string_view{}, remaining};
   charge(remaining, frame);
 
+  // Framing holds the base it resolves against to its canonical spelling, so
+  // every reference destination it reports carries that spelling rather than
+  // the one the caller wrote. Settling it once here is what lets a reference
+  // be compared against it without parsing the base over again for each
+  const JSON::String canonical_base{depth == 0 && !default_base.empty()
+                                        ? URI::canonicalize(default_base)
+                                        : JSON::String{}};
+
   std::vector<std::tuple<JSON, JSON::String, SchemaBaseDialect>> deferred;
   std::vector<std::pair<Pointer, JSON::String>> ref_rewrites;
 
@@ -685,6 +693,16 @@ auto embed_references(
                                  "Could not resolve schema reference");
     }
 
+    // And so is one whose base is the very document being bundled. A wrapper
+    // format puts the schemas it holds below a root that is no schema
+    // resource of its own, so nothing framed answers to the document's base
+    // and the test above passes it over, even though the fragment is all that
+    // leads nowhere
+    if (!canonical_base.empty() && reference.base == canonical_base) {
+      throw SchemaReferenceError(reference.destination, to_pointer(pointer),
+                                 "Could not resolve schema reference");
+    }
+
     if (reference.base.empty()) {
       throw SchemaReferenceError(reference.destination, to_pointer(pointer),
                                  "Could not resolve schema reference");

src/core/jsonschema/frame.cc

@@ -402,6 +402,43 @@ auto canonicalize_pointer_fragment(sourcemeta::core::URI &uri) -> void {
   }
 }
 
+// A static location and a dynamic one of the same spelling are two places, so
+// a URI that names either is looked for under both
+template <typename Locations>
+auto locate(const Locations &locations,
+            const sourcemeta::core::JSON::String &uri)
+    -> std::optional<
+        std::reference_wrapper<const sourcemeta::core::SchemaFrame::Location>> {
+  const auto static_result{
+      locations.find({sourcemeta::core::SchemaReferenceType::Static, uri})};
+  if (static_result != locations.cend()) {
+    return static_result->second;
+  }
+
+  const auto dynamic_result{
+      locations.find({sourcemeta::core::SchemaReferenceType::Dynamic, uri})};
+  if (dynamic_result != locations.cend()) {
+    return dynamic_result->second;
+  }
+
+  return std::nullopt;
+}
+
+// The spelling that every location is framed under and that every reference
+// destination is held to, which is what makes a URI that comes from anywhere
+// else comparable to either of them
+auto canonicalize_reference(const sourcemeta::core::JSON::String &uri)
+    -> std::optional<sourcemeta::core::JSON::String> {
+  try {
+    sourcemeta::core::URI result{uri};
+    result.canonicalize();
+    canonicalize_pointer_fragment(result);
+    return result.recompose();
+  } catch (const sourcemeta::core::URIParseError &) {
+    return std::nullopt;
+  }
+}
+
 auto set_base_and_fragment(sourcemeta::core::SchemaFrame::Reference &entry)
     -> void {
   const std::string_view destination_view{entry.destination};
@@ -2080,19 +2117,24 @@ auto SchemaFrame::traverse(
 auto SchemaFrame::traverse(const std::string_view uri) const
     -> std::optional<std::reference_wrapper<const Location>> {
   const sourcemeta::core::JSON::String uri_string{uri};
-  const auto static_result{
-      this->locations_.find({SchemaReferenceType::Static, uri_string})};
-  if (static_result != this->locations_.cend()) {
-    return static_result->second;
-  }
-
-  const auto dynamic_result{
-      this->locations_.find({SchemaReferenceType::Dynamic, uri_string})};
-  if (dynamic_result != this->locations_.cend()) {
-    return dynamic_result->second;
+  const auto direct{locate(this->locations_, uri_string)};
+  if (direct.has_value()) {
+    return direct;
+  }
+
+  // Framing writes every location down under the canonical spelling of its
+  // URI, and holds the destination of every reference to that same spelling,
+  // so one that reaches here spelled any other way names what a reference
+  // spelled identically would reach. RFC 3986 Section 6.2.2.2 leaves a
+  // percent-encoded reserved character where it stands, and RFC 6901 Section
+  // 6 only asks for the characters a fragment disallows to be encoded, so one
+  // pointer has more than one spelling to begin with
+  const auto canonical{canonicalize_reference(uri_string)};
+  if (!canonical.has_value() || canonical.value() == uri_string) {
+    return std::nullopt;
   }
 
-  return std::nullopt;
+  return locate(this->locations_, canonical.value());
 }
 
 auto SchemaFrame::traverse(const sourcemeta::core::WeakPointer &pointer) const

src/core/openapi/bundle.cc

@@ -55,20 +55,36 @@ struct OpenAPIPending {
 };
 
 // What the description reaches for and does not hold. A reference that names
-// the document being read and lands nowhere is left alone, as OpenAPI
-// Specification 3.1.1, Section 4.8.23 holds a `$ref` to the form of a URI and
-// says nothing about it having to resolve, so there is nothing to fetch here
-// and nothing this specification lets us report
+// the document being read and lands nowhere is settled here rather than
+// collected, as there is no document left to fetch that could supply the
+// place it named. Nothing asks for that. OpenAPI Specification 3.1.1, Section
+// 4.8.23 holds a `$ref` to the form of a URI and says nothing about it having
+// to resolve, so turning one of these down is a choice this makes rather than
+// a rule it follows.
+//
+// What makes the choice is that nothing is left to wait for. The one restraint
+// this specification writes down is Section 4.3.1's "requirement to parse
+// complete documents before deeming a Schema Object reference to be
+// unresolvable", and a Schema Object reference never reaches here, so holding
+// back would be waiting on a document that no reference named. Saying nothing
+// instead would hand back a result that the very same reading calls
+// incomplete, while a reference into another document that lands nowhere is
+// already turned down once that document has been read
 auto pending(const sourcemeta::core::OpenAPIWalk &walk)
     -> std::vector<OpenAPIPending> {
   std::vector<OpenAPIPending> result;
   for (const auto &entry : walk.references) {
-    if (walk.locations.contains(entry.second.destination) ||
-        sourcemeta::core::openapi_within_document(entry.second.destination,
-                                                  walk.base)) {
+    if (walk.locations.contains(entry.second.destination)) {
       continue;
     }
 
+    if (sourcemeta::core::openapi_within_document(entry.second.destination,
+                                                  walk.base)) {
+      throw sourcemeta::core::OpenAPIReferenceError{
+          walk.base, entry.second.origin, entry.second.destination,
+          "This reference must name a place within the document it points at"};
+    }
+
     result.push_back({.origin = entry.second.origin,
                       .destination = entry.second.destination,
                       .expected = entry.second.expected,
@@ -83,12 +99,18 @@ auto pending(const sourcemeta::core::OpenAPIWalk &walk)
   // references above only because a single one of those Objects may name
   // several schemes, which is more than one entry keyed by where it sits
   for (const auto &entry : walk.security_references) {
-    if (walk.locations.contains(entry.second.destination) ||
-        sourcemeta::core::openapi_within_document(entry.second.destination,
-                                                  walk.base)) {
+    if (walk.locations.contains(entry.second.destination)) {
       continue;
     }
 
+    if (sourcemeta::core::openapi_within_document(entry.second.destination,
+                                                  walk.base)) {
+      throw sourcemeta::core::OpenAPIReferenceError{
+          walk.base, entry.second.origin, entry.second.destination,
+          "This security requirement must name a Security Scheme Object "
+          "within the document it points at"};
+    }
+
     result.push_back({.origin = entry.second.origin,
                       .destination = entry.second.destination,
                       .expected = entry.second.expected,

src/core/openapi/include/sourcemeta/core/openapi.h

@@ -996,9 +996,10 @@ using OpenAPIResolverResult = OwnedOrReference<JSON>;
 
 /// @ingroup openapi
 /// How bundling reaches the other documents that an OpenAPI Description is
-/// split across. Every document handed back must itself be an OpenAPI
-/// Description, and a URI that names nothing is reported by handing back no
-/// value. For example:
+/// split across. A URI that names nothing is reported by handing back no
+/// value, and whatever else a URI names is handed back as it stands, for
+/// bundling to decide whether it is a document a description may span. For
+/// example:
 ///
 /// ```cpp
 /// #include <sourcemeta/core/json.h>

src/core/yaml/include/sourcemeta/core/yaml.h

@@ -70,6 +70,26 @@ auto parse_yaml(std::basic_istream<JSON::Char, JSON::CharTraits> &stream)
 SOURCEMETA_CORE_YAML_EXPORT
 auto parse_yaml(const JSON::String &input) -> JSON;
 
+/// @ingroup yaml
+///
+/// Create a JSON document from a YAML string, reporting whether the string
+/// held more documents after the one that was read. For example:
+///
+/// ```cpp
+/// #include <sourcemeta/core/json.h>
+/// #include <sourcemeta/core/yaml.h>
+///
+/// #include <cassert>
+///
+/// bool more_documents{false};
+/// const sourcemeta::core::JSON document =
+///   sourcemeta::core::parse_yaml("hello: world\n---\nsecond: document",
+///                                more_documents);
+/// assert(more_documents);
+/// ```
+SOURCEMETA_CORE_YAML_EXPORT
+auto parse_yaml(const JSON::String &input, bool &more_documents) -> JSON;
+
 /// @ingroup yaml
 ///
 /// Read a JSON document from a file location that represents a YAML file. The
@@ -92,6 +112,28 @@ auto parse_yaml(const JSON::String &input) -> JSON;
 SOURCEMETA_CORE_YAML_EXPORT
 auto read_yaml(const std::filesystem::path &path) -> JSON;
 
+/// @ingroup yaml
+///
+/// Read a JSON document from a file location that represents a YAML file,
+/// reporting whether the file held more documents after the one that was
+/// read. For example:
+///
+/// ```cpp
+/// #include <sourcemeta/core/json.h>
+/// #include <sourcemeta/core/yaml.h>
+///
+/// #include <cassert>
+/// #include <filesystem>
+///
+/// bool more_documents{false};
+/// const std::filesystem::path path{"test.yaml"};
+/// const sourcemeta::core::JSON document =
+///   sourcemeta::core::read_yaml(path, more_documents);
+/// assert(!more_documents);
+/// ```
+SOURCEMETA_CORE_YAML_EXPORT
+auto read_yaml(const std::filesystem::path &path, bool &more_documents) -> JSON;
+
 /// @ingroup yaml
 ///
 /// Parse a YAML document from a C++ standard input stream into an existing
@@ -113,6 +155,18 @@ SOURCEMETA_CORE_YAML_EXPORT
 auto parse_yaml(const JSON::String &input, JSON &output,
                 const JSON::ParseCallback &callback) -> void;
 
+/// @ingroup yaml
+///
+/// Parse a YAML string into an existing JSON value, invoking the given
+/// callback during parsing and reporting whether the string held more
+/// documents after the one that was read. The result is constructed directly
+/// into the given reference rather than returned by value to ensure that
+/// references passed through the parse callback remain valid after parsing
+/// completes.
+SOURCEMETA_CORE_YAML_EXPORT
+auto parse_yaml(const JSON::String &input, bool &more_documents, JSON &output,
+                const JSON::ParseCallback &callback) -> void;
+
 /// @ingroup yaml
 ///
 /// Read a YAML file into an existing JSON value, invoking the given callback
@@ -123,6 +177,17 @@ SOURCEMETA_CORE_YAML_EXPORT
 auto read_yaml(const std::filesystem::path &path, JSON &output,
                const JSON::ParseCallback &callback) -> void;
 
+/// @ingroup yaml
+///
+/// Read a YAML file into an existing JSON value, invoking the given callback
+/// during parsing and reporting whether the file held more documents after
+/// the one that was read. The result is constructed directly into the given
+/// reference rather than returned by value to ensure that references passed
+/// through the parse callback remain valid after parsing completes.
+SOURCEMETA_CORE_YAML_EXPORT
+auto read_yaml(const std::filesystem::path &path, bool &more_documents,
+               JSON &output, const JSON::ParseCallback &callback) -> void;
+
 /// @ingroup yaml
 ///
 /// Read a JSON document from a file location that represents a YAML file or a

src/core/yaml/parser.h

@@ -237,7 +237,12 @@ class Parser {
     }
   }
 
-  auto validate_end_of_stream() -> void {
+  // Whether anything stood after the document that was read, which is what a
+  // caller holding the whole input has no other way of learning. YAML 1.2.2
+  // Section 9.2 makes a stream "zero or more documents", so one that carries
+  // more than the single document handed back is well formed rather than
+  // malformed, and saying so is all there is to report
+  auto validate_end_of_stream() -> bool {
     // Whatever stands after the document that was read is checked rather than
     // handed over, so nothing of it is announced to whoever is listening for
     // the one document the caller gets
@@ -258,7 +263,7 @@ class Parser {
     bool saw_document_end{this->document_ended_};
     this->skip_document_ends(token, saw_document_end);
     if (token.type == TokenType::StreamEnd) {
-      return;
+      return false;
     }
     while (token.type != TokenType::StreamEnd) {
       // Section 9.2: a document that is not terminated by a document end
@@ -271,7 +276,7 @@ class Parser {
         opened_document = true;
         token = this->next_token();
         if (token.type == TokenType::StreamEnd) {
-          return;
+          return true;
         }
         // Section 8.2.3 admits an empty node, so an explicit document may
         // carry nothing of its own and be closed by its end marker directly
@@ -310,6 +315,8 @@ class Parser {
       token = this->next_token();
       this->skip_document_ends(token, saw_document_end);
     }
+
+    return true;
   }
 
 private: