Separate JSON Pointer fragment tokens before decoding them

src/core/jsonpointer/include/sourcemeta/core/jsonpointer.h

@@ -409,9 +409,10 @@ auto to_pointer(
     -> Pointer;
 
 /// @ingroup jsonpointer
-/// Parse the URI fragment representation of a JSON Pointer, percent-decoding
-/// the fragment data before interpretation as mandated by RFC 3986, Section
-/// 2.4 and RFC 6901, Section 6. The result is not set if the URI has no
+/// Parse the URI fragment representation of a JSON Pointer. Reference tokens
+/// are separated before their octets are decoded, so a percent-encoded slash
+/// neither separates two tokens, per RFC 3986, Section 2.2, nor belongs to
+/// one, per RFC 6901, Section 3. The result is not set if the URI has no
 /// fragment or if its fragment is not a valid JSON Pointer. For example:
 ///
 /// ```cpp

src/core/jsonpointer/jsonpointer.cc

@@ -11,7 +11,6 @@
 #include "stringify.h"
 
 #include <cassert>     // assert
-#include <cstddef>     // std::size_t
 #include <iterator>    // std::cbegin, std::cend, std::prev, std::advance
 #include <memory>      // std::allocator
 #include <optional>    // std::optional, std::nullopt
@@ -24,6 +23,29 @@
 
 namespace {
 
+// The slash that belongs to a reference token is spelled `~1`, and the
+// transformation evaluation applies to a token is alone in turning that back
+// into one, so a token still holding a slash once its octets are decoded
+// conforms to no reference token and is an error condition rather than a
+// member name
+// See https://www.rfc-editor.org/rfc/rfc6901#section-3
+// See https://www.rfc-editor.org/rfc/rfc6901#section-4
+auto append_reference_token(sourcemeta::core::JSON::String &output,
+                            const std::string_view token) -> bool {
+  if (token.find('%') == std::string_view::npos) {
+    output.append(token);
+    return true;
+  }
+
+  const auto decoded{sourcemeta::core::URI::unescape(token)};
+  if (decoded.find('/') != std::string::npos) {
+    return false;
+  }
+
+  output.append(decoded);
+  return true;
+}
+
 template <template <typename T> typename Allocator, typename V,
           typename PointerT = sourcemeta::core::GenericPointer<
               typename V::String,
@@ -336,34 +358,44 @@ auto fragment_to_pointer(const URI &uri) -> std::optional<Pointer> {
     return std::nullopt;
   }
 
+  const auto &value{fragment.value()};
+  if (value.empty()) {
+    return Pointer{};
+  }
+
   // When a URI is dereferenced, the components and subcomponents significant
-  // to the scheme-specific dereferencing process (if any) must be parsed and
-  // separated before the percent-encoded octets within those components can
-  // be safely decoded.
+  // to the dereferencing process must be parsed and separated before the
+  // percent-encoded octets within them can be safely decoded, as the data
+  // would otherwise be mistaken for a delimiter. The only exception is an
+  // octet that stands for an unreserved character, which may be decoded at
+  // any time.
   // See https://www.rfc-editor.org/rfc/rfc3986#section-2.4
-  const auto &value{fragment.value()};
+  //
+  // The slash that precedes every reference token is the delimiter here, and
+  // a reserved character is not equivalent to the octet that stands for it,
+  // so a slash that was percent-encoded never separates two tokens.
+  // See https://www.rfc-editor.org/rfc/rfc3986#section-2.2
+  // See https://www.rfc-editor.org/rfc/rfc6901#section-3
+  if (value.front() != '/') {
+    return std::nullopt;
+  }
+
   JSON::String input;
   input.reserve(value.size());
-  for (std::size_t index = 0; index < value.size(); ++index) {
-    const auto character{value[index]};
-    if (character == '%' && index + 2 < value.size()) {
-      const auto high{sourcemeta::core::hex_digit_value(value[index + 1])};
-      const auto low{sourcemeta::core::hex_digit_value(value[index + 2])};
-      if (high >= 0 && low >= 0) {
-        input.push_back(static_cast<char>((high * 16) + low));
-        index += 2;
-        continue;
-      }
+  auto rest{value.substr(1)};
+  while (true) {
+    const auto parts{split_once(rest, '/')};
+    input.push_back('/');
+    if (!append_reference_token(input,
+                                parts.has_value() ? parts->first : rest)) {
+      return std::nullopt;
     }
 
-    input.push_back(character);
-  }
+    if (!parts.has_value()) {
+      break;
+    }
 
-  if (input.empty()) {
-    return Pointer{};
-  }
-  if (input.front() != '/') {
-    return std::nullopt;
+    rest = parts->second;
   }
 
   try {

src/core/jsonschema/frame.cc

@@ -426,7 +426,10 @@ auto locate(const Locations &locations,
 
 // 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
+// else comparable to either of them. 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
 auto canonicalize_reference(const sourcemeta::core::JSON::String &uri)
     -> std::optional<sourcemeta::core::JSON::String> {
   try {
@@ -439,6 +442,18 @@ auto canonicalize_reference(const sourcemeta::core::JSON::String &uri)
   }
 }
 
+// A lookup that came up empty is only worth repeating under a spelling it has
+// not tried yet
+auto other_spelling(const sourcemeta::core::JSON::String &uri)
+    -> std::optional<sourcemeta::core::JSON::String> {
+  const auto canonical{canonicalize_reference(uri)};
+  if (!canonical.has_value() || canonical.value() == uri) {
+    return std::nullopt;
+  }
+
+  return canonical;
+}
+
 auto set_base_and_fragment(sourcemeta::core::SchemaFrame::Reference &entry)
     -> void {
   const std::string_view destination_view{entry.destination};
@@ -2028,6 +2043,19 @@ auto SchemaFrame::location(const SchemaReferenceType type,
     return result->second;
   }
 
+  // A URI is settled the same way whichever kind of location it is asked
+  // for, as the kind narrows which places answer rather than how the name of
+  // a place is spelled
+  const auto canonical{other_spelling(uri_string)};
+  if (!canonical.has_value()) {
+    return std::nullopt;
+  }
+
+  const auto canonical_result{this->locations_.find({type, canonical.value()})};
+  if (canonical_result != this->locations_.cend()) {
+    return canonical_result->second;
+  }
+
   return std::nullopt;
 }
 
@@ -2122,15 +2150,10 @@ auto SchemaFrame::traverse(const std::string_view uri) const
     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) {
+  // A URI that reaches here spelled any other way names what a reference
+  // spelled identically would reach
+  const auto canonical{other_spelling(uri_string)};
+  if (!canonical.has_value()) {
     return std::nullopt;
   }
 

src/core/jsonschema/include/sourcemeta/core/jsonschema_frame.h

@@ -214,7 +214,9 @@ class SOURCEMETA_CORE_JSONSCHEMA_EXPORT SchemaFrame {
             const sourcemeta::core::WeakPointer &pointer) const
       -> std::optional<std::reference_wrapper<const Reference>>;
 
-  /// Get a specific location entry by reference type and URI
+  /// Get a specific location entry by reference type and URI, settling the
+  /// spelling of the URI the way framing settles the one it files a location
+  /// under
   [[nodiscard]] auto location(const SchemaReferenceType type,
                               const std::string_view uri) const
       -> std::optional<std::reference_wrapper<const Location>>;

test/jsonpointer/jsonpointer_fragment_to_pointer_test.cc

@@ -85,24 +85,90 @@ TEST(uri_with_percent_encoded_caret) {
   EXPECT_EQ(pointer.value().at(0).to_property(), "e^f");
 }
 
+// RFC 3986 Section 2.4 separates the subcomponents of a URI before decoding
+// the percent-encoded octets within them, and RFC 3986 Section 2.2 keeps a
+// reserved character from being equivalent to the octet that stands for it, so
+// the slash that RFC 6901 Section 3 puts in front of every reference token is
+// never one that was percent-encoded. RFC 6901 Section 3 gives the slash that
+// belongs to a token the one spelling `~1`, so a decoded one leaves the
+// fragment conforming to nothing
 TEST(uri_with_percent_encoded_slash) {
   const sourcemeta::core::URI uri{"#/foo%2Fbar"};
+  EXPECT_FALSE(sourcemeta::core::fragment_to_pointer(uri).has_value());
+}
+
+TEST(uri_with_lowercase_percent_encoded_slash) {
+  const sourcemeta::core::URI uri{"#/foo%2fbar"};
+  EXPECT_FALSE(sourcemeta::core::fragment_to_pointer(uri).has_value());
+}
+
+TEST(uri_with_percent_encoded_slash_and_an_unencoded_one) {
+  const sourcemeta::core::URI uri{"#/foo%2Fbar/baz"};
+  EXPECT_FALSE(sourcemeta::core::fragment_to_pointer(uri).has_value());
+}
+
+TEST(uri_with_percent_encoded_slash_as_a_whole_token) {
+  const sourcemeta::core::URI uri{"#/%2F"};
+  EXPECT_FALSE(sourcemeta::core::fragment_to_pointer(uri).has_value());
+}
+
+TEST(uri_with_percent_encoded_slash_ending_a_token) {
+  const sourcemeta::core::URI uri{"#/foo%2F"};
+  EXPECT_FALSE(sourcemeta::core::fragment_to_pointer(uri).has_value());
+}
+
+TEST(uri_with_percent_encoded_slash_after_an_empty_token) {
+  const sourcemeta::core::URI uri{"#//%2F"};
+  EXPECT_FALSE(sourcemeta::core::fragment_to_pointer(uri).has_value());
+}
+
+TEST(uri_with_percent_encoded_slash_before_another_token) {
+  const sourcemeta::core::URI uri{"#/%2F/x"};
+  EXPECT_FALSE(sourcemeta::core::fragment_to_pointer(uri).has_value());
+}
+
+TEST(uri_with_a_lone_separator) {
+  const sourcemeta::core::URI uri{"#/"};
   const auto pointer{sourcemeta::core::fragment_to_pointer(uri)};
   EXPECT_TRUE(pointer.has_value());
-  EXPECT_EQ(pointer.value().size(), 2);
+  EXPECT_EQ(pointer.value().size(), 1);
   EXPECT_TRUE(pointer.value().at(0).is_property());
-  EXPECT_EQ(pointer.value().at(0).to_property(), "foo");
-  EXPECT_TRUE(pointer.value().at(1).is_property());
-  EXPECT_EQ(pointer.value().at(1).to_property(), "bar");
+  EXPECT_EQ(pointer.value().at(0).to_property(), "");
 }
 
+// RFC 6901 Section 3 wants a slash in front of every reference token, and a
+// percent-encoded one does not stand in for it, so nothing opens the first
+// token here
 TEST(uri_with_percent_encoded_leading_slash) {
   const sourcemeta::core::URI uri{"#%2Ffoo"};
   const auto pointer{sourcemeta::core::fragment_to_pointer(uri)};
+  EXPECT_FALSE(pointer.has_value());
+}
+
+// RFC 3986 Section 2.4 lets an octet that stands for an unreserved character
+// be decoded at any time, and the tilde is one, so it still forms the escape
+// that RFC 6901 Section 3 reads it as
+TEST(uri_with_percent_encoded_tilde_forming_an_escape) {
+  const sourcemeta::core::URI uri{"#/a%7E1b"};
+  const auto pointer{sourcemeta::core::fragment_to_pointer(uri)};
   EXPECT_TRUE(pointer.has_value());
   EXPECT_EQ(pointer.value().size(), 1);
   EXPECT_TRUE(pointer.value().at(0).is_property());
-  EXPECT_EQ(pointer.value().at(0).to_property(), "foo");
+  EXPECT_EQ(pointer.value().at(0).to_property(), "a/b");
+}
+
+TEST(uri_with_percent_encoded_slash_next_to_a_tilde) {
+  const sourcemeta::core::URI uri{"#/a~%2Fb"};
+  const auto pointer{sourcemeta::core::fragment_to_pointer(uri)};
+  EXPECT_FALSE(pointer.has_value());
+}
+
+TEST(round_trip_token_with_a_slash) {
+  const sourcemeta::core::Pointer pointer{"foo/bar"};
+  const auto result{
+      sourcemeta::core::fragment_to_pointer(sourcemeta::core::to_uri(pointer))};
+  EXPECT_TRUE(result.has_value());
+  EXPECT_EQ(result.value(), pointer);
 }
 
 TEST(uri_with_lowercase_percent_encoding) {