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.

Sign up for GitHub

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

Jump to bottom

fix: fixed string conversion to utf8 vs binary (closes #563) #564

Merged
merged 2 commits into from
Nov 30, 2023
Merged

fix: fixed string conversion to utf8 vs binary (closes #563) #564

merged 2 commits into from
Nov 30, 2023

Conversation

titanism
Copy link
Contributor

@andris9 this is a core bug that needs merged/fixed

details are in #563

calling compiled + "something" will result in the buffer being converted to utf8 instead of binary, since the default buffer.toString is utf8

@titanism
fix: fixed string conversion to uf8 vs binary (closes #563)
55611b2 
@andris9 this is a core bug that needs merged/fixed

details are in #563 

calling `compiled + "something"` will result in the buffer being converted to utf8 instead of binary, since the default buffer.toString is utf8
@titanism titanism changed the title fix: fixed string conversion to uf8 vs binary (closes #563) fix: fixed string conversion to utf8 vs binary (closes #563) Nov 27, 2023
@andris9
Copy link
Member

andris9 commented Nov 27, 2023

If the value is a binary string it gets converted to utf8 with this change which isn’t correct either, there should probably be a conditional check

@titanism
Copy link
Contributor Author

Not sure how that would look? if isBuffer then push, else push Buffer.from?

@andris9
Copy link
Member

andris9 commented Nov 27, 2023

Yes, the easiest check would be to check if the value typeof is object, then push, if string, then concat and convert from Binary, and if something else, then maybe return and not push data at all.

@titanism
Copy link
Contributor Author

@andris9 PR ready for review titanism@a8fbe83

@titanism
Copy link
Contributor Author

@andris9 just a friendly-ping - this PR is ready and fixes a core bug

@andris9 andris9 merged commit ee2708e into nodemailer:master Nov 30, 2023
3 checks passed
@github-actions github-actions bot mentioned this pull request Nov 30, 2023
Merged
@titanism titanism deleted the patch-4 branch December 4, 2023 04:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andris9 andris9 andris9 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@titanism @andris9