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

Update Headers and RawHeaders using canonical MIME keys. #47

Merged
merged 1 commit into from
Aug 12, 2024
Merged

Update Headers and RawHeaders using canonical MIME keys. #47

merged 1 commit into from
Aug 12, 2024

Conversation

naspeh
Copy link
Contributor

@naspeh naspeh commented Jul 4, 2024

Hello!

Thank you for the package!

I've discovered problems using msg.Headers().Get("Message-ID") which I try to fix in this PR.


If you have a header like

Message-ID: msg@id

You can't use msg.Headers().Get("Message-ID") or msg.Headers().Get("Message-Id") , as you get an empty string. Because MIMEHeader.Get uses CanonicalMIMEHeaderKey.

I reproduced this with tests (included with the PR)

=== RUN   TestHeaders
    gmime_test.go:182: 
        	Error Trace:	gmime_test.go:182
        	Error:      	Not equal: 
        	            	expected: "<CAGPJ=uY91HEGoszHE9ELkB3wfcNJN4NGORM9q-vV8o_XJceBmg@mail.gmail.com>"
        	            	actual  : ""
        	            	
        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
        	            	@@ -1 +1 @@
        	            	-<CAGPJ=uY91HEGoszHE9ELkB3wfcNJN4NGORM9q-vV8o_XJceBmg@mail.gmail.com>
        	            	+
        	Test:       	TestHeaders
    gmime_test.go:183: 
        	Error Trace:	gmime_test.go:183
        	Error:      	Not equal: 
        	            	expected: "<CAGPJ=uY91HEGoszHE9ELkB3wfcNJN4NGORM9q-vV8o_XJceBmg@mail.gmail.com>"
        	            	actual  : ""
        	            	
        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
        	            	@@ -1 +1 @@
        	            	-<CAGPJ=uY91HEGoszHE9ELkB3wfcNJN4NGORM9q-vV8o_XJceBmg@mail.gmail.com>
        	            	+
        	Test:       	TestHeaders


...
=== RUN   TestRawHeaders
    gmime_test.go:218: 
        	Error Trace:	gmime_test.go:218
        	Error:      	Not equal: 
        	            	expected: " <CAGPJ=uY91HEGoszHE9ELkB3wfcNJN4NGORM9q-vV8o_XJceBmg@mail.gmail.com>\n"
        	            	actual  : ""
        	            	
        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
        	            	@@ -1,2 +1 @@
        	            	- <CAGPJ=uY91HEGoszHE9ELkB3wfcNJN4NGORM9q-vV8o_XJceBmg@mail.gmail.com>
        	            	 
        	Test:       	TestRawHeaders
    gmime_test.go:219: 
        	Error Trace:	gmime_test.go:219
        	Error:      	Not equal: 
        	            	expected: " <CAGPJ=uY91HEGoszHE9ELkB3wfcNJN4NGORM9q-vV8o_XJceBmg@mail.gmail.com>\n"
        	            	actual  : ""
        	            	
        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
        	            	@@ -1,2 +1 @@
        	            	- <CAGPJ=uY91HEGoszHE9ELkB3wfcNJN4NGORM9q-vV8o_XJceBmg@mail.gmail.com>

        	            	 


@naspeh
Copy link
Contributor Author

naspeh commented Jul 8, 2024

@ericychoi could you please check this PR?

@naspeh
Copy link
Contributor Author

naspeh commented Jul 23, 2024

@xdsxc or @qngn could you please check this PR?

@xdsxc
Copy link
Contributor

xdsxc commented Jul 29, 2024

@xdsxc or @qngn could you please check this PR?

Hey @naspeh, thanks for the contribution. I'll ping @ericychoi since I think he has the most context around this issue and we'll get back to you shortly

Copy link
Contributor

@ericychoi ericychoi left a comment

Choose a reason for hiding this comment

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

LGTM

@naspeh
Copy link
Contributor Author

naspeh commented Aug 3, 2024

Thank you!

I don't have write access to this repo to merge.

@ericychoi could you merge it, please?

Also, I'm interested in the release to remove my fork from a project's go.mod.

@naspeh
Copy link
Contributor Author

naspeh commented Aug 12, 2024

Hey @xdsxc, could you help to merge it, please?

@xdsxc xdsxc merged commit 3eee0b7 into sendgrid:master Aug 12, 2024
@xdsxc
Copy link
Contributor

xdsxc commented Aug 12, 2024

@naspeh Done - cut release v1.1.1.

Apologies for the delay here. Thanks for staying on top of it.

@naspeh naspeh deleted the headers-with-canonical-mime-keys branch August 12, 2024 16:21
@naspeh
Copy link
Contributor Author

naspeh commented Aug 12, 2024

@xdsxc thank you 🙇‍♂️

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.

3 participants