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] JetStream: check stream and consumer names #947

Merged
merged 1 commit into from Apr 4, 2022

Conversation

kozlovic
Copy link
Member

@kozlovic kozlovic commented Apr 4, 2022

Many APIs were not properly checking that the stream and/or
consumer names were provided and valid (no "." in them), which
could lead to requests being sent on a subject that the server
has no interest on and no response was therefore sent to the
client library, which would fail with a timeout/context deadline.

Signed-off-by: Ivan Kozlovic ivan@synadia.com

@coveralls
Copy link

coveralls commented Apr 4, 2022

Coverage Status

Coverage increased (+0.06%) to 85.249% when pulling 3e2512a on js_stream_and_cons_name_check into e076b0d on main.

jsm.go Show resolved Hide resolved
nats.go Show resolved Hide resolved
Many APIs were not properly checking that the stream and/or
consumer names were provided and valid (no "." in them), which
could lead to requests being sent on a subject that the server
has no interest on and no response was therefore sent to the
client library, which would fail with a timeout/context deadline.

Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
@kozlovic kozlovic force-pushed the js_stream_and_cons_name_check branch from e54d1b2 to 3e2512a Compare April 4, 2022 20:10
@kozlovic kozlovic changed the title [FIXED] JetStream: check consumer name validity in APIs [FIXED] JetStream: check stream and consumer names Apr 4, 2022
@kozlovic
Copy link
Member Author

kozlovic commented Apr 4, 2022

@derekcollison Made the changes (quite a bit more.. force pushed since almost nothing in common with first push). Also added all tests in TestJetStreamManagement.

Copy link
Member

@derekcollison derekcollison left a comment

Choose a reason for hiding this comment

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

LGTM

@kozlovic kozlovic merged commit 7623eaa into main Apr 4, 2022
@kozlovic kozlovic deleted the js_stream_and_cons_name_check branch April 4, 2022 20:24
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