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

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

Open
10kc-svats opened this issue Oct 6, 2023 · 7 comments

Comments

@10kc-svats
Copy link

10kc-svats commented Oct 6, 2023

  • OpenPGP.js version: 5.5.0
  • Affected platform (Browser or Node.js version): > Node 20.x.x

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;

Screenshot 2023-10-05 at 3 18 43 PM

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
Copy link
Collaborator

larabr commented Oct 11, 2023

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

@10kc-svats
Copy link
Author

@larabr hi, unfortunately this does not work

@10kc-svats
Copy link
Author

@larabr if it can help you
Screenshot 2023-10-11 at 5 29 37 PM

@10kc-svats
Copy link
Author

@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
Copy link
Collaborator

larabr commented Oct 12, 2023

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

@10kc-svats
Copy link
Author

10kc-svats commented Oct 12, 2023

@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
Copy link
Collaborator

larabr commented Oct 13, 2023

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.

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

2 participants