Skip to content

[chttp2] Tarpit invalid requests#34641

Merged
ctiller merged 19 commits intogrpc:masterfrom
ctiller:tarpit
Oct 11, 2023
Merged

[chttp2] Tarpit invalid requests#34641
ctiller merged 19 commits intogrpc:masterfrom
ctiller:tarpit

Conversation

@ctiller
Copy link
Copy Markdown
Member

@ctiller ctiller commented Oct 10, 2023

If a request is invalid, take a random amount of time before sending the RST_STREAM, so that MAX_CONCURRENT_STREAMS remaining becomes unpredictable.

auto* arena = GetContext<Arena>();
auto hdl = arena->MakePooled<ServerMetadata>(arena);
hdl->Set(GrpcStatusMetadata(), GRPC_STATUS_UNKNOWN);
hdl->Set(GrpcMessageMetadata(), Slice::FromCopiedString(explanation));
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couldn't we use FromStaticString() in this case?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

// Error contract: the transport that gets this op must cause cancel_error
// to be unref'ed after processing it
grpc_error_handle cancel_error;
bool tarpit = false;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please document this field.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

RemovedStreamHandle() = default;
explicit RemovedStreamHandle(
grpc_core::RefCountedPtr<grpc_chttp2_transport> t)
: transport_(t) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

std::move()?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@ctiller ctiller added the release notes: yes Indicates if PR needs to be in release notes label Oct 10, 2023
@ctiller ctiller merged commit e527581 into grpc:master Oct 11, 2023
@copybara-service copybara-service bot added the imported Specifies if the PR has been imported to the internal repository label Oct 11, 2023
ctiller added a commit to ctiller/grpc that referenced this pull request Oct 20, 2023
If a request is invalid, take a random amount of time before sending the
RST_STREAM, so that MAX_CONCURRENT_STREAMS remaining becomes
unpredictable.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bloat/low imported Specifies if the PR has been imported to the internal repository lang/core per-call-memory/neutral per-channel-memory/neutral release notes: yes Indicates if PR needs to be in release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants