Skip to content

Commit

Permalink
http: fix and generalize PercentEncoding utility. (envoyproxy#11677)
Browse files Browse the repository at this point in the history
* Fix an off-by-one bug in PercentEncoding::encode(). It doesn't appear that this utility is used
  anywhere that this would be security sensitive, but it is used for sendLocalReply() for gRPC
  messages.

* Generalize from '%' to arbitrary visible reserved chars. RFC3986 encoding requires more
  flexibility, e.g. being able to percent encode `/` or `#`.

This change was motivated by a larger patch that provides encoding/decoding of udpa:// URIs, as part
of envoyproxy#11264.

Risk level: Low
Testing: Additional unit tests.

Signed-off-by: Harvey Tuch <[email protected]>
  • Loading branch information
htuch authored and songhu committed Jun 25, 2020
1 parent 8e63737 commit 8c750f3
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 11 deletions.
15 changes: 9 additions & 6 deletions source/common/http/utility.cc
Original file line number Diff line number Diff line change
Expand Up @@ -833,7 +833,9 @@ void Utility::traversePerFilterConfigGeneric(
}
}

std::string Utility::PercentEncoding::encode(absl::string_view value) {
std::string Utility::PercentEncoding::encode(absl::string_view value,
absl::string_view reserved_chars) {
absl::flat_hash_set<char> reserved_char_set{reserved_chars.begin(), reserved_chars.end()};
for (size_t i = 0; i < value.size(); ++i) {
const char& ch = value[i];
// The escaping characters are defined in
Expand All @@ -842,22 +844,23 @@ std::string Utility::PercentEncoding::encode(absl::string_view value) {
// We do checking for each char in the string. If the current char is included in the defined
// escaping characters, we jump to "the slow path" (append the char [encoded or not encoded]
// to the returned string one by one) started from the current index.
if (ch < ' ' || ch >= '~' || ch == '%') {
return PercentEncoding::encode(value, i);
if (ch < ' ' || ch >= '~' || reserved_char_set.find(ch) != reserved_char_set.end()) {
return PercentEncoding::encode(value, i, reserved_char_set);
}
}
return std::string(value);
}

std::string Utility::PercentEncoding::encode(absl::string_view value, const size_t index) {
std::string Utility::PercentEncoding::encode(absl::string_view value, const size_t index,
const absl::flat_hash_set<char>& reserved_char_set) {
std::string encoded;
if (index > 0) {
absl::StrAppend(&encoded, value.substr(0, index - 1));
absl::StrAppend(&encoded, value.substr(0, index));
}

for (size_t i = index; i < value.size(); ++i) {
const char& ch = value[i];
if (ch < ' ' || ch >= '~' || ch == '%') {
if (ch < ' ' || ch >= '~' || reserved_char_set.find(ch) != reserved_char_set.end()) {
// For consistency, URI producers should use uppercase hexadecimal digits for all
// percent-encodings. https://tools.ietf.org/html/rfc3986#section-2.1.
absl::StrAppend(&encoded, fmt::format("%{:02X}", ch));
Expand Down
14 changes: 9 additions & 5 deletions source/common/http/utility.h
Original file line number Diff line number Diff line change
Expand Up @@ -142,12 +142,15 @@ class Url {
class PercentEncoding {
public:
/**
* Encodes string view to its percent encoded representation.
* Encodes string view to its percent encoded representation. Non-visible ASCII is always escaped,
* in addition to a given list of reserved chars.
*
* @param value supplies string to be encoded.
* @return std::string percent-encoded string based on
* https://github.com/grpc/grpc/blob/master/doc/PROTOCOL-HTTP2.md#responses.
* @param reserved_chars list of reserved chars to escape. By default the escaped chars in
* https://github.com/grpc/grpc/blob/master/doc/PROTOCOL-HTTP2.md#responses are used.
* @return std::string percent-encoded string.
*/
static std::string encode(absl::string_view value);
static std::string encode(absl::string_view value, absl::string_view reserved_chars = "%");

/**
* Decodes string view from its percent encoded representation.
Expand All @@ -158,7 +161,8 @@ class PercentEncoding {

private:
// Encodes string view to its percent encoded representation, with start index.
static std::string encode(absl::string_view value, const size_t index);
static std::string encode(absl::string_view value, const size_t index,
const absl::flat_hash_set<char>& reserved_char_set);
};

/**
Expand Down
6 changes: 6 additions & 0 deletions test/common/http/utility_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1194,5 +1194,11 @@ TEST(PercentEncoding, Trailing) {
EXPECT_EQ(Utility::PercentEncoding::decode("too%20large%"), "too large%");
}

TEST(PercentEncoding, Encoding) {
EXPECT_EQ(Utility::PercentEncoding::encode("too%large"), "too%25large");
EXPECT_EQ(Utility::PercentEncoding::encode("too%!large/"), "too%25!large/");
EXPECT_EQ(Utility::PercentEncoding::encode("too%!large/", "%!/"), "too%25%21large%2F");
}

} // namespace Http
} // namespace Envoy

0 comments on commit 8c750f3

Please sign in to comment.