net/mail: ParseDate incorrectly prunes out RFC 5322 date with a capital "T" when dealing with an obsolete timezone #39260

sywesk · 2020-05-26T20:24:13Z

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

$ go version
go version go1.14.3 linux/amd64

Does this issue reproduce with the latest release?

The version i'm using is the latest at the time of writing.

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

go env Output

$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home//.cache/go-build"
GOENV="/home//.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home//go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/c/Users//Documents/projects//backend/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build302313865=/tmp/go-build -gno-record-gcc-switches"

What did you do?

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

package main

import (
	"net/mail"
)

func main() {
	_, err := mail.ParseDate("Thu, 14 May 2020 11:44:40 GMT")
	if err != nil {
		panic(err)
	} 
	
	println("ok")
}

What did you expect to see?

I expect the time string "Thu, 14 May 2020 11:44:40 GMT" to be parsed correctly without error.

What did you see instead?

mail: header could not be parsed

Research

Before posting here I did spend some time researching the problem. Here is a plaground link with the full ParseDate code extracted and isolated to be able to investigate.

I did at first regenerate all the time layouts using buildDateLayouts(), and my specific time string did match with two of the generated layouts. So that's a good start.

But looking at the ParseDate(date string) (time.Time, error) we can see that the following code seems to rewrite the date string sequence leaving only the letter T:

        else if ind := strings.Index(p.s, "T"); ind != -1 && len(p.s) >= ind+1 {
		// The last letter T of the obsolete time zone is checked when no standard time zone is found.
		// If T is misplaced, the date to parse is garbage.
		date = p.s[:ind+1]
		p.s = p.s[ind+1:]
	}

It seems that this particular time string triggers this come into thinking that the T from Thu is about an obsolete timezone. Changing the time string to "Mon, 14 May 2020 11:44:40 GMT" for example solves the problem.

I hope my report is complete, and it may help you identify the logic bug causing this problem. If can help with anything, let me know. With some explanations about the logic the problematic code section I may even be able to make a pull request :D

Thanks for your time,

The text was updated successfully, but these errors were encountered:

odeke-em · 2020-05-26T21:03:19Z

Thank you for catching this interesting bug @sywesk, for the investigation, but also welcome to the Go project, it is great to receive your report!

This issue was introduced almost 2 years ago (September 2018 in Go1.12) in the bug fix for #22661 in CL https://go-review.googlesource.com/c/go/+/117596. As you noticed, the presence of "T" anywhere in the string will choke it and return an error, so other days of the week will work except for "Tue" and "Thur". I believe a fix for this will include:
a) Invoking strings.LastIndex instead of strings.Index
b) Ensuring that the found index+1 <= len(p.s)

which will hopefully be a trivial fix and we'll also need more tests to catch the bug and prevent regressions.

Kindly paging @iwdgo @ianlancetaylor.

gopherbot · 2020-05-26T21:20:39Z

Change https://golang.org/cl/235200 mentions this issue: net/mail: fix ParseDate obsolete timezone "T" index check

sywesk · 2020-05-27T07:49:10Z

Hello @odeke-em ! Thank you for your kind & quick message, plus the quick CL 👍 I went through your modifications and saw you did a wonderful job correcting other problematic cases ;)

geniass · 2021-01-11T17:02:31Z

@gopherbot please consider this for backport to 1.14.
This is breaking my program when parsing email header Dates. Ironically the problematic emails are coming from Google.

gopherbot · 2021-01-11T17:02:49Z

Backport issue(s) opened: #43629 (for 1.14).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://golang.org/wiki/MinorReleases.

odeke-em · 2021-01-11T18:16:41Z

Thanks for requesting the backport @geniass, however in the future just let Gobot automatically open the backports directly so that it can target the last 2 releases, as we do either all or none for backports. To ensure coherence, please also request it to backport to Go1.15, explicitly like you did for Go1.14. Thank you.

networkimprov · 2021-01-11T19:21:15Z

We can't currently file a second backport request on an issue via gopherbot (known bug, unless fixed recently?). The release team will add a second issue.

cc @dmitshur @toothrot

geniass · 2021-01-12T16:05:41Z

@odeke-em I do remember reading somewhere (can't find it now) that gopherbot only responds to the first backport request.

Do you mean something like please consider this for backport (without the version)? If so, do you think it's possible to modify the wiki to clarify this? (https://github.com/golang/go/wiki/MinorReleases)

sywesk changed the title ~~ParseDate corner case error~~ net/mail ParseDate() Corner case error May 26, 2020

odeke-em changed the title ~~net/mail ParseDate() Corner case error~~ net/mail: ParseDate incorrectly assumes that any RFC 5322 date with a capital "T" is dealing with an obsolete timezone May 26, 2020

odeke-em added this to the Go1.16 milestone May 26, 2020

odeke-em changed the title ~~net/mail: ParseDate incorrectly assumes that any RFC 5322 date with a capital "T" is dealing with an obsolete timezone~~ net/mail: ParseDate incorrectly prunes out RFC 5322 date with a capital "T" when dealing with an obsolete timezone May 26, 2020

andybons added the NeedsFix The path to resolution is known, but the work has not been done. label May 27, 2020

iwdgo mentioned this issue Jul 17, 2020

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

Closed

ianlancetaylor mentioned this issue Oct 6, 2020

Regression in implementation of mail.ParseDate(...) #41822

Closed

jameshoulahan mentioned this issue Oct 7, 2020

Message parsing issues in Import-Export app ProtonMail/proton-bridge#89

Closed

exander77 mentioned this issue Oct 8, 2020

Singificantly improve Date parsing and Address parsing ProtonMail/proton-bridge#94

Closed

gopherbot closed this as completed in 3ef8562 Nov 5, 2020

gopherbot mentioned this issue Jan 11, 2021

net/mail: ParseDate incorrectly prunes out RFC 5322 date with a capital "T" when dealing with an obsolete timezone [1.14 backport] #43629

Closed

golang locked and limited conversation to collaborators Jan 12, 2022

gopherbot added the FrozenDueToAge label Jan 12, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

net/mail: ParseDate incorrectly prunes out RFC 5322 date with a capital "T" when dealing with an obsolete timezone #39260

net/mail: ParseDate incorrectly prunes out RFC 5322 date with a capital "T" when dealing with an obsolete timezone #39260

sywesk commented May 26, 2020 •

edited

odeke-em commented May 26, 2020

gopherbot commented May 26, 2020

sywesk commented May 27, 2020

geniass commented Jan 11, 2021

gopherbot commented Jan 11, 2021

odeke-em commented Jan 11, 2021

networkimprov commented Jan 11, 2021

geniass commented Jan 12, 2021

net/mail: ParseDate incorrectly prunes out RFC 5322 date with a capital "T" when dealing with an obsolete timezone #39260

net/mail: ParseDate incorrectly prunes out RFC 5322 date with a capital "T" when dealing with an obsolete timezone #39260

Comments

sywesk commented May 26, 2020 • edited

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

Does this issue reproduce with the latest release?

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

What did you do?

What did you expect to see?

What did you see instead?

Research

odeke-em commented May 26, 2020

gopherbot commented May 26, 2020

sywesk commented May 27, 2020

geniass commented Jan 11, 2021

gopherbot commented Jan 11, 2021

odeke-em commented Jan 11, 2021

networkimprov commented Jan 11, 2021

geniass commented Jan 12, 2021

sywesk commented May 26, 2020 •

edited

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

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