Skip to content

thrift_proxy: support header flags #18806

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
zuercher merged 5 commits into from
Nov 3, 2021
Merged

thrift_proxy: support header flags #18806

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
1f9cbf1
support header flags
xiaoma2015 Oct 28, 2021
bbf776f
update code
xiaoma2015 Nov 1, 2021
4c7ba0e
Merge branch 'main' of github.com:xiaoma2015/envoy into thrift_header…
xiaoma2015 Nov 1, 2021
72a645c
update doc
xiaoma2015 Nov 1, 2021
f7aec50
update doc
xiaoma2015 Nov 1, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .bazelrc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ build --action_env=CC
build --action_env=CXX
build --action_env=LLVM_CONFIG
build --action_env=PATH
build --action_env=GOPROXY=https://goproxy.cn
Comment thread
xiaoma2015 marked this conversation as resolved.
Outdated

# Common flags for sanitizers
build:sanitizer --define tcmalloc=disabled
Expand Down
11 changes: 9 additions & 2 deletions source/extensions/filters/network/thrift_proxy/header_transport_impl.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,9 @@ bool HeaderTransportImpl::decodeFrameStart(Buffer::Instance& buffer, MessageMeta
throw EnvoyException(fmt::format("invalid thrift header transport magic {:04x}", magic));
}

// offset 6: 16 bit flags field, unused
// offset 6: 16 bit flags field
int16_t header_flags = buffer.peekBEInt<int16_t>(6);

// offset 8: 32 bit sequence number field
int32_t seq_id = buffer.peekBEInt<int32_t>(8);

