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

add newline to A: pseudo header when adding attachment #50

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

hbog
Copy link
Contributor

@hbog hbog commented May 16, 2024

The eutil.add_header_line function, used to add the pseudo header, expects a newline terminated string. Otherwise it will remove the RFC defined newline between header and body in the message being composed.

fixes #49

The eutil.add_header_line function, used to add the pseudo header,
expects a newline terminated string. Otherwise it will remove the RFC
defined newline between header and body in the message being composed.
@The-Compiler
Copy link
Contributor

The-Compiler commented May 16, 2024

Wouldn't it make more sense to fix the surprising behavior of add_header_line instead of adding the newlines for every caller (and then probably reintroducing the bug when using add_header_line again later)?

I have a fix for this on my own branch doing exactly that, but haven't gotten around to upstreaming all those: The-Compiler@9390aef

@hbog
Copy link
Contributor Author

hbog commented May 16, 2024

@The-Compiler that also crossed my mind, but I would add the the newline only when it is not already there, because in that case the function would add an extra newline to the body. I made the assumption that, in this code, a header is a newline terminated string, but I agree that your approach absolutely makes sense.

@The-Compiler
Copy link
Contributor

I suppose that could be avoided with a h.strip() in the function. But I'd argue it's the callers fault if the caller adds a newline to the header value.

@akissinger
Copy link
Owner

What's the status of this? It looks like the discussion is inconclusive on whether this should be merged.

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 this pull request may close these issues.

Adding an attachment removes the line feed between headers and body.
3 participants