Skip to content

fix: plugin jwe-decrypt decryption failure#10843

Merged
juststillthinking merged 11 commits into
apache:masterfrom
hanqingwu:fix-10835
Jan 24, 2024
Merged

fix: plugin jwe-decrypt decryption failure#10843
juststillthinking merged 11 commits into
apache:masterfrom
hanqingwu:fix-10835

Conversation

@hanqingwu

@hanqingwu hanqingwu commented Jan 18, 2024

Copy link
Copy Markdown
Contributor

Description

Fixes #10835

Checklist

  • I have explained the need for this PR and the problem it solves
  • I have explained the changes or the new features added to this PR
  • I have added tests corresponding to this change
  • I have updated the documentation to reflect this change
  • I have verified that this change is backward compatible (If not, please discuss on the APISIX mailing list first)

@juststillthinking

Copy link
Copy Markdown
Contributor

Could you add test cases for this?

@hanqingwu

Copy link
Copy Markdown
Contributor Author

@kayx23 , Any idea about how to do this test case ?

@Vacant2333

Copy link
Copy Markdown
Contributor

is this pr ready to review now? @monkeyDluffy6017 pls help run the ci

@kayx23

kayx23 commented Jan 18, 2024

Copy link
Copy Markdown
Member

@hanqingwu please check the failed tests, thanks.

@kayx23 kayx23 changed the title fix: plugin jwe-decrypt decrypt failed #10835 fix: plugin jwe-decrypt decryption failure Jan 18, 2024
@hanqingwu

Copy link
Copy Markdown
Contributor Author

@kayx23 , the test case directly check heads Authorization is not suitable. So maybe we should think about how to do this If we need add testcase .

@kayx23

kayx23 commented Jan 19, 2024

Copy link
Copy Markdown
Member

Maybe use an upstream that returns the headers? I see httpbun.org is used in other test files. https://httpbun.com

@juststillthinking

Copy link
Copy Markdown
Contributor

please make the ci pass

@hanqingwu

Copy link
Copy Markdown
Contributor Author

@monkeyDluffy6017 , Can you telll me how to debug test case in my local env ?

@hanqingwu

Copy link
Copy Markdown
Contributor Author

@shreemaan-abhishek, thanks!

@hanqingwu

Copy link
Copy Markdown
Contributor Author

@monkeyDluffy6017 , I have add test case , please review it.

@juststillthinking

Copy link
Copy Markdown
Contributor

please merge the master branch

@hanqingwu

Copy link
Copy Markdown
Contributor Author

@monkeyDluffy6017 , Have merged

@juststillthinking
juststillthinking merged commit ee209a7 into apache:master Jan 24, 2024
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.

bug: jwe-decrypt Authorization header not showing up

5 participants