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

filterByTag check never throws due to List() truthyness #907

Closed
adhooaa opened this issue May 28, 2019 · 1 comment · Fixed by #1289
Closed

filterByTag check never throws due to List() truthyness #907

adhooaa opened this issue May 28, 2019 · 1 comment · Fixed by #1289

Comments

@adhooaa
Copy link

adhooaa commented May 28, 2019

At message.js:158 there is:
const symESKeyPacketlist = this.packets.filterByTag(enums.packet.symEncryptedSessionKey); if (!symESKeyPacketlist) { throw new Error('No symmetrically encrypted session key packet found.'); }
However the implementation filterByTag always returns a new List() (which may be empty).

Even when empty, if (!symESKeyPacketlist) { always evaluates false because empty lists are still truthy

There may be other places in the code that follows this pattern...

@twiss
Copy link
Member

twiss commented Jun 4, 2019

Thanks, good catch! In this case, it shouldn't matter too much if that list is empty as we'd still throw a bit further down:

throw exception || new Error('Session key decryption failed.');

However, feel free to make a PR anyway, in order to improve the error message, and to fix any other cases if you find them.

@chesnokovilya chesnokovilya self-assigned this Apr 28, 2020
twiss pushed a commit that referenced this issue Apr 29, 2021
…types (#1289)

Changes:
- Implementation:
  - Remove `PacketList.prototype.concat` and `push`
    (we solely rely on `Array.push` instead)
  - Fix #907 by
    correctly handling result of `filterByTag`
  - Implement `write()` method for `Trust` and `Marker` packets,
    to make them compatible with the `BasePacket` interface
- Types:
  - Simplify and updated `PacketList` type definitions
  - Fix types for `Packet.tag`, which is `static` since
    #1268
  - Prevent passing SubkeyPackets where KeyPackets are expected,
    and vice versa
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

Successfully merging a pull request may close this issue.

3 participants