Skip to content

fix(kafka-pubsub): restore pb.state appropriately#11135

Merged
shreemaan-abhishek merged 4 commits into
apache:masterfrom
shreemaan-abhishek:fix/kafka-pubsub
Apr 10, 2024
Merged

fix(kafka-pubsub): restore pb.state appropriately#11135
shreemaan-abhishek merged 4 commits into
apache:masterfrom
shreemaan-abhishek:fix/kafka-pubsub

Conversation

@shreemaan-abhishek

@shreemaan-abhishek shreemaan-abhishek commented Apr 9, 2024

Copy link
Copy Markdown
Contributor

Description

pb state should be restored before doing operations like pb.encode() as the state might have been changed/polluted by code in other places that use the pb module.

Fixes #10938

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)

@shreemaan-abhishek
shreemaan-abhishek marked this pull request as ready for review April 10, 2024 03:16
Comment thread apisix/core/pubsub.lua
local pb_old_state = pb.state(pb_state)

-- only recover state if it has changed
if pb.state() ~= pb_state then

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.

It is a good idea to add a check to make sure that the same pb.state is indeed equal. i.e. verifying that == can indeed be used to check for equality.
I'm concerned that this behavior may be broken by updates to openresty and luajit.

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.

we can deal with that later.

@shreemaan-abhishek
shreemaan-abhishek merged commit c7c70e4 into apache:master Apr 10, 2024
@shreemaan-abhishek
shreemaan-abhishek deleted the fix/kafka-pubsub branch April 10, 2024 17:23
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: PubSub/Kafka protobuf decode error when sending response

3 participants