Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Chunked response fix #84

Merged
merged 3 commits into from

2 participants

@renedx

This fixes an issue with chunked responses (large amounts of data). It removes the last \0 from every response before it gets parsed. If we keep the \0, it will be merged with the total response and crash the parser.

Parser will otherwise try to parse the wrong line (command), as it thinks that the command is already received.

The error was:

Parse:
   VALUE invoices::#17748 2 196
Error:
   error - 21/01 - 22:17 Caught exception: SyntaxError: Unexpected token V
   info  - 21/01 - 22:17 SyntaxError: Unexpected token V
    at Object.parse (native)
    at Socket.value (/node/node_modules/memcached/lib/memcached.js:432:17)
    at EventEmitter.rawDataReceived (/node/node_modules/memcached/lib/memcached.js:624:51)
    at EventEmitter.BufferBuffer (/node/node_modules/memcached/lib/memcached.js:584:12)
    at Socket.bowlofcurry (/node/node_modules/memcached/lib/utils.js:109:15)
    at Socket.EventEmitter.emit (events.js:96:17)
    at TCP.onread (net.js:392:31)

I've got it running for a while now, without any issues loading up millions of items in seconds for testing.

@3rd-Eden
Owner

Thanks a lot for the pull request. Could you add a small test against it? I don't want to introduce any regressions when I release my 1.0 refactor.

@renedx

You are right, gonna check if I can make something that doesn't insert millions of data to test it. Because it needs network delay to create the case, locally it doesn't provide us with chunked responses. Thats the main reason I removed my initial test-case. Any suggestions on reproducing network delays?

@3rd-Eden
Owner

For the new memcached parser I have developed a memcached server response fuzzer to handle edge cases like these where the parse is the root cause of the issue.

Maybe you could setup a node based memcached server for your test and add some random setTimeout's in there. Other then that, I don't really know.

@3rd-Eden 3rd-Eden merged commit 70a31eb into 3rd-Eden:master

1 check passed

Details default The Travis build passed
@3rd-Eden
Owner

@renedx just merged this, just hope that this will also be fixed in my other parser.

@renedx

Thanks! I tried to reproduce it with tests. If I find something that keeps working (as test should) I will add it anyway so you can test your new parser on it too :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Feb 21, 2013
  1. @renedx

    Fixed chunking issues

    renedx authored
Commits on Feb 27, 2013
  1. @renedx
  2. @renedx
This page is out of date. Refresh to see the latest.
Showing with 5 additions and 0 deletions.
  1. +5 −0 lib/memcached.js
View
5 lib/memcached.js
@@ -594,6 +594,11 @@ Client.config = {
});
}
+ // Fix zero-line endings in the middle
+ var chunkLength = (chunks.length-1);
+ if( chunks[chunkLength].length == 0 )
+ chunks.splice(chunkLength, 1)
+
S.responseBuffer = ""; // clear!
this.rawDataReceived(S, S.bufferArray = S.bufferArray.concat(chunks));
}
Something went wrong with that request. Please try again.