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

Fixing map deserialization when header name or value is not a valid UTF8 string #188

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

aqua777
Copy link

@aqua777 aqua777 commented Feb 14, 2023

I used Proxy WASM SDK to write an Envoy plugin to perform some manipulations on requests' headers, for ex. analyzing presence and content of JWT.

Recently the Envoy running with this plugin experienced an outage. Some of the requests received by Envoy contained headers with values which were not valid UTF8 strings. As result, the plugin crashed and rendered corresponding Envoy's worker inaccessible.

The example log trace from such crash can be seen below:

[2023-02-12 19:15:47.296][31][critical][wasm] [source/extensions/common/wasm/context.cc:1179] wasm log: panicked at '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], error: Utf8Error { valid_up_to: 2, error_len: Some(1) } }', /tmp/.cargo/registry/src/github.com-1ecc6299db9ec823/proxy-wasm-0.2.0/src/hostcalls.rs:1192:17
[2023-02-12 19:15:47.297][31][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:  0x9e0a6 - rust_panic
  1:  0x48e74 - _ZN3std9panicking20rust_panic_with_hook17h2ed774a221e1fba5E
  2:  0x9e803 - _ZN3std9panicking19begin_panic_handler28_$u7b$$u7b$closure$u7d$$u7d$17h52dc4136c22c8436E
  3:  0x9e73c - _ZN3std10sys_common9backtrace26__rust_end_short_backtrace17h01f9905fbe35a014E
  4:  0x3d1fe - rust_begin_unwind
  5:  0x3e43 - _ZN4core9panicking9panic_fmt17h5e87d704dd0e33d8E
  6:  0x3d0f - _ZN4core6result13unwrap_failed17h4087e6a27003bc2eE
  7:  0x5172 - _ZN10proxy_wasm9hostcalls7get_map17h1c32dd056d4a3c34E
  8:  0x1e78d - _ZN79_$LT$...$u20$as$u20$proxy_wasm..traits..HttpContext$GT$23on_http_request_headers17ha2fbc94ade00c1b5E
  9:  0x4e6df - proxy_on_request_headers

The proposed solution is to replace invalid UTF8 keys and values with arbitrary string, for ex. "?".

If there is a better way of handling such situations please let me know.


Signed-off-by: aqua777 piotr.gridniew@gmail.com

…TF8 string

Signed-off-by: aqua777 <piotr.gridniew@gmail.com>
@aqua777
Copy link
Author

aqua777 commented Mar 28, 2023

@PiotrSikora Hi Piotr, would you be able to review my pull request pls?

@jcrugzz
Copy link

jcrugzz commented Apr 1, 2023

@aqua777 from experience you are better off using get_header_bytes and using String::from_utf8_lossy() (exact apis lookup as im pulling from memory).

Theres definitely improvements we could do for this potentially but its not quite this simple.

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.

None yet

2 participants