Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Wrong conversion between slow and fast buffer #11

efolio opened this Issue Aug 1, 2012 · 3 comments


None yet
3 participants

efolio commented Aug 1, 2012

Problem description:
Create a database with a single binary column, put an image in it. Create a query for this cell, put the resulting buffer in a file and try to open it. The file is pretty much always corrupted.

I found this is due to the fact that the data is converted to a String type before the buffer creation. Though this is the quickest way to create a Buffer from a char*, this also implies an utf-8 conversion that scrambles the content of the buffer.

I have a proposed patch for this I'll attach right away.

efolio commented Aug 1, 2012

--- a/query.cc  2011-11-17 20:14:10.000000000 +0100
+++ b/query.cc  2012-08-01 18:59:16.225018229 +0200
@@ -1319,7 +1319,12 @@
                     case node_db::Result::Column::TEXT:
                         if (this->bufferText || currentColumn->isBinary()) {
-                            value = v8::Local<v8::Value>::New(node::Buffer::New(v8::String::New(currentValue, currentLength)));
+                            node::Buffer* buf = node::Buffer::New(currentLength);
+                            memcpy(node::Buffer::Data(buf), currentValue, currentLength);
+                            v8::Local<v8::Object> globalObj = v8::Context::GetCurrent()->Global();
+                            v8::Local<v8::Function> bufferConstructor = v8::Local<v8::Function>::Cast(globalObj->Get(v8::String::New("Buffer")));
+                            v8::Handle<v8::Value> constructorArgs[3] = { buf->handle_, v8::Integer::New(currentLength), v8::Integer::New(0) };
+                            value = bufferConstructor->NewInstance(3, constructorArgs);
                         } else {
                             value = v8::String::New(currentValue, currentLength);

qraynaud commented Aug 2, 2012

Same issue here. The patch works wonders. Thanks.

roncli commented Aug 2, 2012

I had actually come up with a more involved solution, but this is much nicer. Thank you!

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