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

fix: Wasm-rust plugin crashed when header name or value is not a valid UTF8 string #1295

Merged
merged 2 commits into from
Sep 10, 2024

Conversation

jaymie9019
Copy link
Contributor

Ⅰ. Describe what this PR did

优化 wasm rust sdk 中 PluginHttpWrapper req_header 获取方方式,避免因为不规范的 http header 导致 plugin panic,并且打印不规范的 header 日志。已在生产环境验证

Ⅱ. Does this pull request fix one issue?

fixes #1280

Ⅲ. Why don't you add test cases (unit test/integration test)?

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

@CLAassistant
Copy link

CLAassistant commented Sep 10, 2024

CLA assistant check
All committers have signed the CLA.

@007gzs
Copy link
Collaborator

007gzs commented Sep 10, 2024

response_headers 也可以加上

@jaymie9019
Copy link
Contributor Author

response_headers 也可以加上

ok 已添加

@CH3CHO
Copy link
Collaborator

CH3CHO commented Sep 10, 2024

上面的 CLA 麻烦签署一下。 @jaymie9019

@jaymie9019
Copy link
Contributor Author

上面的 CLA 麻烦签署一下。 @jaymie9019

有点尴尬,我一开始用了我公司的云效账号的 git 账号和邮箱,后来我发现了,第二次提交改了本地仓库的用户和邮箱。 xiaoma 这个用户并没有 github 账号。 需要把整个分支删除,然后重新提交一次代码吗

@CH3CHO
Copy link
Collaborator

CH3CHO commented Sep 10, 2024

上面的 CLA 麻烦签署一下。 @jaymie9019

有点尴尬,我一开始用了我公司的云效账号的 git 账号和邮箱,后来我发现了,第二次提交改了本地仓库的用户和邮箱。 xiaoma 这个用户并没有 github 账号。 需要把整个分支删除,然后重新提交一次代码吗

commit 信息可以改的,你rebase一下或者reset一下之后force push

@jaymie9019 jaymie9019 force-pushed the fix/wasm-rust-panic branch 2 times, most recently from 6054f95 to a041f64 Compare September 10, 2024 08:55
@codecov-commenter
Copy link

codecov-commenter commented Sep 10, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 44.31%. Comparing base (ef31e09) to head (4b3a28a).
Report is 90 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1295      +/-   ##
==========================================
+ Coverage   35.91%   44.31%   +8.40%     
==========================================
  Files          69       75       +6     
  Lines       11576     9823    -1753     
==========================================
+ Hits         4157     4353     +196     
+ Misses       7104     5142    -1962     
- Partials      315      328      +13     

see 90 files with indirect coverage changes

@007gzs
Copy link
Collaborator

007gzs commented Sep 10, 2024

cargo fmtcargo clippy先处理下

jaymie9019 and others added 2 commits September 10, 2024 19:15
优化 PluginHttpWrapper req_header 与 resp_header 的获取方方式,避免因为不规范的 http header 导致 plugin panic,并且打印不规范的 header 日志
Copy link
Collaborator

@007gzs 007gzs left a comment

Choose a reason for hiding this comment

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

LGTM

@007gzs 007gzs changed the title fix(wasm): 优化 req header 获取 fix: Wasm-rust plugin crashed when header name or value is not a valid UTF8 string Sep 10, 2024
@007gzs 007gzs merged commit ffe3ace into alibaba:main Sep 10, 2024
12 checks passed
@jaymie9019
Copy link
Contributor Author

LGTM

thanks~

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.

rust wasm plugin painc
6 participants