Change MakeQuicTag and MakeVersionLabel argument type to unsigned.
Chromium has signed integers and this could cause difficult to debug issues.
PiperOrigin-RevId: 425921423
diff --git a/quic/core/quic_tag.cc b/quic/core/quic_tag.cc
index 32b7bf9..b5f64f1 100644
--- a/quic/core/quic_tag.cc
+++ b/quic/core/quic_tag.cc
@@ -66,7 +66,7 @@
reinterpret_cast<const char*>(&orig_tag), sizeof(orig_tag)));
}
-uint32_t MakeQuicTag(char a, char b, char c, char d) {
+uint32_t MakeQuicTag(uint8_t a, uint8_t b, uint8_t c, uint8_t d) {
return static_cast<uint32_t>(a) | static_cast<uint32_t>(b) << 8 |
static_cast<uint32_t>(c) << 16 | static_cast<uint32_t>(d) << 24;
}
diff --git a/quic/core/quic_tag.h b/quic/core/quic_tag.h
index c77219e..1dfc754 100644
--- a/quic/core/quic_tag.h
+++ b/quic/core/quic_tag.h
@@ -5,6 +5,7 @@
#ifndef QUICHE_QUIC_CORE_QUIC_TAG_H_
#define QUICHE_QUIC_CORE_QUIC_TAG_H_
+#include <cstdint>
#include <map>
#include <string>
#include <vector>
@@ -28,7 +29,8 @@
// MakeQuicTag returns a value given the four bytes. For example:
// MakeQuicTag('C', 'H', 'L', 'O');
-QUIC_EXPORT_PRIVATE QuicTag MakeQuicTag(char a, char b, char c, char d);
+QUIC_EXPORT_PRIVATE QuicTag MakeQuicTag(uint8_t a, uint8_t b, uint8_t c,
+ uint8_t d);
// Returns true if |tag_vector| contains |tag|.
QUIC_EXPORT_PRIVATE bool ContainsQuicTag(const QuicTagVector& tag_vector,
diff --git a/quic/core/quic_versions.cc b/quic/core/quic_versions.cc
index 63d21a1..b9a48b9 100644
--- a/quic/core/quic_versions.cc
+++ b/quic/core/quic_versions.cc
@@ -23,12 +23,6 @@
namespace quic {
namespace {
-// Constructs a version label from the 4 bytes such that the on-the-wire
-// order will be: d, c, b, a.
-QuicVersionLabel MakeVersionLabel(char a, char b, char c, char d) {
- return MakeQuicTag(d, c, b, a);
-}
-
QuicVersionLabel CreateRandomVersionLabelForNegotiation() {
QuicVersionLabel result;
if (!GetQuicFlag(FLAGS_quic_disable_version_negotiation_grease_randomness)) {
@@ -207,6 +201,10 @@
return os;
}
+QuicVersionLabel MakeVersionLabel(uint8_t a, uint8_t b, uint8_t c, uint8_t d) {
+ return MakeQuicTag(d, c, b, a);
+}
+
std::ostream& operator<<(std::ostream& os,
const QuicVersionLabelVector& version_labels) {
os << QuicVersionLabelVectorToString(version_labels);
diff --git a/quic/core/quic_versions.h b/quic/core/quic_versions.h
index 6c2144a..99d3b2f 100644
--- a/quic/core/quic_versions.h
+++ b/quic/core/quic_versions.h
@@ -23,6 +23,7 @@
#ifndef QUICHE_QUIC_CORE_QUIC_VERSIONS_H_
#define QUICHE_QUIC_CORE_QUIC_VERSIONS_H_
+#include <cstdint>
#include <string>
#include <vector>
@@ -394,6 +395,11 @@
using QuicVersionLabel = uint32_t;
using QuicVersionLabelVector = std::vector<QuicVersionLabel>;
+// Constructs a version label from the 4 bytes such that the on-the-wire
+// order will be: d, c, b, a.
+QUIC_EXPORT_PRIVATE QuicVersionLabel MakeVersionLabel(uint8_t a, uint8_t b,
+ uint8_t c, uint8_t d);
+
QUIC_EXPORT_PRIVATE std::ostream& operator<<(
std::ostream& os, const QuicVersionLabelVector& version_labels);
diff --git a/quic/core/quic_versions_test.cc b/quic/core/quic_versions_test.cc
index 6f384dd..7df721a 100644
--- a/quic/core/quic_versions_test.cc
+++ b/quic/core/quic_versions_test.cc
@@ -18,20 +18,13 @@
using ::testing::ElementsAre;
using ::testing::IsEmpty;
-class QuicVersionsTest : public QuicTest {
- protected:
- QuicVersionLabel MakeVersionLabel(char a, char b, char c, char d) {
- return MakeQuicTag(d, c, b, a);
- }
-};
-
-TEST_F(QuicVersionsTest, CreateQuicVersionLabelUnsupported) {
+TEST(QuicVersionsTest, CreateQuicVersionLabelUnsupported) {
EXPECT_QUIC_BUG(
CreateQuicVersionLabel(UnsupportedQuicVersion()),
"Unsupported version QUIC_VERSION_UNSUPPORTED PROTOCOL_UNSUPPORTED");
}
-TEST_F(QuicVersionsTest, KnownAndValid) {
+TEST(QuicVersionsTest, KnownAndValid) {
for (const ParsedQuicVersion& version : AllSupportedVersions()) {
EXPECT_TRUE(version.IsKnown());
EXPECT_TRUE(ParsedQuicVersionIsValid(version.handshake_protocol,
@@ -58,7 +51,7 @@
static_cast<QuicTransportVersion>(99)));
}
-TEST_F(QuicVersionsTest, Features) {
+TEST(QuicVersionsTest, Features) {
ParsedQuicVersion parsed_version_q043 = ParsedQuicVersion::Q043();
ParsedQuicVersion parsed_version_draft_29 = ParsedQuicVersion::Draft29();
@@ -109,7 +102,7 @@
EXPECT_FALSE(parsed_version_draft_29.UsesQuicCrypto());
}
-TEST_F(QuicVersionsTest, ParseQuicVersionLabel) {
+TEST(QuicVersionsTest, ParseQuicVersionLabel) {
EXPECT_EQ(ParsedQuicVersion::Q043(),
ParseQuicVersionLabel(MakeVersionLabel('Q', '0', '4', '3')));
EXPECT_EQ(ParsedQuicVersion::Q046(),
@@ -132,7 +125,7 @@
MakeVersionLabel(0xff, 0x00, 0x00, 0x1d)}));
}
-TEST_F(QuicVersionsTest, ParseQuicVersionString) {
+TEST(QuicVersionsTest, ParseQuicVersionString) {
EXPECT_EQ(ParsedQuicVersion::Q043(), ParseQuicVersionString("Q043"));
EXPECT_EQ(ParsedQuicVersion::Q046(),
ParseQuicVersionString("QUIC_VERSION_46"));
@@ -158,7 +151,7 @@
}
}
-TEST_F(QuicVersionsTest, ParseQuicVersionVectorString) {
+TEST(QuicVersionsTest, ParseQuicVersionVectorString) {
ParsedQuicVersion version_q046 = ParsedQuicVersion::Q046();
ParsedQuicVersion version_q050 = ParsedQuicVersion::Q050();
ParsedQuicVersion version_draft_29 = ParsedQuicVersion::Draft29();
@@ -210,34 +203,31 @@
ElementsAre(version_draft_29));
}
-TEST_F(QuicVersionsTest, CreateQuicVersionLabel) {
- EXPECT_EQ(MakeVersionLabel('Q', '0', '4', '3'),
- CreateQuicVersionLabel(ParsedQuicVersion::Q043()));
- EXPECT_EQ(MakeVersionLabel('Q', '0', '4', '6'),
- CreateQuicVersionLabel(ParsedQuicVersion::Q046()));
- EXPECT_EQ(MakeVersionLabel('Q', '0', '5', '0'),
- CreateQuicVersionLabel(ParsedQuicVersion::Q050()));
- EXPECT_EQ(MakeVersionLabel(0xff, 0x00, 0x00, 0x1d),
- CreateQuicVersionLabel(ParsedQuicVersion::Draft29()));
- EXPECT_EQ(MakeVersionLabel(0x00, 0x00, 0x00, 0x01),
- CreateQuicVersionLabel(ParsedQuicVersion::RFCv1()));
- EXPECT_EQ(MakeVersionLabel(0x70, 0x9a, 0x50, 0xc4),
+// Do not use MakeVersionLabel() to generate expectations, because
+// CreateQuicVersionLabel() uses MakeVersionLabel() internally,
+// in case it has a bug.
+TEST(QuicVersionsTest, CreateQuicVersionLabel) {
+ EXPECT_EQ(0x51303433u, CreateQuicVersionLabel(ParsedQuicVersion::Q043()));
+ EXPECT_EQ(0x51303436u, CreateQuicVersionLabel(ParsedQuicVersion::Q046()));
+ EXPECT_EQ(0x51303530u, CreateQuicVersionLabel(ParsedQuicVersion::Q050()));
+ EXPECT_EQ(0xff00001du, CreateQuicVersionLabel(ParsedQuicVersion::Draft29()));
+ EXPECT_EQ(0x00000001u, CreateQuicVersionLabel(ParsedQuicVersion::RFCv1()));
+ EXPECT_EQ(0x709a50c4u,
CreateQuicVersionLabel(ParsedQuicVersion::V2Draft01()));
// Make sure the negotiation reserved version is in the IETF reserved space.
EXPECT_EQ(
- MakeVersionLabel(0xda, 0x5a, 0x3a, 0x3a) & 0x0f0f0f0f,
+ 0xda5a3a3au & 0x0f0f0f0f,
CreateQuicVersionLabel(ParsedQuicVersion::ReservedForNegotiation()) &
0x0f0f0f0f);
// Make sure that disabling randomness works.
SetQuicFlag(FLAGS_quic_disable_version_negotiation_grease_randomness, true);
- EXPECT_EQ(
- MakeVersionLabel(0xda, 0x5a, 0x3a, 0x3a),
- CreateQuicVersionLabel(ParsedQuicVersion::ReservedForNegotiation()));
+ EXPECT_EQ(0xda5a3a3au, CreateQuicVersionLabel(
+ ParsedQuicVersion::ReservedForNegotiation()));
}
-TEST_F(QuicVersionsTest, QuicVersionLabelToString) {
+TEST(QuicVersionsTest, QuicVersionLabelToString) {
QuicVersionLabelVector version_labels = {
MakeVersionLabel('Q', '0', '3', '5'),
MakeVersionLabel('Q', '0', '3', '7'),
@@ -258,7 +248,7 @@
EXPECT_EQ("Q035,Q037,T038", os.str());
}
-TEST_F(QuicVersionsTest, QuicVersionToString) {
+TEST(QuicVersionsTest, QuicVersionToString) {
EXPECT_EQ("QUIC_VERSION_UNSUPPORTED",
QuicVersionToString(QUIC_VERSION_UNSUPPORTED));
@@ -290,7 +280,7 @@
EXPECT_EQ("QUIC_VERSION_UNSUPPORTED,QUIC_VERSION_43", os.str());
}
-TEST_F(QuicVersionsTest, ParsedQuicVersionToString) {
+TEST(QuicVersionsTest, ParsedQuicVersionToString) {
EXPECT_EQ("0", ParsedQuicVersionToString(ParsedQuicVersion::Unsupported()));
EXPECT_EQ("Q043", ParsedQuicVersionToString(ParsedQuicVersion::Q043()));
EXPECT_EQ("Q046", ParsedQuicVersionToString(ParsedQuicVersion::Q046()));
@@ -321,7 +311,7 @@
EXPECT_EQ("0,Q043", os.str());
}
-TEST_F(QuicVersionsTest, FilterSupportedVersionsAllVersions) {
+TEST(QuicVersionsTest, FilterSupportedVersionsAllVersions) {
for (const ParsedQuicVersion& version : AllSupportedVersions()) {
QuicEnableVersion(version);
}
@@ -334,7 +324,7 @@
EXPECT_EQ(expected_parsed_versions, AllSupportedVersions());
}
-TEST_F(QuicVersionsTest, FilterSupportedVersionsWithoutFirstVersion) {
+TEST(QuicVersionsTest, FilterSupportedVersionsWithoutFirstVersion) {
for (const ParsedQuicVersion& version : AllSupportedVersions()) {
QuicEnableVersion(version);
}
@@ -348,7 +338,7 @@
FilterSupportedVersions(AllSupportedVersions()));
}
-TEST_F(QuicVersionsTest, LookUpParsedVersionByIndex) {
+TEST(QuicVersionsTest, LookUpParsedVersionByIndex) {
ParsedQuicVersionVector all_versions = AllSupportedVersions();
int version_count = all_versions.size();
for (int i = -5; i <= version_count + 1; ++i) {
@@ -364,7 +354,7 @@
// This test may appear to be so simplistic as to be unnecessary,
// yet a typo was made in doing the #defines and it was caught
// only in some test far removed from here... Better safe than sorry.
-TEST_F(QuicVersionsTest, CheckTransportVersionNumbersForTypos) {
+TEST(QuicVersionsTest, CheckTransportVersionNumbersForTypos) {
static_assert(SupportedVersions().size() == 6u,
"Supported versions out of sync");
EXPECT_EQ(QUIC_VERSION_43, 43);
@@ -375,7 +365,7 @@
EXPECT_EQ(QUIC_VERSION_IETF_2_DRAFT_01, 81);
}
-TEST_F(QuicVersionsTest, AlpnForVersion) {
+TEST(QuicVersionsTest, AlpnForVersion) {
static_assert(SupportedVersions().size() == 6u,
"Supported versions out of sync");
EXPECT_EQ("h3-Q043", AlpnForVersion(ParsedQuicVersion::Q043()));
@@ -386,7 +376,7 @@
EXPECT_EQ("h3", AlpnForVersion(ParsedQuicVersion::V2Draft01()));
}
-TEST_F(QuicVersionsTest, QuicVersionEnabling) {
+TEST(QuicVersionsTest, QuicVersionEnabling) {
for (const ParsedQuicVersion& version : AllSupportedVersions()) {
QuicFlagSaver flag_saver;
QuicDisableVersion(version);
@@ -396,7 +386,7 @@
}
}
-TEST_F(QuicVersionsTest, ReservedForNegotiation) {
+TEST(QuicVersionsTest, ReservedForNegotiation) {
EXPECT_EQ(QUIC_VERSION_RESERVED_FOR_NEGOTIATION,
QuicVersionReservedForNegotiation().transport_version);
// QUIC_VERSION_RESERVED_FOR_NEGOTIATION MUST NOT be supported.
@@ -405,7 +395,7 @@
}
}
-TEST_F(QuicVersionsTest, SupportedVersionsHasCorrectList) {
+TEST(QuicVersionsTest, SupportedVersionsHasCorrectList) {
size_t index = 0;
for (HandshakeProtocol handshake_protocol : SupportedHandshakeProtocols()) {
for (int trans_vers = 255; trans_vers > 0; trans_vers--) {
@@ -423,7 +413,7 @@
EXPECT_EQ(SupportedVersions().size(), index);
}
-TEST_F(QuicVersionsTest, SupportedVersionsAllDistinct) {
+TEST(QuicVersionsTest, SupportedVersionsAllDistinct) {
for (size_t index1 = 0; index1 < SupportedVersions().size(); ++index1) {
ParsedQuicVersion version1 = SupportedVersions()[index1];
for (size_t index2 = index1 + 1; index2 < SupportedVersions().size();
@@ -443,7 +433,7 @@
}
}
-TEST_F(QuicVersionsTest, CurrentSupportedHttp3Versions) {
+TEST(QuicVersionsTest, CurrentSupportedHttp3Versions) {
ParsedQuicVersionVector h3_versions = CurrentSupportedHttp3Versions();
ParsedQuicVersionVector all_current_supported_versions =
CurrentSupportedVersions();
