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

net/mail: ParseDate Does not correctly parse RFC5322 timezone #39906

Open
NDagestad opened this issue Jun 29, 2020 · 9 comments · May be fixed by #43461
Open

net/mail: ParseDate Does not correctly parse RFC5322 timezone #39906

NDagestad opened this issue Jun 29, 2020 · 9 comments · May be fixed by #43461

Comments

@NDagestad
Copy link

@NDagestad NDagestad commented Jun 29, 2020

What version of Go are you using (go version)?

$ go version
go version go1.14.4 linux/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
...
GOARCH="amd64"
GOHOSTARCH="amd64"
GOOS="linux"
...

What did you do?

https://play.golang.org/p/BFTQrcW9JzP

package main

import (
	"fmt"
	"net/mail"
)

func main() {
	t, err := mail.ParseDate("Sun, 28 Jun 2020 21:51:24 UT")
	if err != nil {
		fmt.Printf("%v\n", err)
	} else {
		fmt.Printf("%s\n", t)
	}
}

What did you expect to see?

2020-06-28 21:51:24 +0000 UT

What did you see instead?

mail: header could not be parsed

Research

I looked at the source and found where the problem lies.
time.Parse() does not accept UT as a valide timezone even though rfc5322 allows it (section 4.3) because time.parseTimeZone() does not see a timezone with less than 3 characters as valid.

This could be fixed by adding a special case in email.ParseDate() if the timezone is "UT" and change it to something time.Parse() accepts, or adding yet another special case to time.Parse().

@ianlancetaylor ianlancetaylor changed the title mail.ParseDate Does not correctly parse RFC5322 timezone net/mail: ParseDate Does not correctly parse RFC5322 timezone Jun 29, 2020
@ianlancetaylor ianlancetaylor added this to the Backlog milestone Jun 29, 2020
@gopherbot
Copy link

@gopherbot gopherbot commented Jun 29, 2020

Change https://golang.org/cl/240357 mentions this issue: time: parse RFC5233 timezone UT correctly

Loading

@iwdgo
Copy link
Contributor

@iwdgo iwdgo commented Jul 17, 2020

Somehow duplicates #39260 as obsolete time zones are not supported.

Loading

@NDagestad
Copy link
Author

@NDagestad NDagestad commented Jul 17, 2020

Do you mean it should not be fixed ?
I get that obsolete timezones should not be generated, but there are many mail servers still sending email with them in their date fields.
Regardless, all the obsolete timezones should be treated the same way, don't you think ?

Loading

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Jul 17, 2020

I think the point is that any fix in this area may as well fix both issues at the same time.

Loading

@jimist
Copy link

@jimist jimist commented Dec 30, 2020

@ianlancetaylor I'm going for this if it's not a work in progress.
It will be my first contribution to Go Repo, any tips would be appreciated. :-D

Loading

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Dec 30, 2020

@jimist Go for it. Thanks. The place to change is probably net/mail.ParseDate.

Loading

@gopherbot
Copy link

@gopherbot gopherbot commented Jan 1, 2021

Change https://golang.org/cl/281052 mentions this issue: net/http: add UT to mail ParseDate

Loading

@jimist jimist linked a pull request that will close this issue Jan 1, 2021
@jimist
Copy link

@jimist jimist commented Jan 1, 2021

@ianlancetaylor
As a follow-up explanation, the tests of UT were used for testing if any timezone with T shouldn't work and they're now valid.
Since UT is just a standard, I replaced it with UTC to be used as the timezone.

Loading

@gopherbot
Copy link

@gopherbot gopherbot commented Jan 1, 2021

Change https://golang.org/cl/281072 mentions this issue: net/http: add UT to mail ParseDate

Loading

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

5 participants