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

feat(modify): new modifier add_header #771

Open
wants to merge 3 commits into
base: dev
Choose a base branch
from

Conversation

msessa
Copy link

@msessa msessa commented Mar 2, 2025

Partially addresses #158

Description

This PR adds a new modifier add_header which allows arbitrary headers to be added to a message in the pipeline.

Use Case

A practical use case is to inject the X-SES-FROM-ARN header before forwarding the message to AWS SES to control which identity will be used to verify the sender domain.

See reference here

@msessa msessa changed the title feat(modify): new modifier add_header feat(modify): new modifier add_header Mar 2, 2025
Copy link
Owner

@foxcpp foxcpp left a comment

Choose a reason for hiding this comment

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

Overall LGTM. Can you add a configuration option to not return an error on duplicate header? It is correct to have multiple values for some header fields and such configuration option may be useful for some cases. I would probably make it default even as "least surprise" principle.

@msessa
Copy link
Author

msessa commented Mar 3, 2025

@foxcpp thanks for you review (and for creating maddy!)

I didn't realize the spec permitted fields appearing multiple times.
I've adapted the code and tests for that

@msessa
Copy link
Author

msessa commented Mar 3, 2025

btw I'm happy to also implement other variations of the modifier, eg: upsert_header remove_header if you think they could be useful

@msessa msessa requested a review from foxcpp March 5, 2025 01:15
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.

2 participants