From fda2e4b03b6555a7848fca4fa25629441cdbaebb Mon Sep 17 00:00:00 2001
From: Glyn Matthews <glyn.matthews@gmail.com>
Date: Wed, 8 Jul 2020 10:56:02 +0200
Subject: [PATCH] Refactored percent encoding, using std::byte which is more
 appropriate

---
 .../v1/percent_encoding/percent_decode.hpp    |   2 +-
 .../percent_encoding/percent_decode_range.hpp |  33 ++---
 .../v1/percent_encoding/percent_encode.hpp    |  12 +-
 .../percent_encoding/percent_encoded_char.hpp | 133 +++++++++++-------
 src/v1/core/host.cpp                          |   2 +-
 src/v1/core/url_parser_context.cpp            |  10 +-
 src/v1/url/url.cpp                            |   4 +-
 .../percent_encoding_tests.cpp                |  15 +-
 8 files changed, 123 insertions(+), 88 deletions(-)

diff --git a/include/skyr/v1/percent_encoding/percent_decode.hpp b/include/skyr/v1/percent_encoding/percent_decode.hpp
index 63d4189f..7aa06472 100644
--- a/include/skyr/v1/percent_encoding/percent_decode.hpp
+++ b/include/skyr/v1/percent_encoding/percent_decode.hpp
@@ -30,4 +30,4 @@ inline auto percent_decode(std::string_view input) -> tl::expected<std::string,
 }  // namespace v1
 }  // namespace skyr
 
