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

Nodedb should handle undefined and Buffer as input #12

Open
qraynaud opened this issue Aug 2, 2012 · 7 comments
Open

Nodedb should handle undefined and Buffer as input #12

qraynaud opened this issue Aug 2, 2012 · 7 comments

Comments

@qraynaud
Copy link

qraynaud commented Aug 2, 2012

I believe nodedb should support Buffer as a standard input type because they are the (only ?) "correct" way to handle binary in nodejs. I also believe undefined is quite like NULL in a database and added support for this too because this looks sensitive.

I wrote the following patch to do just that (sorry I'm not a git user and thus won't make a pull request directly on github) :

--- a/query.cc    2012-08-02 18:05:56.920615897 +0200
+++ b/query.cc  2012-08-02 18:17:57.772591855 +0200
@@ -1392,7 +1392,7 @@
 std::string node_db::Query::value(v8::Local<v8::Value> value, bool inArray, bool escape, int precision) const throw(node_db::Exception&) {
     std::ostringstream currentStream;

-    if (value->IsNull()) {
+    if (value->IsNull() || value->IsUndefined()) {
         currentStream << "NULL";
     } else if (value->IsArray()) {
         v8::Local<v8::Array> array = v8::Array::Cast(*value);
@@ -1419,8 +1419,17 @@
         v8::Handle<v8::String> valueKey = v8::String::New("value");
         v8::Handle<v8::String> escapeKey = v8::String::New("escape");

-        if (object->Has(valueKey)) {
+        if (node::Buffer::HasInstance(object)) {
+            size_t length = node::Buffer::Length(object);
+            std::string string(node::Buffer::Data(object), length);
+
+            try {
+              currentStream << this->connection->quoteString << this->connection->escape(string) << this->connection->quoteString;
+            } catch(node_db::Exception& exception) {
+              throw new node_db::Exception("Escaping of binary string failed: cannot continue");
+            }
+        } else if (object->Has(valueKey)) {
             v8::Handle<v8::String> precisionKey = v8::String::New("precision");
             int precision = -1;

This patch was written on the released file but it should work on the newer one without much adaptation.

Thanks a lot for considering this little addition to your base code.

@efolio
Copy link

efolio commented Aug 2, 2012

This completes the previous issue, as I proposed a debug for fetching binary data (ie. buffers) and this proposes a feature to store it easily.
Thanks a lot!

@qraynaud
Copy link
Author

qraynaud commented Aug 2, 2012

Yes, I'm already using your patch as stated in a comment. Happy this is of help to you ;-).

@qraynaud
Copy link
Author

I'm a little sad to see users taking time to come up with issues and their related patches are not welcome enough here to get even an answer.

Is this project stil maintained or is it abandonned?

@mariano
Copy link
Owner

mariano commented Aug 18, 2012

@even First of all thanks for the patch. Could you fork the project and make a pull request so credit is given where its due?

@qraynaud
Copy link
Author

I would do so if I have to but I'll need to install git and (re)figure out how it works because I haven't used git in decades... I'm a mercurial guy. Yeah I know, I've fallen to the dark side.

I'm not really looking for popularity, I'd rather let you push the patch with your account and be done with it. If it's fine with you.

@mariano
Copy link
Owner

mariano commented Aug 21, 2012

I wouldn't call mercurial the dark side, I like it a lot :) But still GIT is my all-time-fav ;)

Ok I'll work on patching this

@qraynaud
Copy link
Author

Great !

And thanks for the quick answer. It would be awesome if you could do something about bug #11 too because it's related. One is needed to insert buffers and the other to retrieve them without corruption...

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

No branches or pull requests

3 participants