Skip to content

fix(grpc-transcode): filter out illegal INT(string) formats#11367

Merged
moonming merged 3 commits into
apache:masterfrom
zhoujiexiong:issue-11355
Jul 9, 2024
Merged

fix(grpc-transcode): filter out illegal INT(string) formats#11367
moonming merged 3 commits into
apache:masterfrom
zhoujiexiong:issue-11355

Conversation

@zhoujiexiong

@zhoujiexiong zhoujiexiong commented Jun 21, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

Description

Need to sync. lua-protobuf lib. updated by this PR(starwing/lua-protobuf#269).

Fixes #11355

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)

@zhoujiexiong

zhoujiexiong commented Jun 24, 2024

Copy link
Copy Markdown
Contributor Author

Hi @yzeng25 ,
It seems that other test case unrelated fail the check, please help confirm.

image

@zhoujiexiong

zhoujiexiong commented Jun 24, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Hi @yzeng25 , It seems that other test case unrelated fail the check, please help confirm.

image

Hi @yzeng25 ,
Still like that, unrelated.
image

@yzeng25

yzeng25 commented Jun 27, 2024

Copy link
Copy Markdown
Contributor

Hi @yzeng25 , It seems that other test case unrelated fail the check, please help confirm.
image

Hi @yzeng25 , Still like that, unrelated. image

Let's seek help from others who are more familiar with CIs. @pottekkat @shreemaan-abhishek @nic-6443

@zhoujiexiong

zhoujiexiong commented Jun 27, 2024

Copy link
Copy Markdown
Contributor Author

Hi @yzeng25 , It seems that other test case unrelated fail the check, please help confirm.
image

Hi @yzeng25 , Still like that, unrelated. image

Let's seek help from others who are more familiar with CIs. @pottekkat @shreemaan-abhishek @nic-6443

Checks had passed, reviewers are needed. :-)
image

nic-6443
nic-6443 previously approved these changes Jul 2, 2024
View reviewed changes
shreemaan-abhishek
shreemaan-abhishek reviewed Jul 3, 2024
View reviewed changes
Comment thread t/plugin/grpc-transcode3.t Outdated
Comment thread t/plugin/grpc-transcode3.t Outdated
--pb.option "int64_as_string"
--pb.option "int64_as_hexstring"
pb_encode("IntStringPatterns", supported)
local status, err = pcall(pb_encode, "IntStringPatterns", unsupported)

@shreemaan-abhishek shreemaan-abhishek Jul 3, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think it would be better to avoid using repeated and check all supported/unsupported types in a loop.

@zhoujiexiong zhoujiexiong Jul 3, 2024

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.

@shreemaan-abhishek test case updated, pls. re-check.

@shreemaan-abhishek shreemaan-abhishek Jul 3, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

thank you, I will check this PR after the CI completes run.

@zhoujiexiong zhoujiexiong Jul 3, 2024

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.

The check errors are basically the same.

image

@zhoujiexiong

zhoujiexiong commented Jul 3, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

@shreemaan-abhishek @nic-6443

Hi Pals,
One more reviewer needed, could you help?

Ping @yzeng25 :D

@yzeng25
yzeng25 requested a review from nic-6443 July 4, 2024 01:47
@moonming
moonming merged commit 1164374 into apache:master Jul 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nic-6443 nic-6443 nic-6443 approved these changes
@moonming moonming moonming approved these changes
@shreemaan-abhishek shreemaan-abhishek shreemaan-abhishek approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@zhoujiexiong @yzeng25 @nic-6443 @moonming @shreemaan-abhishek