Expand Down Expand Up @@ -92,6 +94,7 @@ bool HeaderTransportImpl::decodeFrameStart(Buffer::Instance& buffer, MessageMeta
// (header_size).
metadata.setFrameSize(
static_cast<uint32_t>(frame_size - header_size - MinFrameStartSizeNoHeaders));
metadata.setHeaderFlags(header_flags);
metadata.setSequenceId(seq_id);

ProtocolType proto = ProtocolType::Auto;
Expand Down Expand Up @@ -237,10 +240,14 @@ void HeaderTransportImpl::encodeFrame(Buffer::Instance& buffer, const MessageMet
if (metadata.hasSequenceId()) {
seq_id = metadata.sequenceId();
}
int16_t header_flags = 0;
if (metadata.hasHeaderFlags()) {
header_flags = metadata.headerFlags();
}

buffer.writeBEInt<uint32_t>(static_cast<uint32_t>(size));
buffer.writeBEInt<uint16_t>(Magic);
buffer.writeBEInt<uint16_t>(0); // flags
buffer.writeBEInt<uint16_t>(header_flags); // flags
buffer.writeBEInt<int32_t>(seq_id);
buffer.writeBEInt<uint16_t>(static_cast<uint16_t>(header_size / 4));

Expand Down
5 changes: 5 additions & 0 deletions source/extensions/filters/network/thrift_proxy/metadata.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,10 @@ class MessageMetadata {
const std::string& methodName() const { return method_name_.value(); }
void setMethodName(const std::string& method_name) { method_name_ = method_name; }

bool hasHeaderFlags() const { return header_flags_.has_value(); }
int16_t headerFlags() const { return header_flags_.value(); }
void setHeaderFlags(int16_t header_flags) { header_flags_ = header_flags; }

bool hasSequenceId() const { return seq_id_.has_value(); }
int32_t sequenceId() const { return seq_id_.value(); }
void setSequenceId(int32_t seq_id) { seq_id_ = seq_id; }
Expand Down Expand Up @@ -174,6 +178,7 @@ class MessageMetadata {
absl::optional<uint32_t> frame_size_{};
absl::optional<ProtocolType> proto_{};
absl::optional<std::string> method_name_{};
absl::optional<int16_t> header_flags_{};
absl::optional<int32_t> seq_id_{};
absl::optional<MessageType> msg_type_{};
absl::optional<ReplyType> reply_type_{};
Expand Down
9 changes: 6 additions & 3 deletions test/extensions/filters/network/thrift_proxy/header_transport_impl_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -243,13 +243,14 @@ TEST(HeaderTransportTest, NoTransformsOrInfo) {

buffer.writeBEInt<int32_t>(100);
buffer.writeBEInt<int16_t>(0x0FFF);
buffer.writeBEInt<int16_t>(0);
buffer.writeBEInt<int16_t>(1); // header flags
buffer.writeBEInt<int32_t>(1); // sequence number
buffer.writeBEInt<int16_t>(1); // size 4
addSeq(buffer, {0, 0, 0, 0}); // 0 = binary proto, 0 = num transforms, pad, pad
EXPECT_TRUE(transport.decodeFrameStart(buffer, metadata));
EXPECT_THAT(metadata, HasFrameSize(86U));
EXPECT_THAT(metadata, HasProtocol(ProtocolType::Binary));
EXPECT_THAT(metadata, HasHeaderFlags(1));
EXPECT_THAT(metadata, HasSequenceId(1));
EXPECT_THAT(metadata, HasNoHeaders());
EXPECT_EQ(buffer.length(), 0);
Expand All @@ -261,13 +262,14 @@ TEST(HeaderTransportTest, NoTransformsOrInfo) {

buffer.writeBEInt<int32_t>(101);
buffer.writeBEInt<int16_t>(0x0FFF);
buffer.writeBEInt<int16_t>(0);
buffer.writeBEInt<int16_t>(2); // header flags
buffer.writeBEInt<int32_t>(2); // sequence number
buffer.writeBEInt<int16_t>(1); // size 4
addSeq(buffer, {2, 0, 0, 0}); // 2 = compact proto, 0 = num transforms, pad, pad
EXPECT_TRUE(transport.decodeFrameStart(buffer, metadata));
EXPECT_THAT(metadata, HasFrameSize(87U));
EXPECT_THAT(metadata, HasProtocol(ProtocolType::Compact));
EXPECT_THAT(metadata, HasHeaderFlags(2));
EXPECT_THAT(metadata, HasSequenceId(2));
EXPECT_THAT(metadata, HasNoHeaders());
}
Expand Down Expand Up @@ -341,7 +343,7 @@ TEST(HeaderTransportTest, InvalidInfoBlock) {

buffer.writeBEInt<int32_t>(100);
buffer.writeBEInt<int16_t>(0x0FFF);
buffer.writeBEInt<int16_t>(0);
buffer.writeBEInt<int16_t>(1); // header flags
buffer.writeBEInt<int32_t>(1); // sequence number
buffer.writeBEInt<int16_t>(1); // size 4
addSeq(buffer, {0, 0, 2, 0}); // 0 = binary proto, 0 = num transforms, 2 = unknown info id, pad
Expand All @@ -350,6 +352,7 @@ TEST(HeaderTransportTest, InvalidInfoBlock) {
EXPECT_TRUE(transport.decodeFrameStart(buffer, metadata));
EXPECT_THAT(metadata, HasFrameSize(86U));
EXPECT_THAT(metadata, HasProtocol(ProtocolType::Binary));
EXPECT_THAT(metadata, HasHeaderFlags(1));
EXPECT_THAT(metadata, HasSequenceId(1));
EXPECT_THAT(metadata, HasNoHeaders());
EXPECT_EQ(buffer.length(), 0);
Expand Down
1 change: 1 addition & 0 deletions test/extensions/filters/network/thrift_proxy/utility.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,7 @@ MATCHER_P(HasFrameSize, n, "") {

MATCHER_P(HasProtocol, p, "") { return arg.hasProtocol() && arg.protocol() == p; }
MATCHER_P(HasSequenceId, id, "") { return arg.hasSequenceId() && arg.sequenceId() == id; }
MATCHER_P(HasHeaderFlags, flags, "") { return arg.hasHeaderFlags() && arg.headerFlags() == flags; }
MATCHER(HasNoHeaders, "") { return arg.headers().size() == 0; }

MATCHER_P2(HasAppException, t, m, "") {
Expand Down