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.

Sign up for GitHub

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

Jump to bottom

Resp protocol parser crashes when given a command ending with a newline #40

Closed
swlynch99 opened this issue Feb 2, 2023 · 1 comment · Fixed by #41
Closed

Resp protocol parser crashes when given a command ending with a newline #40

swlynch99 opened this issue Feb 2, 2023 · 1 comment · Fixed by #41

Comments

@swlynch99
Copy link
Contributor

To reproduce,

  1. Start a pelikan resp service somehow (e.g. momento proxy)
  2. Run netcat to connect to the resp port and type out 
    GET test<enter>

This will cause pelikan to panic and crash. The root cause is this line here: https://github.com/pelikan-io/pelikan/blob/main/src/protocol/resp/src/request/mod.rs#L89
@hderms
Copy link
Contributor

hderms commented Feb 2, 2023

I will fix this shortly

hderms added a commit to hderms/pelikan that referenced this issue Feb 2, 2023
@hderms
add test for pelikan-io#40
1dd076f
hderms added a commit to hderms/pelikan that referenced this issue Feb 2, 2023
@hderms
fix pelikan-io#40
4a13072
@hderms hderms mentioned this issue Feb 2, 2023
Merged
@swlynch99 swlynch99 linked a pull request Feb 2, 2023 that will close this issue
fix: issue #40 panic in RESP protocol parser #41
Merged
@swlynch99 swlynch99 changed the title Resp protocol parser when given a command ending with a newline Resp protocol parser crashes when given a command ending with a newline Feb 2, 2023
brayniac pushed a commit that referenced this issue Feb 3, 2023
@hderms @swlynch99
fix: issue #40 panic RESP protocol parser
635918e 
Fixes issue #40 by correcting the logic when a LF is received instead of a CRLF.
Adds unit tests.

---------

Co-authored-by: Sean Lynch <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: issue #40 panic in RESP protocol parser hderms/pelikan
2 participants
@hderms @swlynch99