Error: MemBuffer overrun when notes contain binary data [files, images etc]

[1110n]

I am working on evernote javascript SDK [[email protected]], things worked fine till I tried to make the following call ::

noteStore.getNoteWithResultSpec(guid, {includeContent:true, includeResourcesData: true})

The call to this function getNoteWithResultSpecfails when my notes on evernote contain binary data e.g. [img / pdf] etc. However if my notes only contain text data [ no img / pdf ] then this function works fine.

The console shows the following ::

` evernote/node_modules/evernote/lib/thrift/transport/memBuffer.js:32 if (this.offset + len > this.buffer.length) throw Error('MemBuffer overrun'); ^

Error: MemBuffer overrun at Error (native) at MemBuffer.read (/evernote/node_modules/evernote/lib/thrift/transport/memBuffer.js:32:55) at BinaryProtocol.readBinary (/evernote/node_modules/evernote/lib/thrift/protocol/binaryProto col.js:327:29) at BinaryProtocol.readType (/evernote/node_modules/evernote/lib/thrift/protocol/binaryProtoco l.js:355:25) at Object.Thrift.Struct.readFields (/evernote/node_modules/evernote/lib/thrift/thrift.js:505: 49) at Thrift.Struct.read (/evernote/node_modules/evernote/lib/thrift/thrift.js:485:19) `

Please suggest / help to resolve this.

[akhaku]

Thanks for your email - let's keep correspondence on this issue. For posterity, here's an excerpt from your email:

Saw that the origin of error is :: memBuffer.js if (this.offset + len > this.buffer.length) throw Error('MemBuffer overrun'); // this is the line

Now it happens because the len in some cases comes out to be way more than the size of the buffer itself [ I logged both of them ]

But I've not been able to figure out where the length of the buffer is set etc.

It looks like the relevant code is in binaryHttpTransport.js, which is loaded in stores.js. BinaryHttpTransport creates a MemBuffer. Looking at the flush method of BinaryHttpTransport, there are a few interesting things happening in that method...

[mariecl]

Is there any plan to fix this one?

[mariecl]

@akhaku Since MemBuffer.prototype.read (from memBuffer.js) can potentially throw MemBuffer overrun, couldn't we at least make sure to catch that error, please? Otherwise, it becomes an uncaughtException which causes crashes.

Locally, if I replace

var header = response.readMessageBegin();

(btw, header was already declared a few lines above this one ;) ) with:

try {
   header = response.readMessageBegin();
}
catch (err) {
  response.readMessageEnd();
  callback(err);
  return
}

in Thrift.Method.prototype.processResponse #L165, the error seems to be caught properly.

I am not creating a PR because I am unsure how you want the SDK to handle this memBuffer exception, but could you please look into it?

[gjermundgaraba]

I am experimenting with the express sample code and kept running into this error every time I logged in. Replacing the code @mariecl mentioned seems to solve the issue for me at least. Thanks! :)

[julen]

I tried with the simplest snippet I could come up with using noteStore.getNoteWithResultSpec():

var Evernote = require('evernote');

var TOKEN = '<put-your-dev-token-here>';
var noteGUID = '<put-your-note-guid-here>';

var client = new Evernote.Client({
  token: TOKEN,
  sandbox: true,
});
var ns = client.getNoteStore();

ns.getNoteWithResultSpec(noteGUID, { includeContent: true, includeResourcesData: true })
  .then(function(note) {
    console.log(note);
  }).catch(function (err) {
    console.log(err);
  });

and I'm unable to reproduce this issue on node v8.9.1. I tested with multiple notes which include binary data.

Can someone affected by this please provide more details?

[mariecl]

This error has re-appeared for me after upgrading to node 9.11.1.

[mariecl]

I just created a PR (#88) leveraging the code I posted above since it looks like it works for a few people other than me. I hope this can get fixed in the near future 🤞.

[zaverden]

I have faced same issue, and can provide you some details

1. This issue is not persistent. Meaning: if it happened for some request it does not mean it'll happen if you retry some time later with the same parameters
2. Whether error will be thrown depends on server response. Meaning: I have my app deployed into 3 environments (prod, qa, integration), 2 instances per environment. All instances begun fail in same time frame. last time it was 2018-07-09 from 09:20 to 10:00 UTC. It looks like Evernote API was sending invalid responses which could not be parsed, and it become a cause of the error
3. The main issue with this error is not that it happens, but with the way how it being thrown. In depth of async function we got a plain throw Error, it does not go to callback, so it cannot be handled.

How you can reproduce the issue:

1. You can find code example here: https://gist.github.com/zaverden/4c0b4a7d5d4f68152a449b6b32a57822. Don't forget to run npm i [email protected], and then run node evernote-sdk-membuf-error.js
2. After first run you'll probably get success message. As I said before, issue is not persistent, and doe not appears very often
3. So, lets make it persistent! Open file node_modules/evernote/lib/thrift/transport/memBuffer.js and remove condition at line 29 - MemBuffer.prototype.read will always throw the error
4. Run the script again - you may expect to see handled async error message, but instead it fall to uncaughtException handler.

Meaning: MemBuffer overrun cannot be handle in current implementation, it always crashes process.

If you'll apply fix from PR #88 you'll get expected handled async error message. I understand that it does not explain why MemBuffer overrun appears, but at least, this fix allows us to handle it safely.

@julen with this information, when do you think we can expect new version with this fix applied?

[zaverden]

Monkey patch to make MemBuffer overrun handlable:

const Thrift = require('evernote/lib/thrift/thrift');
Thrift.Method.prototype.__processResponse = Thrift.Method.prototype.processResponse;
Thrift.Method.prototype.processResponse = function (response, callback) {
  try {
    this.__processResponse(response, callback);
  } catch (err) {
    callback(err);
  }
};

[julen]

Hey @zaverden, thanks for your diagnosis.

I believe the intermittent errors you were having can be attributed to some EN service availability issues that became more frequent at the beginning of July.

I'm not in charge of the SDK, but I heard the Thrift-generated code might get an update soon, which is quite old at the moment, and I reckon that is likely to change the code causing this.

[kevinburkenotion]

We just got a MemBuffer overrun crash in production.

[aguynamedben]

Same thing, MemBuffer overrun crash our app in production.

[nickerlan]

TIP should also remove JSON.stringify(error) to make @zaverden solution work, otherwise getting "Converting circular structure to JSON" error