Skip to content
This repository was archived by the owner on Feb 13, 2022. It is now read-only.

Conversation

@dylancwood
Copy link

Searching the buffer string for "END" causes problems if the data inside contains the substring "END". For example, if I retrieve the string "Please do not EXTEND beyond the edge" from my memcached server, the "END" in EXTEND will signal the end of the data, and only "Please do Not E" will be returned. This is fixed by looking for crlf + "END" + crlf. On the off chance that the data contains that string, it is advisable to search for the last occurrence of it, instead of the first.

From what I can tell, there is no way to retrieve multiple values with a single get request, so looking for the last instance of crlf + "END" + crlf should be fine. However, if I am misinterpreting, and the code needs to be able to handle multiple VALUE...END pieces, let me know and I will change lastIndexOf() to indexOf()

Searching the buffer string for "END" causes problems if the data inside contains the substring "END". For example, if I retrieve the string "Please do not EXTEND beyond the edge" from my memcached server, the "END" in EXTEND will signal the end of the data, and only "Please do Not E" will be returned. This is fixed by looking for ``crlf + "END" + crlf``. On the off chance that the data contains that string, it is advisable to search for the last occurrence of it, instead of the first.
lib/memcache.js Outdated
Copy link
Author

Choose a reason for hiding this comment

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

From what I can tell, there is no way to retrieve multiple values with a single get request, so looking for the last instance of crlf + "END" + crlf should be fine. However, if I am misinterpreting, and the code needs to be able to handle multiple VALUE...END pieces, let me know and I will change lastIndexOf() to indexOf()

@dylancwood
Copy link
Author

Hi Elbart,

Thank you for making the node-memcache module! This is my first pull-request to someone else's code, so please let me know how I did, and if there is anything else I can do to make my simple change better.
Thanks again!

Based on other pull requests to node-memcache that solve the same problem
@jisqyv
Copy link

jisqyv commented Dec 4, 2018

@dylancwood See #46 for a better approach

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants