Skip to content

wasm: fix getHeaderMapValue() in case of early client disconnect.#15224

Merged
lizan merged 1 commit intoenvoyproxy:mainfrom
PiotrSikora:wasm-get_header_map_value
Mar 2, 2021
Merged

wasm: fix getHeaderMapValue() in case of early client disconnect.#15224
lizan merged 1 commit intoenvoyproxy:mainfrom
PiotrSikora:wasm-get_header_map_value

Conversation

@PiotrSikora
Copy link
Copy Markdown
Contributor

Previously, getHeaderMapValuei() would incorrectly return BadArgument
when called in the access log phase after an early client disconnect,
because the code assumed that a HeaderMap can point to a nullptr only
when called from a callback in which given map is not available.

Fixes proxy-wasm/proxy-wasm-rust-sdk#82.

Signed-off-by: Piotr Sikora piotrsikora@google.com

Previously, getHeaderMapValuei() would incorrectly return BadArgument
when called in the access log phase after an early client disconnect,
because the code assumed that a HeaderMap can point to a nullptr only
when called from a callback in which given map is not available.

Fixes proxy-wasm/proxy-wasm-rust-sdk#82.

Signed-off-by: Piotr Sikora <piotrsikora@google.com>
if (!map) {
if (access_log_phase_) {
// Maps might point to nullptr in the access log phase.
if (wasm()->abiVersion() == proxy_wasm::AbiVersion::ProxyWasm_0_1_0) {
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.

Is it necessary to return Ok on 0.1.0?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, see the happy path a few lines below.

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.

I see. Thanks.

Copy link
Copy Markdown
Member

@mathetake mathetake left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@lizan lizan merged commit 1533857 into envoyproxy:main Mar 2, 2021
@PiotrSikora
Copy link
Copy Markdown
Contributor Author

/backport

@repokitteh-read-only repokitteh-read-only Bot added the backport/review Request to backport to stable releases label Mar 6, 2021
@Shikugawa Shikugawa added backport/approved Approved backports to stable releases and removed backport/review Request to backport to stable releases labels Mar 8, 2021
Shikugawa pushed a commit to Shikugawa/envoy that referenced this pull request Mar 10, 2021
…voyproxy#15224)

Previously, getHeaderMapValuei() would incorrectly return BadArgument
when called in the access log phase after an early client disconnect,
because the code assumed that a HeaderMap can point to a nullptr only
when called from a callback in which given map is not available.

Fixes proxy-wasm/proxy-wasm-rust-sdk#82.

Signed-off-by: Piotr Sikora <piotrsikora@google.com>
Signed-off-by: Shikugawa <rei@tetrate.io>
Shikugawa added a commit that referenced this pull request Mar 15, 2021
…5224) (#15395)

Previously, getHeaderMapValuei() would incorrectly return BadArgument
when called in the access log phase after an early client disconnect,
because the code assumed that a HeaderMap can point to a nullptr only
when called from a callback in which given map is not available.

Fixes proxy-wasm/proxy-wasm-rust-sdk#82.

Signed-off-by: Piotr Sikora <piotrsikora@google.com>
Signed-off-by: Shikugawa <rei@tetrate.io>

Co-authored-by: Piotr Sikora <piotrsikora@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport/approved Approved backports to stable releases

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Error in hostcalls functions causes filter to panic

4 participants