openpgpjs / openpgpjs

OpenPGP implementation for JavaScript
https://openpgpjs.org
GNU Lesser General Public License v3.0
5.68k stars 797 forks source link

Handle closing the file descriptors in Node 20.x.x when work with streams #1690

Closed 10kc-svats closed 4 days ago

10kc-svats commented 1 year ago

When we switched from node 18 to node 20 we've encountered this problem, where decrypt method resolves the promise, but the data does not contain a decrypted stream.

The code reads a file using a stream with the fs.createReadStream method. If the stream isn't properly closed (either because of an error, or because the stream wasn't fully read), the file remains open. In the original code, we were passing the stream directly to openpgp.readMessage, and if openpgp.readMessage didn't read the stream fully or there was an error in between, the stream was left open.

The piece of code:

const stream = fs.createReadStream(filePath, 'utf-8');
const privateKey = await openpgp.decryptKey({
  privateKey: await openpgp.readPrivateKey({
    armoredKey: this.privateKeyASCII,
  }),
  passphrase: this.passPhrase,
});
const options: openpgp.DecryptOptions = {
  decryptionKeys: privateKey,
  message: await openpgp.readMessage({
    armoredMessage: stream,
  }),
};
const { data } = await openpgp.decrypt(options);
return data;

<img width="1271" alt="Screenshot 2023-10-05 at 3 18 43 PM" src="https://github.com/openpgpjs/openpgpjs/assets/112363847/f70453d8-0527-426a-ad0d-0e58cf14e19d">

Solution: To resolve the issue, we explicitly close the stream using the stream.destroy() method after we're done processing it. This ensures that the file descriptor is closed and released appropriately, and it doesn't have to wait for the garbage collector to do it.

larabr commented 1 year ago

Hi @10kc-svats , could you try and see if 96d6e76c05df1dc6822bd437cd85f2e3d6a9f4e4 (#1691) fixes the issue ?

10kc-svats commented 1 year ago

@larabr hi, unfortunately this does not work

10kc-svats commented 1 year ago

@larabr if it can help you

Screenshot 2023-10-11 at 5 29 37 PM
10kc-svats commented 1 year ago

@larabr actually, stream.destroy() does not solve the problem, I accidentally switched to the prev node version and it works well (I thought I'm using node 20.x.x).

this solution works for me

larabr commented 1 year ago

How are you testing for the issue? Passing a stream with data that triggers an OpenPGP.js error?

10kc-svats commented 1 year ago

@larabr

  1. saving an encrypted file to the local file
  2. creating a read stream from that file
  3. Passing the stream to the decrypt method
const options: openpgp.DecryptOptions = {
  config: {
    allowInsecureDecryptionWithSigningKeys: true,
    allowUnauthenticatedMessages: true,
  },
  decryptionKeys: privateKey,
  message: await openpgp.readMessage({
    armoredMessage: readStream,
  }),
};
const { data } = await openpgp.decrypt(options);
larabr commented 1 year ago

I am not clear on the issue you are observing.

Are you reading out the data stream (i.e. pulling chunks)? If not, the decryption process doesn't even start (and neither does readMessage consume the input stream), and it's expected that the file descriptor remains open.