Make writing Prioritized Element ID and Element Dependency ID optional when handling priority.
These fields are specified to be absent when priority type or dependency type is ROOT_OF_TREE.
On the decoding side, the absent fields will be parsed as 0 for default value. Those fields won't be used when later the frame is processed.
gfe-relnote: v99 only, not flag protected.
PiperOrigin-RevId: 254072432
Change-Id: I336e06b39d89ff01507e54818dafa67923e96e4c
diff --git a/quic/core/http/http_decoder.cc b/quic/core/http/http_decoder.cc
index f08d2ff..4513f04 100644
--- a/quic/core/http/http_decoder.cc
+++ b/quic/core/http/http_decoder.cc
@@ -519,11 +519,13 @@
frame->dependency_type =
static_cast<PriorityElementType>(ExtractBits(flags, 2, 4));
frame->exclusive = flags % 2 == 1;
- if (!reader->ReadVarInt62(&frame->prioritized_element_id)) {
+ if (frame->prioritized_type != ROOT_OF_TREE &&
+ !reader->ReadVarInt62(&frame->prioritized_element_id)) {
RaiseError(QUIC_INTERNAL_ERROR, "Unable to read prioritized_element_id");
return false;
}
- if (!reader->ReadVarInt62(&frame->element_dependency_id)) {
+ if (frame->dependency_type != ROOT_OF_TREE &&
+ !reader->ReadVarInt62(&frame->element_dependency_id)) {
RaiseError(QUIC_INTERNAL_ERROR, "Unable to read element_dependency_id");
return false;
}
diff --git a/quic/core/http/http_decoder_test.cc b/quic/core/http/http_decoder_test.cc
index b99689f..c25d086 100644
--- a/quic/core/http/http_decoder_test.cc
+++ b/quic/core/http/http_decoder_test.cc
@@ -343,6 +343,26 @@
EXPECT_EQ(QUIC_NO_ERROR, decoder_.error());
EXPECT_EQ("", decoder_.error_detail());
+ char input2[] = {// type (PRIORITY)
+ 0x2,
+ // length
+ 0x2,
+ // root of tree, root of tree, exclusive
+ 0xf1,
+ // weight
+ 0xFF};
+ PriorityFrame frame2;
+ frame2.prioritized_type = ROOT_OF_TREE;
+ frame2.dependency_type = ROOT_OF_TREE;
+ frame2.exclusive = true;
+ frame2.weight = 0xFF;
+
+ EXPECT_CALL(visitor_, OnPriorityFrame(frame2));
+ EXPECT_EQ(QUIC_ARRAYSIZE(input2),
+ decoder_.ProcessInput(input2, QUIC_ARRAYSIZE(input2)));
+ EXPECT_EQ(QUIC_NO_ERROR, decoder_.error());
+ EXPECT_EQ("", decoder_.error_detail());
+
// Test on the situation when the visitor wants to stop processing.
EXPECT_CALL(visitor_, OnPriorityFrame(frame)).WillOnce(Return(false));
EXPECT_EQ(0u, decoder_.ProcessInput(input, QUIC_ARRAYSIZE(input)));
diff --git a/quic/core/http/http_encoder.cc b/quic/core/http/http_encoder.cc
index 4505dc7..2b8be8e 100644
--- a/quic/core/http/http_encoder.cc
+++ b/quic/core/http/http_encoder.cc
@@ -3,6 +3,7 @@
// found in the LICENSE file.
#include "net/third_party/quiche/src/quic/core/http/http_encoder.h"
+
#include "net/third_party/quiche/src/quic/core/quic_data_writer.h"
namespace quic {
@@ -43,6 +44,8 @@
static const size_t kPriorityWeightLength = 1;
// Length of a priority frame's first byte.
static const size_t kPriorityFirstByteLength = 1;
+// The bit that indicates Priority frame is exclusive.
+static const uint8_t kPriorityExclusiveBit = 1;
} // namespace
@@ -95,8 +98,12 @@
std::unique_ptr<char[]>* output) {
QuicByteCount payload_length =
kPriorityFirstByteLength +
- QuicDataWriter::GetVarInt62Len(priority.prioritized_element_id) +
- QuicDataWriter::GetVarInt62Len(priority.element_dependency_id) +
+ (priority.prioritized_type == ROOT_OF_TREE
+ ? 0
+ : QuicDataWriter::GetVarInt62Len(priority.prioritized_element_id)) +
+ (priority.dependency_type == ROOT_OF_TREE
+ ? 0
+ : QuicDataWriter::GetVarInt62Len(priority.element_dependency_id)) +
kPriorityWeightLength;
QuicByteCount total_length =
GetTotalLength(payload_length, HttpFrameType::PRIORITY);
@@ -111,16 +118,14 @@
}
// Set the first byte of the payload.
- uint8_t bits = 0;
- bits = SetPriorityFields(bits, priority.prioritized_type, true);
- bits = SetPriorityFields(bits, priority.dependency_type, false);
+ uint8_t firstByte = 0;
+ firstByte = SetPriorityFields(firstByte, priority.prioritized_type, true);
+ firstByte = SetPriorityFields(firstByte, priority.dependency_type, false);
if (priority.exclusive) {
- bits |= 1;
+ firstByte |= kPriorityExclusiveBit;
}
- if (writer.WriteUInt8(bits) &&
- writer.WriteVarInt62(priority.prioritized_element_id) &&
- writer.WriteVarInt62(priority.element_dependency_id) &&
+ if (writer.WriteUInt8(firstByte) && MaybeWriteIds(priority, &writer) &&
writer.WriteUInt8(priority.weight)) {
return total_length;
}
@@ -282,4 +287,27 @@
payload_length;
}
+bool HttpEncoder::MaybeWriteIds(const PriorityFrame& priority,
+ QuicDataWriter* writer) {
+ if (priority.prioritized_type != ROOT_OF_TREE) {
+ if (!writer->WriteVarInt62(priority.prioritized_element_id)) {
+ return false;
+ }
+ } else {
+ DCHECK_EQ(0u, priority.prioritized_element_id)
+ << "Prioritized element id should be 0 when prioritized type is "
+ "ROOT_OF_TREE";
+ }
+ if (priority.dependency_type != ROOT_OF_TREE) {
+ if (!writer->WriteVarInt62(priority.element_dependency_id)) {
+ return false;
+ }
+ } else {
+ DCHECK_EQ(0u, priority.element_dependency_id)
+ << "Element dependency id should be 0 when dependency type is "
+ "ROOT_OF_TREE";
+ }
+ return true;
+}
+
} // namespace quic
diff --git a/quic/core/http/http_encoder.h b/quic/core/http/http_encoder.h
index a3c1f4c..12c5bab 100644
--- a/quic/core/http/http_encoder.h
+++ b/quic/core/http/http_encoder.h
@@ -76,6 +76,9 @@
QuicByteCount GetTotalLength(QuicByteCount payload_length,
HttpFrameType type);
+
+ // Write prioritized element id and element dependency id if needed.
+ bool MaybeWriteIds(const PriorityFrame& priority, QuicDataWriter* writer);
};
} // namespace quic
diff --git a/quic/core/http/http_encoder_test.cc b/quic/core/http/http_encoder_test.cc
index 6a029d8..d0696ed 100644
--- a/quic/core/http/http_encoder_test.cc
+++ b/quic/core/http/http_encoder_test.cc
@@ -3,6 +3,7 @@
// found in the LICENSE file.
#include "net/third_party/quiche/src/quic/core/http/http_encoder.h"
+
#include "net/third_party/quiche/src/quic/platform/api/quic_arraysize.h"
#include "net/third_party/quiche/src/quic/platform/api/quic_test.h"
#include "net/third_party/quiche/src/quic/test_tools/quic_test_utils.h"
@@ -68,6 +69,45 @@
EXPECT_EQ(QUIC_ARRAYSIZE(output), length);
CompareCharArraysWithHexError("PRIORITY", buffer.get(), length, output,
QUIC_ARRAYSIZE(output));
+
+ PriorityFrame priority2;
+ priority2.prioritized_type = ROOT_OF_TREE;
+ priority2.dependency_type = REQUEST_STREAM;
+ priority2.exclusive = true;
+ priority2.element_dependency_id = 0x04;
+ priority2.weight = 0xFF;
+ char output2[] = {// type (PRIORIRTY)
+ 0x2,
+ // length
+ 0x3,
+ // root of tree, request stream, exclusive
+ 0xc1,
+ // element_dependency_id
+ 0x04,
+ // weight
+ 0xff};
+ length = encoder_.SerializePriorityFrame(priority2, &buffer);
+ EXPECT_EQ(QUIC_ARRAYSIZE(output2), length);
+ CompareCharArraysWithHexError("PRIORITY", buffer.get(), length, output2,
+ QUIC_ARRAYSIZE(output2));
+
+ PriorityFrame priority3;
+ priority3.prioritized_type = ROOT_OF_TREE;
+ priority3.dependency_type = ROOT_OF_TREE;
+ priority3.exclusive = true;
+ priority3.weight = 0xFF;
+ char output3[] = {// type (PRIORITY)
+ 0x2,
+ // length
+ 0x2,
+ // root of tree, root of tree, exclusive
+ 0xf1,
+ // weight
+ 0xff};
+ length = encoder_.SerializePriorityFrame(priority3, &buffer);
+ EXPECT_EQ(QUIC_ARRAYSIZE(output3), length);
+ CompareCharArraysWithHexError("PRIORITY", buffer.get(), length, output3,
+ QUIC_ARRAYSIZE(output3));
}
TEST_F(HttpEncoderTest, SerializeCancelPushFrame) {
diff --git a/quic/core/http/http_frames.h b/quic/core/http/http_frames.h
index f1aa7ff..ff59614 100644
--- a/quic/core/http/http_frames.h
+++ b/quic/core/http/http_frames.h
@@ -53,12 +53,12 @@
};
struct PriorityFrame {
- PriorityElementType prioritized_type;
- PriorityElementType dependency_type;
- bool exclusive;
- uint64_t prioritized_element_id;
- uint64_t element_dependency_id;
- uint8_t weight;
+ PriorityElementType prioritized_type = REQUEST_STREAM;
+ PriorityElementType dependency_type = REQUEST_STREAM;
+ bool exclusive = false;
+ uint64_t prioritized_element_id = 0;
+ uint64_t element_dependency_id = 0;
+ uint8_t weight = 0;
bool operator==(const PriorityFrame& rhs) const {
return prioritized_type == rhs.prioritized_type &&
