Skip to content
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.

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

filter: clarify comment #13678

Merged
merged 9 commits into from
Oct 27, 2020
Merged

Conversation

rgs1
Copy link
Member

@rgs1 rgs1 commented Oct 21, 2020

The comment currently states that after returning
FilterHeadersStatus::StopIteration, decodeData() or encodeData() might
still be called which isn't possible since the filter chain is stopped.

The only way to unblock the chain is via continueDecoding() or
continueEncoding() from some callback.

Signed-off-by: Raul Gutierrez Segales [email protected]

The comment currently states that after returning
FilterHeadersStatus::StopIteration, decodeData() or encodeData() might
still be called which isn't possible since the filter chain is stopped.

The only way to unblock the the chain is via continueDecoding() or
continueEncoding() from some callback.

Signed-off-by: Raul Gutierrez Segales <[email protected]>
Copy link
Contributor

@asraa asraa left a comment

Choose a reason for hiding this comment

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

Thanks! I wonder if this was a copy paste snafu from FilterDataStatus's comments. 😮
Could you check spelling please?
/wait

@asraa asraa self-assigned this Oct 21, 2020
Signed-off-by: Raul Gutierrez Segales <[email protected]>
@rgs1
Copy link
Member Author

rgs1 commented Oct 21, 2020

Thanks! I wonder if this was a copy paste snafu from FilterDataStatus's comments. 😮
Could you check spelling please?
/wait

ah, it was clang-format not spelling. updated -- thanks!

@rgs1
Copy link
Member Author

rgs1 commented Oct 21, 2020

The macos failure is not format nor spelling, seems transient.

@dio
Copy link
Member

dio commented Oct 22, 2020

/retest

@repokitteh-read-only
Copy link

Retrying Azure Pipelines.
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #13678 (comment) was created by @dio.

see: more, trace.

include/envoy/http/filter.h Outdated Show resolved Hide resolved
Raul Gutierrez Segales added 3 commits October 22, 2020 12:51
This reverts commit 2ab7c30.

Signed-off-by: Raul Gutierrez Segales <[email protected]>
This reverts commit b4ba91f.

Signed-off-by: Raul Gutierrez Segales <[email protected]>
* note that StopIteration refers only to header processing
  (data/body processing still moves forward)
* however, if a local reply is sent than StopIteration
  stops headers/data/trailers for all filters

Signed-off-by: Raul Gutierrez Segales <[email protected]>
include/envoy/http/filter.h Outdated Show resolved Hide resolved
include/envoy/http/filter.h Outdated Show resolved Hide resolved
Raul Gutierrez Segales added 2 commits October 22, 2020 14:20
lizan
lizan previously approved these changes Oct 22, 2020
@asraa asraa merged commit f69a78c into envoyproxy:master Oct 27, 2020
rgs1 pushed a commit to rgs1/envoy that referenced this pull request Nov 25, 2020
Follow-up to envoyproxy#13678.

Signed-off-by: Raul Gutierrez Segales <[email protected]>
mattklein123 pushed a commit that referenced this pull request Nov 28, 2020
Follow-up to #13678.

Signed-off-by: Raul Gutierrez Segales <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants