Skip to content

ext_authz: Make sure initiateCall only called once#6949

Merged
mattklein123 merged 2 commits intoenvoyproxy:masterfrom
dio:fix-6787
May 20, 2019
Merged

ext_authz: Make sure initiateCall only called once#6949
mattklein123 merged 2 commits intoenvoyproxy:masterfrom
dio:fix-6787

Conversation

@dio
Copy link
Member

@dio dio commented May 15, 2019

Description:
After watermarked, it is possible that decodeData will be called
again. Hence, check if we have finished decoding and do initiateCall
only once.

Risk Level: Low
Testing: Unit tests
Docs Changes: N/A
Release Notes: N/A
Fixes #6787

Signed-off-by: Dhi Aurrahman dio@tetrate.io

After watermarked, it is possible that the decodeData will be called
again. Hence, check if we have finished decoding and do initiateCall
only once.

Signed-off-by: Dhi Aurrahman <dio@tetrate.io>
@dio dio changed the title Make sure initiateCall only called once ext_authz: Make sure initiateCall only called once May 15, 2019
if (end_stream || isBufferFull()) {
ENVOY_STREAM_LOG(debug, "ext_authz filter finished buffering the request", *callbacks_);
initiateCall(*request_headers_);
const bool buffer_is_full = isBufferFull();
Copy link
Member

Choose a reason for hiding this comment

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

shall we guard this in initiateCall?

Copy link
Member Author

Choose a reason for hiding this comment

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

Let me try.

Copy link
Member Author

Choose a reason for hiding this comment

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

Modified in c6b9334.

Signed-off-by: Dhi Aurrahman <dio@tetrate.io>
Copy link
Member

@lizan lizan left a comment

Choose a reason for hiding this comment

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

LGTM, @gsagula?

@gsagula
Copy link
Member

gsagula commented May 17, 2019

Sorry for the delay, looking at this now. Thanks, @dio @lizan

Copy link
Member

@gsagula gsagula left a comment

Choose a reason for hiding this comment

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

@dio Looks good to me. Thanks for doing this.

}

void Filter::initiateCall(const Http::HeaderMap& headers) {
if (filter_return_ == FilterReturn::StopDecoding) {
Copy link
Member

Choose a reason for hiding this comment

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

👍

@mattklein123 mattklein123 merged commit a8e9100 into envoyproxy:master May 20, 2019
mpuncel added a commit to mpuncel/envoy that referenced this pull request May 22, 2019
* master: (65 commits)
  proto: Add PATCH method to RequestMethod enum (envoyproxy#6737)
  exe: drop unused deps on zlib compressor code (envoyproxy#7022)
  coverage: fix some misc coverage (envoyproxy#7033)
  Enable proto schema for router_check_tool (envoyproxy#6992)
  stats: rework stat sink flushing to centralize counter latching (envoyproxy#6996)
  [test] convert lds api test config stubs to v2 (envoyproxy#7021)
  router: scoped rds (2c): implement scoped rds API (envoyproxy#6932)
  build: Add option for size-optimized binary (envoyproxy#6960)
  test: adding an integration test framework for file-based LDS (envoyproxy#6933)
  doc: update obsolete ref to api/XDS_PROTOCOL.md (envoyproxy#7002)
  dispatcher: faster runOnAllThreads (envoyproxy#7011)
  example: add csrf sandbox (envoyproxy#6805)
  fix syntax of gcov exclusion zone. (envoyproxy#7023)
  /runtime_modify: add support for query params in body (envoyproxy#6977)
  stats: Create stats for http codes with the symbol table. (envoyproxy#6733)
  health check: fix more fallout from inline deletion change (envoyproxy#6988)
  Max heap fix (envoyproxy#7016)
  Add support to unregister from lifecycle notifications (envoyproxy#6984)
  build spdy_core_alt_svc_wire_format (envoyproxy#7010)
  ext_authz: Make sure initiateCall only called once (envoyproxy#6949)
  ...

Signed-off-by: Michael Puncel <mpuncel@squareup.com>
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.

Segfault using ext_authz request buffering with allow_partial_message set

4 participants