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

[Bug] Wasm plugin crashed when calling get_http_request_headers() #217

Open
ytsssun opened this issue Jan 12, 2024 · 2 comments
Open

[Bug] Wasm plugin crashed when calling get_http_request_headers() #217

ytsssun opened this issue Jan 12, 2024 · 2 comments

Comments

@ytsssun
Copy link

ytsssun commented Jan 12, 2024

We use proxy-wasm-rust-sdk for our stats plugin. Recently we received a bug report from our customer. By looking into the backtrace, we found that the crash happened within the hostcalls.rs in the SDK.

Error log:

[2024-01-03 23:46:27.247][92][critical][wasm] [source/extensions/common/wasm/context.cc:1157] wasm log aws.appmesh.ingress_http_stats: panicked at /codebuild/output/src3353/src/s3/00/wasm/cargo-project/vendor/proxy-wasm/src/hostcalls.rs:1192:42:
called `Result::unwrap()` on an `Err` value: FromUtf8Error { bytes: [49, 32, 192, 167, 192, 162, 37, 50, 53, 50, 55, 37, 50, 53, 50, 50, 44, 32, 49, 53, 52, 46, 51, 56, 46, 49, 55, 50, 46, 50, 52, 51, 44, 32, 49, 53, 46, 49, 53, 56, 46, 52, 55, 46, 49, 52, 49], error: Utf8Error { valid_up_to: 2, error_len: Some(1) } }
[2024-01-03 23:46:27.247][92][error][wasm] [source/extensions/common/wasm/wasm_vm.cc:38] Function: proxy_on_request_headers failed: Uncaught RuntimeError: unreachable
Proxy-Wasm plugin in-VM backtrace:
0: 0x385ad - rust_panic
1: 0x37842 - std::panicking::rust_panic_with_hook::h3aa054d35a0817d7
2: 0x38a56 - std::panicking::begin_panic_handler::_$u7b$$u7b$closure$u7d$$u7d$::h2f73e4cf6cd6319a
3: 0x3898f - std::sys_common::backtrace::__rust_end_short_backtrace::h98ac61a6abbff7e9
4: 0x24033 - rust_begin_unwind
5: 0x24d1 - core::panicking::panic_fmt::h3e1dd3d08288569e
6: 0x425f - core::result::unwrap_failed::h8b3db0f11171b57b
7: 0x103f3 - proxy_wasm::hostcalls::get_map::hfc84c18de12c4fd4
8: 0xff0e - _$LT$amzn_appmesh_wasm..reader..host..Host$u20$as$u20$amzn_appmesh_wasm..reader..HttpMapReader$GT$::get_request_headers::h60ed0437583308a0
9: 0xd219 - _$LT$amzn_appmesh_wasm..plugin..context..http..HttpFilterContext$u20$as$u20$proxy_wasm..traits..HttpContext$GT$::on_http_request_headers::h706d8f144b9ef225
[93][critical][wasm] [source/extensions/common/wasm/context.cc:1157] wasm log aws.appmesh.egress_http_stats: panicked at /codebuild/output/src3353/src/s3/00/wasm/cargo-project/vendor/proxy-wasm/src/hostcalls.rs:1192:42:

This easily reproducible if I ran following code, see the playground:

#![allow(unused)]
fn main() {
  use std::str;

  // some bytes, in a vector
  let problematic_bytes = vec! [49, 32, 192, 167, 192, 162, 37, 50, 53, 50, 55, 37, 50, 53, 50, 50, 44, 32, 49, 53, 52, 46, 51, 56, 46, 49, 55, 50, 46, 50, 52, 51, 44, 32, 49, 53, 46, 49, 53, 56, 46, 52, 55, 46, 49, 52, 49];

  let result = str::from_utf8(&problematic_bytes).unwrap();
}

Looks like the byte is not following UTF-8 encoding rules, are there any breaking change introduced from Envoy side that would make the bytes invalid UTF-8 encoding?

Would need some help troubleshooting this thanks!

@PiotrSikora
Copy link
Contributor

The issue is that this SDK originally defined get_http_call_response_headers() as returning Vec<(String, String)>, and String is always UTF-8 in Rust, but HTTP header values are not.

You should be using get_http_call_response_headers_bytes() that returns Vec<(String, Bytes)> instead and process Bytes inside the plugin.

@ytsssun
Copy link
Author

ytsssun commented Jan 12, 2024

Thanks for your response. This makes a lot sense! I will use get_http_call_response_headers_bytes() as per your suggestion. Meanwhile, can we deprecate get_http_call_response_headers() given that this may be a known bug?

We also are using these APIs:

get_http_request_trailers()
get_http_response_headers()
get_http_response_trailers()

I assume we should be using the "byte" version of them as well?

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

No branches or pull requests

2 participants