Skip to content
This repository has been archived by the owner. It is now read-only.

add test and fix for passing an invalid serialized dat info to client deserialize #153

Merged
merged 1 commit into from Feb 4, 2019

Conversation

@pes10k
Copy link
Collaborator

pes10k commented Dec 14, 2018

Passing an invalid string (something that wasn't generated from client.serialize) to client.deserialize crashes node.

This PR includes a test and fix

Copy link
Member

bbondy left a comment

Does this also fail if you just use C++ directly? Wondering if the fix should be in ad_block_client.cc instead.

Copy link
Member

bbondy left a comment

where was this failing before exactly? We don't have exceptions in the main lib

@pes10k
Copy link
Collaborator Author

pes10k commented Dec 18, 2018

its just an assert, not an exception

> const abpLib = require(".");
> (new abpLib.AdBlockClient()).deserialize("bad string")
node[21703]: ../src/node_buffer.cc:203:char *node::Buffer::Data(Local<v8::Value>): Assertion `val->IsArrayBufferView()' failed.
 1: 0x10005fbff node::Abort() [/usr/local/bin/node]
 2: 0x10005fb3c node::PrintErrorString(char const*, ...) [/usr/local/bin/node]
 3: 0x10004a179 node::Buffer::Data(v8::Local<v8::Object>) [/usr/local/bin/node]
 4: 0x103f32235 ad_block_client_wrap::AdBlockClientWrap::Deserialize(v8::FunctionCallbackInfo<v8::Value> const&) [/Users/pes/Code/brave-ad-block/build/Release/ad-block.node]
 5: 0x1001ca3f2 v8::internal::FunctionCallbackArguments::Call(v8::internal::CallHandlerInfo*) [/usr/local/bin/node]
 6: 0x1001c9a52 v8::internal::MaybeHandle<v8::internal::Object> v8::internal::(anonymous namespace)::HandleApiCallHelper<false>(v8::internal::Isolate*, v8::internal::Handle<v8::internal::HeapObject>, v8::internal::Handle<v8::internal::HeapObject>, v8::internal::Handle<v8::internal::FunctionTemplateInfo>, v8::internal::Handle<v8::internal::Object>, v8::internal::BuiltinArguments) [/usr/local/bin/node]
 7: 0x1001c9272 v8::internal::Builtin_Impl_HandleApiCall(v8::internal::BuiltinArguments, v8::internal::Isolate*) [/usr/local/bin/node]
 8: 0x6010d4cfb7d 
fish: 'node' terminated by signal SIGABRT (Abort)
@pes10k
Copy link
Collaborator Author

pes10k commented Feb 4, 2019

@bbondy just a ping on this. Does it look good?

@bbondy
bbondy approved these changes Feb 4, 2019
@bbondy bbondy merged commit e56fbf2 into brave:master Feb 4, 2019
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.