Skip to content

Commit

Permalink
Update ParseQuicTag to only use hex decoding if valid
Browse files Browse the repository at this point in the history
This relands cl/616801281. It should be safe to land now that Envoy uses a newer revision of abseil (see envoyproxy/envoy#36317)

ParseQuicTag was using absl::HexStringToBytes to unconditionally decode strings when they were 8 bytes long, but for strings with non-hexidecimal characters the return value of calling absl::HexStringToBytes is unspecified. This CL updates ParseQuicTag to only use the output of absl::HexStringToBytes when the call returns success.

This also moves the code away from using the deprecated version of absl::HexStringToBytes, allowing the Chrome build of QUICHE to build without the `-Wno-deprecated-declarations` clang flag.

PiperOrigin-RevId: 714809406
  • Loading branch information
recvfrom authored and copybara-github committed Jan 13, 2025
1 parent cd46f49 commit c14c499
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 7 deletions.
9 changes: 5 additions & 4 deletions quiche/quic/core/quic_tag.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,16 @@
#include "quiche/quic/core/quic_tag.h"

#include <algorithm>
#include <cstddef>
#include <cstdint>
#include <string>
#include <vector>

#include "absl/base/macros.h"
#include "absl/strings/ascii.h"
#include "absl/strings/escaping.h"
#include "absl/strings/str_split.h"
#include "quiche/quic/platform/api/quic_flag_utils.h"
#include "quiche/quic/platform/api/quic_flags.h"
#include "absl/strings/string_view.h"
#include "quiche/common/quiche_text_utils.h"

namespace quic {
Expand Down Expand Up @@ -80,8 +81,8 @@ bool ContainsQuicTag(const QuicTagVector& tag_vector, QuicTag tag) {
QuicTag ParseQuicTag(absl::string_view tag_string) {
quiche::QuicheTextUtils::RemoveLeadingAndTrailingWhitespace(&tag_string);
std::string tag_bytes;
if (tag_string.length() == 8) {
tag_bytes = absl::HexStringToBytes(tag_string);
if (tag_string.length() == 8 &&
absl::HexStringToBytes(tag_string, &tag_bytes)) {
tag_string = tag_bytes;
}
QuicTag tag = 0;
Expand Down
10 changes: 7 additions & 3 deletions quiche/quic/core/quic_tag.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,13 @@ QUICHE_EXPORT bool FindMutualQuicTag(const QuicTagVector& our_tags,
QUICHE_EXPORT std::string QuicTagToString(QuicTag tag);

// Utility function that converts a string of the form "ABCD" to its
// corresponding QuicTag. Note that tags that are less than four characters
// long are right-padded with zeroes. Tags that contain non-ASCII characters
// are represented as 8-character-long hexadecimal strings.
// corresponding QuicTag. Note that `tag_string` will have leading and trailing
// whitespace removed and will then be converted to a QuicTag as follows:
// - If the tag string is 8 bytes in length and all bytes are valid hexidecimal
// ASCII characters, then the returned QuicTag will have a corresponding
// hexidecimal value.
// - Otherwise, the QuicTag will be produced using the first four bytes of the
// tag string, right-padding with zeroes if there are fewer than four bytes.
QUICHE_EXPORT QuicTag ParseQuicTag(absl::string_view tag_string);

// Utility function that converts a string of the form "ABCD,EFGH" to a vector
Expand Down
5 changes: 5 additions & 0 deletions quiche/quic/core/quic_tag_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,17 @@ TEST_F(QuicTagTest, MakeQuicTag) {
TEST_F(QuicTagTest, ParseQuicTag) {
QuicTag tag_abcd = MakeQuicTag('A', 'B', 'C', 'D');
EXPECT_EQ(ParseQuicTag("ABCD"), tag_abcd);
EXPECT_EQ(ParseQuicTag(" ABCD "), tag_abcd);
EXPECT_EQ(ParseQuicTag("ABCDE"), tag_abcd);
EXPECT_EQ(ParseQuicTag("ABCDEFGH"), tag_abcd);
QuicTag tag_efgh = MakeQuicTag('E', 'F', 'G', 'H');
EXPECT_EQ(ParseQuicTag("EFGH"), tag_efgh);
QuicTag tag_ijk = MakeQuicTag('I', 'J', 'K', 0);
EXPECT_EQ(ParseQuicTag("IJK"), tag_ijk);
QuicTag tag_l = MakeQuicTag('L', 0, 0, 0);
EXPECT_EQ(ParseQuicTag("L"), tag_l);
EXPECT_EQ(ParseQuicTag(" L"), tag_l);
EXPECT_EQ(ParseQuicTag("L "), tag_l);
QuicTag tag_hex = MakeQuicTag('M', 'N', 'O', static_cast<char>(255));
EXPECT_EQ(ParseQuicTag("4d4e4fff"), tag_hex);
EXPECT_EQ(ParseQuicTag("4D4E4FFF"), tag_hex);
Expand All @@ -52,6 +56,7 @@ TEST_F(QuicTagTest, ParseQuicTag) {
EXPECT_EQ(ParseQuicTag("r$_7"), tag_with_custom_chars);
QuicTag tag_zero = 0;
EXPECT_EQ(ParseQuicTag(""), tag_zero);
EXPECT_EQ(ParseQuicTag(" "), tag_zero);
QuicTagVector tag_vector;
EXPECT_EQ(ParseQuicTagVector(""), tag_vector);
EXPECT_EQ(ParseQuicTagVector(" "), tag_vector);
Expand Down

0 comments on commit c14c499

Please sign in to comment.