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

[FIXED] Consumer deliver subject incorrect when imported and crossing a route. #3017

Merged
merged 3 commits into from
Apr 9, 2022

Conversation

derekcollison
Copy link
Member

Signed-off-by: Derek Collison derek@nats.io

/cc @nats-io/core

…sage crossed a route we dropped the delivered subject.

Signed-off-by: Derek Collison <derek@nats.io>
Signed-off-by: Derek Collison <derek@nats.io>
Copy link
Contributor

@matthiashanel matthiashanel left a comment

Choose a reason for hiding this comment

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

LGTM.
Question with @ being a special (internal) character, do we make sure client's can't publish with it? Would pub foo@bar be received as bar ?

@derekcollison
Copy link
Member Author

It's not on the subject, its tacked onto the reply subject and we pull that out and set local state and switch on last mile.

Copy link
Contributor

@matthiashanel matthiashanel left a comment

Choose a reason for hiding this comment

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

last minute question, leaf and gateway do not need to be considered right?

@@ -4161,6 +4163,11 @@ func (c *client) processMsgResults(acc *Account, r *SublistResult, msg, deliver,
} else {
dsubj = append(_dsubj[:0], sub.im.to...)
}

// Make sure deliver is set if inbound from a route.
if remapped && c.kind == ROUTER {
Copy link
Contributor

Choose a reason for hiding this comment

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

What about Leaf and Gateway?

Copy link
Member Author

Choose a reason for hiding this comment

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

Possible, just trying to get the fix to pass which it does, and not break any existing tests.

Signed-off-by: Derek Collison <derek@nats.io>
Copy link
Member

@kozlovic kozlovic left a comment

Choose a reason for hiding this comment

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

LGTM

@derekcollison derekcollison merged commit 4f83736 into main Apr 9, 2022
@derekcollison derekcollison deleted the consumer_subj_rewrite branch April 9, 2022 19:26
matthiashanel added a commit that referenced this pull request Apr 11, 2022
… gateway

followup to #3017

Signed-off-by: Matthias Hanel <mh@synadia.com>
matthiashanel added a commit that referenced this pull request Apr 12, 2022
… gateway (#3025)

followup to #3017

Signed-off-by: Matthias Hanel <mh@synadia.com>
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

3 participants