Skip to content

wasm: fix crash on HTTP callout to a bad cluster.#15311

Merged
lizan merged 2 commits intoenvoyproxy:mainfrom
PiotrSikora:wasm-fix_http_call
Mar 10, 2021
Merged

wasm: fix crash on HTTP callout to a bad cluster.#15311
lizan merged 2 commits intoenvoyproxy:mainfrom
PiotrSikora:wasm-fix_http_call

Conversation

@PiotrSikora
Copy link
Copy Markdown
Contributor

Fixes #14878, proxy-wasm/proxy-wasm-cpp-sdk#81.

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

Comment on lines +722 to +723
// Simulate code path for "no healthy host for HTTP connection pool" inline callback.
cb.onSuccess(request, std::move(response));
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.

To your original question as to why this is "success" vs. failure I think it's just historical. It probably should be a failure in the async client case.

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.

What's even more surprising is that AsyncClient::send() calls onSuccess() inline, before returning nullptr.

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.

Again, historical. Happy to have this code be improved/changed if you are interested.

@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
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.

Thanks!

@lizan lizan merged commit b9b95f8 into envoyproxy:main Mar 10, 2021
@Shikugawa Shikugawa added backport/approved Approved backports to stable releases and removed backport/review Request to backport to stable releases labels Mar 10, 2021
Shikugawa pushed a commit to Shikugawa/envoy that referenced this pull request Mar 10, 2021
Fixes envoyproxy#14878, proxy-wasm/proxy-wasm-cpp-sdk#81.

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
Fixes #14878, proxy-wasm/proxy-wasm-cpp-sdk#81.

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.

WASM httpCall() problems if cluster of type ORIGINAL_DST

5 participants