-#endif //SKYR_V1_PERCENT_DECODING_PERCENT_DECODE_HPP
+#endif  // SKYR_V1_PERCENT_DECODING_PERCENT_DECODE_HPP
diff --git a/include/skyr/v1/percent_encoding/percent_decode_range.hpp b/include/skyr/v1/percent_encoding/percent_decode_range.hpp
index 2177c273..19911906 100644
--- a/include/skyr/v1/percent_encoding/percent_decode_range.hpp
+++ b/include/skyr/v1/percent_encoding/percent_decode_range.hpp
@@ -17,17 +17,17 @@ namespace skyr {
 inline namespace v1 {
 namespace percent_encoding {
 namespace details {
-inline auto letter_to_hex(char byte) noexcept -> tl::expected<char, percent_encode_errc> {
-  if ((byte >= '0') && (byte <= '9')) {
-    return static_cast<char>(byte - '0');
+inline auto alnum_to_hex(char value) noexcept -> tl::expected<std::byte, percent_encode_errc> {
+  if ((value >= '0') && (value <= '9')) {
+    return static_cast<std::byte>(value - '0');
   }
 
-  if ((byte >= 'a') && (byte <= 'f')) {
-    return static_cast<char>(byte + '\x0a' - 'a');
+  if ((value >= 'a') && (value <= 'f')) {
+    return static_cast<std::byte>(value + '\x0a' - 'a');
   }
 
-  if ((byte >= 'A') && (byte <= 'F')) {
-    return static_cast<char>(byte + '\x0a' - 'A');
+  if ((value >= 'A') && (value <= 'F')) {
+    return static_cast<std::byte>(value + '\x0a' - 'A');
   }
 
   return tl::make_unexpected(percent_encoding::percent_encode_errc::non_hex_input);
@@ -86,14 +86,15 @@ class percent_decode_iterator {
       if (remainder_.size() < 3) {
         return tl::make_unexpected(percent_encoding::percent_encode_errc::overflow);
       }
-      auto v0 = details::letter_to_hex(remainder_[1]);
-      auto v1 = details::letter_to_hex(remainder_[2]);
+      auto v0 = details::alnum_to_hex(remainder_[1]);
+      auto v1 = details::alnum_to_hex(remainder_[2]);
 
       if (!v0 || !v1) {
         return tl::make_unexpected(percent_encoding::percent_encode_errc::non_hex_input);
       }
 
-      return static_cast<char>((0x10u * v0.value()) + v1.value());
+      return static_cast<char>(
+          (0x10u * std::to_integer<unsigned int>(v0.value())) + std::to_integer<unsigned int>(v1.value()));
     } else {
       return remainder_[0];
     }
@@ -145,26 +146,26 @@ class percent_decode_range {
 
   ///
   /// \return
-  [[nodiscard]] auto begin() const noexcept {
+  [[nodiscard]] auto cbegin() const noexcept {
     return it_;
   }
 
   ///
   /// \return
-  [[nodiscard]] auto end() const noexcept {
+  [[nodiscard]] auto cend() const noexcept {
     return sentinel{};
   }
 
   ///
   /// \return
-  [[nodiscard]] auto cbegin() const noexcept {
-    return begin();
+  [[nodiscard]] auto begin() const noexcept {
+    return cbegin();
   }
 
   ///
   /// \return
-  [[nodiscard]] auto cend() const noexcept {
-    return end();
+  [[nodiscard]] auto end() const noexcept {
+    return cend();
   }
 
   ///
diff --git a/include/skyr/v1/percent_encoding/percent_encode.hpp b/include/skyr/v1/percent_encoding/percent_encode.hpp
index 0fe82c2c..4705615e 100644
--- a/include/skyr/v1/percent_encoding/percent_encode.hpp
+++ b/include/skyr/v1/percent_encoding/percent_encode.hpp
@@ -19,20 +19,20 @@ inline auto percent_encode(std::string_view input) {
   using percent_encoding::percent_encoded_char;
 
   static constexpr auto encode = [] (auto byte) {
-    if (byte == '\x20') {
-      return percent_encoded_char('+', percent_encoded_char::no_encode());
-    } else if ((byte == '\x2a') || (byte == '\x2d') || (byte == '\x2e') ||
+    if ((byte == '\x2a') || (byte == '\x2d') || (byte == '\x2e') ||
         ((byte >= '\x30') && (byte <= '\x39')) ||
         ((byte >= '\x41') && (byte <= '\x5a')) || (byte == '\x5f') ||
         ((byte >= '\x61') && (byte <= '\x7a'))) {
       return percent_encoded_char(
-          byte, percent_encoded_char::no_encode());
+          std::byte(byte), percent_encoded_char::no_encode());
+    } else if (byte == '\x20') {
+      return percent_encoded_char(std::byte('+'), percent_encoded_char::no_encode());
     }
-    return percent_encoded_char(byte);
+    return percent_encoded_char(std::byte(byte));
   };
 
   auto result = std::string{};
-  for (auto encoded : input | ranges::views::transform(encode)) {
+  for (const auto &encoded : input | ranges::views::transform(encode)) {
     result += std::string(std::cbegin(encoded), std::cend(encoded));
   }
   return result;
diff --git a/include/skyr/v1/percent_encoding/percent_encoded_char.hpp b/include/skyr/v1/percent_encoding/percent_encoded_char.hpp
index e434a56f..3dce8ffd 100644
--- a/include/skyr/v1/percent_encoding/percent_encoded_char.hpp
+++ b/include/skyr/v1/percent_encoding/percent_encoded_char.hpp
@@ -8,59 +8,75 @@
 
 #include <string>
 #include <locale>
+#include <cstddef>
 
 namespace skyr {
 inline namespace v1 {
 namespace percent_encoding {
 namespace details {
-inline constexpr auto hex_to_letter(char byte) noexcept {
-  if ((byte >= '\x00') && (byte < '\x0a')) {
-    return static_cast<char>(byte + '0');
+///
+/// \param value
+/// \return
+inline constexpr auto hex_to_alnum(std::byte value) noexcept {
+  if ((value >= std::byte(0x00)) && (value < std::byte(0x0a))) {
+    return static_cast<char>(std::to_integer<unsigned>(value) + '0');
   }
 
-  if ((byte >= '\x0a') && (byte < '\x10')) {
-    return static_cast<char>(byte - '\x0a' + 'A');
+  if ((value >= std::byte(0x0a)) && (value < std::byte(0x10))) {
+    return static_cast<char>(std::to_integer<unsigned>(value) - '\x0a' + 'A');
   }
 
-  return byte;
+  return static_cast<char>(value);
 }
 
-inline constexpr auto is_c0_control_byte(char byte) noexcept {
-  return (byte <= '\x1f') || (byte > '\x7e');
+///
+/// \param value
+/// \return
+inline constexpr auto is_c0_control_byte(std::byte value) noexcept {
+  return (value <= std::byte(0x1f)) || (value > std::byte(0x7e));
 }
 
-inline constexpr auto is_fragment_byte(char byte) {
+///
+/// \param value
+/// \return
+inline constexpr auto is_fragment_byte(std::byte value) {
   return
-      is_c0_control_byte(byte) ||
-      (byte == '\x20') ||
-      (byte == '\x22') ||
-      (byte == '\x3c') ||
-      (byte == '\x3e') ||
-      (byte == '\x60');
+      is_c0_control_byte(value) ||
+      (value == std::byte(0x20)) ||
+      (value == std::byte(0x22)) ||
+      (value == std::byte(0x3c)) ||
+      (value == std::byte(0x3e)) ||
+      (value == std::byte(0x60));
 }
 
-inline constexpr auto is_path_byte(char byte) {
+///
+/// \param value
+/// \return
+inline constexpr auto is_path_byte(std::byte value) {
   return
-      is_fragment_byte(byte) ||
-      (byte == '\x23') ||
-      (byte == '\x3f') ||
-      (byte == '\x7b') ||
-      (byte == '\x7d');
+      is_fragment_byte(value) ||
+      (value == std::byte(0x23)) ||
+      (value == std::byte(0x3f)) ||
+      (value == std::byte(0x7b)) ||
+      (value == std::byte(0x7d));
 }
 
-inline constexpr auto is_userinfo_byte(char byte) {
+///
+/// \param value
+/// \return
+inline constexpr auto is_userinfo_byte(std::byte value) {
   return
-      is_path_byte(byte) ||
-      (byte == '\x2f') ||
-      (byte == '\x3a') ||
-      (byte == '\x3b') ||
-      (byte == '\x3d') ||
-      (byte == '\x40') ||
-      (byte == '\x5b') ||
-      (byte == '\x5c') ||
-      (byte == '\x5d') ||
-      (byte == '\x5e') ||
-      (byte == '\x7c');
+      is_path_byte(value) ||
+      (value == std::byte(0x2f)) ||
+      (value == std::byte(0x3a)) ||
+      (value == std::byte(0x3b)) ||
+      (value == std::byte(0x3d)) ||
+      (value == std::byte(0x40)) ||
+      (value == std::byte(0x5b)) ||
+      (value == std::byte(0x5c)) ||
+      (value == std::byte(0x5d)) ||
+      (value == std::byte(0x5e)) ||
+      (value == std::byte(0x7c));
 }
 }  // namespace details
 
@@ -83,6 +99,8 @@ struct percent_encoded_char {
 
   using impl_type = std::string;
 
+  static constexpr std::byte mask = std::byte(0x0f);
+
  public:
 
   ///
@@ -101,27 +119,38 @@ struct percent_encoded_char {
   percent_encoded_char() = default;
 
   ///
-  /// \param byte
-  percent_encoded_char(char byte, no_encode)
-      : impl_{byte} {}
+  /// \param value
+  percent_encoded_char(std::byte value, no_encode)
+      : impl_{static_cast<char>(value)} {}
+
   ///
-  /// \param byte
-  explicit percent_encoded_char(char byte)
+  /// \param value
+  explicit percent_encoded_char(std::byte value)
       : impl_{
-      '%',
-      details::hex_to_letter(static_cast<char>((static_cast<unsigned>(byte) >> 4u) & 0x0fu)),
-      details::hex_to_letter(static_cast<char>(static_cast<unsigned>(byte) & 0x0fu))} {}
+      '%', details::hex_to_alnum((value >> 4u) & mask), details::hex_to_alnum(value & mask)} {}
+
+  ///
+  /// \return
+  [[nodiscard]] auto cbegin() const noexcept {
+    return impl_.cbegin();
+  }
+
+  ///
+  /// \return
+  [[nodiscard]] auto cend() const noexcept {
+    return impl_.cend();
+  }
 
   ///
   /// \return
   [[nodiscard]] auto begin() const noexcept {
-    return impl_.begin();
+    return cbegin();
   }
 
   ///
   /// \return
   [[nodiscard]] auto end() const noexcept {
-    return impl_.end();
+    return cend();
   }
 
   ///
@@ -160,7 +189,7 @@ struct percent_encoded_char {
 /// \param pred
 /// \return
 template <class Pred>
-inline auto percent_encode_byte(char byte, Pred pred) -> percent_encoded_char {
+inline auto percent_encode_byte(std::byte byte, Pred pred) -> percent_encoded_char {
   if (pred(byte)) {
     return percent_encoding::percent_encoded_char(byte);
   }
@@ -169,23 +198,23 @@ inline auto percent_encode_byte(char byte, Pred pred) -> percent_encoded_char {
 }
 
 ///
-/// \param byte
+/// \param value
 /// \param excludes
 /// \return
-inline auto percent_encode_byte(char byte, encode_set excludes) -> percent_encoded_char {
+inline auto percent_encode_byte(std::byte value, encode_set excludes) -> percent_encoded_char {
   switch (excludes) {
     case encode_set::none:
-      return percent_encoding::percent_encoded_char(byte);
+      return percent_encoding::percent_encoded_char(value);
     case encode_set::c0_control:
-      return percent_encode_byte(byte, details::is_c0_control_byte);
+      return percent_encode_byte(value, details::is_c0_control_byte);
     case encode_set::userinfo:
-      return percent_encode_byte(byte, details::is_userinfo_byte);
+      return percent_encode_byte(value, details::is_userinfo_byte);
     case encode_set::path:
-      return percent_encode_byte(byte, details::is_path_byte);
+      return percent_encode_byte(value, details::is_path_byte);
     case encode_set::fragment:
-      return percent_encode_byte(byte, details::is_fragment_byte);
+      return percent_encode_byte(value, details::is_fragment_byte);
   }
-  return percent_encoding::percent_encoded_char(byte);
+  return percent_encoding::percent_encoded_char(value);
 }
 
 /// Tests whether the input string contains percent encoded values
diff --git a/src/v1/core/host.cpp b/src/v1/core/host.cpp
index 16e081b6..ca84e0a0 100644
--- a/src/v1/core/host.cpp
+++ b/src/v1/core/host.cpp
@@ -40,7 +40,7 @@ auto parse_opaque_host(std::string_view input,
 
   auto output = std::string();
   for (auto c : input) {
-    auto pct_encoded = percent_encode_byte(c, percent_encoding::encode_set::c0_control);
+    auto pct_encoded = percent_encode_byte(std::byte(c), percent_encoding::encode_set::c0_control);
     output += pct_encoded.to_string();
   }
   return skyr::v1::opaque_host{output};
diff --git a/src/v1/core/url_parser_context.cpp b/src/v1/core/url_parser_context.cpp
index 997ffa0a..6a79679f 100644
--- a/src/v1/core/url_parser_context.cpp
+++ b/src/v1/core/url_parser_context.cpp
@@ -362,7 +362,7 @@ auto url_parser_context::parse_authority(char byte) -> tl::expected<url_parse_ac
         continue;
       }
 
-      auto pct_encoded = percent_encode_byte(c, percent_encoding::encode_set::userinfo);
+      auto pct_encoded = percent_encode_byte(std::byte(c), percent_encoding::encode_set::userinfo);
       if (password_token_seen_flag) {
         url.password += pct_encoded.to_string();
       } else {
@@ -698,7 +698,7 @@ auto url_parser_context::parse_path(char byte) -> tl::expected<url_parse_action,
       *validation_error |= true;
     }
 
-    auto pct_encoded = percent_encode_byte(byte, percent_encoding::encode_set::path);
+    auto pct_encoded = percent_encode_byte(std::byte(byte), percent_encoding::encode_set::path);
     buffer += pct_encoded.to_string();
   }
 
@@ -721,7 +721,7 @@ auto url_parser_context::parse_cannot_be_a_base_url(char byte) -> tl::expected<u
       *validation_error |= true;
     }
     if (!is_eof()) {
-      auto pct_encoded = percent_encode_byte(byte, percent_encoding::encode_set::c0_control);
+      auto pct_encoded = percent_encode_byte(std::byte(byte), percent_encoding::encode_set::c0_control);
       url.path[0] += pct_encoded.to_string();
     }
   }
@@ -737,7 +737,7 @@ auto url_parser_context::parse_query(char byte) -> tl::expected<url_parse_action
         (byte > '~') ||
         (contains(byte, R"("#<>)"sv)) ||
         ((byte == '\'') && url.is_special())) {
-      auto pct_encoded = percent_encode_byte(byte, percent_encoding::encode_set::none);
+      auto pct_encoded = percent_encode_byte(std::byte(byte), percent_encoding::encode_set::none);
       url.query.value() += pct_encoded.to_string();
     } else {
       url.query.value().push_back(byte);
@@ -757,7 +757,7 @@ auto url_parser_context::parse_fragment(char byte) -> tl::expected<url_parse_act
       *validation_error |= true;
     }
 
-    auto pct_encoded = percent_encode_byte(byte, percent_encoding::encode_set::fragment);
+    auto pct_encoded = percent_encode_byte(std::byte(byte), percent_encoding::encode_set::fragment);
     url.fragment.value() += pct_encoded.to_string();
   }
   return url_parse_action::increment;
diff --git a/src/v1/url/url.cpp b/src/v1/url/url.cpp
index 60e6d1f5..033b08dd 100644
--- a/src/v1/url/url.cpp
+++ b/src/v1/url/url.cpp
@@ -84,7 +84,7 @@ auto url::set_username(string_view username) -> std::error_code {
 
   new_url.username.clear();
   for (auto c : username) {
-    auto pct_encoded = percent_encode_byte(c, percent_encoding::encode_set::userinfo);
+    auto pct_encoded = percent_encode_byte(std::byte(c), percent_encoding::encode_set::userinfo);
     new_url.username += pct_encoded.to_string();
   }
 
@@ -102,7 +102,7 @@ auto url::set_password(string_view password) -> std::error_code {
 
   new_url.password.clear();
   for (auto c : password) {
-    auto pct_encoded = percent_encode_byte(c, percent_encoding::encode_set::userinfo);
+    auto pct_encoded = percent_encode_byte(std::byte(c), percent_encoding::encode_set::userinfo);
     new_url.password += pct_encoded.to_string();
   }
 
diff --git a/tests/percent_encoding/percent_encoding_tests.cpp b/tests/percent_encoding/percent_encoding_tests.cpp
index 4f1b9c4c..d3cb2823 100644
--- a/tests/percent_encoding/percent_encoding_tests.cpp
+++ b/tests/percent_encoding/percent_encoding_tests.cpp
@@ -14,7 +14,8 @@ TEST_CASE("encode fragment", "[percent_encoding]") {
       ' ', '\"', '<', '>', '`');
 
   SECTION("encode_fragment_set") {
-    auto encoded = skyr::percent_encoding::percent_encode_byte(c, skyr::percent_encoding::encode_set::fragment);
+    auto encoded = skyr::percent_encoding::percent_encode_byte(
+        std::byte(c), skyr::percent_encoding::encode_set::fragment);
     CHECK(encoded.is_encoded());
   }
 }
@@ -24,7 +25,8 @@ TEST_CASE("encode path", "[percent_encoding]") {
       ' ', '\"', '<', '>', '`', '#', '?', '{', '}');
 
   SECTION("encode_path_set") {
-    auto encoded = skyr::percent_encoding::percent_encode_byte(c, skyr::percent_encoding::encode_set::path);
+    auto encoded = skyr::percent_encoding::percent_encode_byte(
+        std::byte(c), skyr::percent_encoding::encode_set::path);
     CHECK(encoded.is_encoded());
   }
 }
@@ -35,7 +37,8 @@ TEST_CASE("encode userinfo", "[percent_encoding]") {
       ':', ';', '=', '@', '[', '\\', ']', '^', '|');
 
   SECTION("encode_userinfo_set") {
-    auto encoded = skyr::percent_encoding::percent_encode_byte(c, skyr::percent_encoding::encode_set::userinfo);
+    auto encoded = skyr::percent_encoding::percent_encode_byte(
+        std::byte(c), skyr::percent_encoding::encode_set::userinfo);
     CHECK(encoded.is_encoded());
   }
 }
@@ -45,14 +48,16 @@ TEST_CASE("encode_tests", "[percent_encoding]") {
 
   SECTION("encode_codepoints_before_0x20_set") {
     for (auto i = 0u; i < 0x20u; ++i) {
-      auto encoded = skyr::percent_encoding::percent_encode_byte(i, skyr::percent_encoding::encode_set::c0_control);
+      auto encoded = skyr::percent_encoding::percent_encode_byte(
+          std::byte(i), skyr::percent_encoding::encode_set::c0_control);
       CHECK(fmt::format("%{:02X}", i) == encoded.to_string());
     }
   }
 
   SECTION("encode_codepoints_before_0x7e_set") {
     for (auto i = 0x7fu; i <= 0xffu; ++i) {
-      auto encoded = skyr::percent_encoding::percent_encode_byte(i, skyr::percent_encoding::encode_set::c0_control);
+      auto encoded = skyr::percent_encoding::percent_encode_byte(
+          std::byte(i), skyr::percent_encoding::encode_set::c0_control);
       CHECK(fmt::format("%{:02X}", i) == encoded.to_string());
     }
   }
