Skip to content

#6276 Improve message parsing for signed message - #6277

Draft
martgil wants to merge 2 commits into
masterfrom
issue-6276-fix-recursive-parsing-issue
Draft

#6276 Improve message parsing for signed message#6277
martgil wants to merge 2 commits into
masterfrom
issue-6276-fix-recursive-parsing-issue

Conversation

@martgil

Copy link
Copy Markdown
Collaborator

This PR improve message parsing for signed message and prevents recursive parsing.

close#6276


Tests(delete all except exactly one):

  • Tests added or updated

To be filled by reviewers

I have reviewed that this PR... (tick whichever items you personally focused on during this review):

  • addresses the issue it closes (if any)
  • code is readable and understandable
  • is accompanied with tests, or tests are not needed
  • is free of vulnerabilities
  • is documented clearly and usefully, or doesn't need documentation

@martgil

Copy link
Copy Markdown
CollaboratorAuthor

Hi @sosnovsky - This one is ready for a review. Thanks!

@sosnovskysosnovsky left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

works well, just a couple minor improvements 👍

Comment threadextension/js/common/core/mime.ts Outdated
if (!Mime.isSignedContentNode(current)) {
return currentSubject || fallbackSubject;
}
fallbackSubject = currentSubject;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it'll be better to use fallbackSubject = currentSubject || fallbackSubject for cases when currentSubject is undefined, so we can use previous non-null value.

Comment threadextension/js/common/core/mime.ts Outdated
/* eslint-enable no-underscore-dangle */
}
/* eslint-enable no-underscore-dangle */
return undefined;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also here we should return fallbackSubject instead of undefined, for cases when in nested nodes some deep node is undefined, but there was some non-null node in previous nodes

type SendingType = 'to' | 'cc' | 'bcc';

export class Mime {
private static readonly MAX_SIGNED_CONTENT_DEPTH = 50;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there is similar recursive call in

if(!sigResult?.match&&signedContentInDecryptedData){

which can cause extension lag for nested signed content. we'll need to implement similar fix there, but let's leave it for separate PR

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it - Will remember this one and fix it on separate PR after this one.

@martgil
martgil marked this pull request as draft August 21, 2026 12:10
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Client slowdown when parsing recursive multipart/signed messages

2 participants

@martgil@sosnovsky