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: Issue with escaping backslash in Quoted String (email name field) #44573

Closed
TVenuMadhav opened this issue Feb 24, 2021 · 7 comments
Closed
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.

Comments

@TVenuMadhav
Copy link

TVenuMadhav commented Feb 24, 2021

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

$ go version
go1.14.14 (go playground)

What did you do?

Run this (Reference Go playground : https://play.golang.org/p/9SmYZp-xPdc)

package main

import (
	"fmt"
	"log"
	"net/mail"
	"reflect"
)

func main() {

	tables := []struct{
		input string
		expected string
	}{
		{"\"Loops \\ \" <eren@jaeger.com>", "\"Loops \\ \" <eren@jaeger.com>"},
		{"\"Loops \\\\ \" <eren@jaeger.com>", "\"Loops \\\\ \" <eren@jaeger.com>"},
		{"\"Loops \\\" <eren@jaeger.com>", "\"Loops \\\" <eren@jaeger.com>"},
	}
	
	for i, table := range tables {
		actual, err := mail.ParseAddress(table.input)
		if err != nil {
			fmt.Printf("[ERROR][%d]: %v", i, err)
		}
		if !reflect.DeepEqual(table.expected, actual.String()) {
    			log.Printf("[NOT EQUAL][%d]: expected: %#v, got: %#v", i, table.expected, actual.String())
		} else {
			log.Printf("[EQUAL][%d]: expected %#v, got %#v", i, table.expected, actual.String())
		}
		// Misc.
		fmt.Println( actual.Name, actual.Address)
	}

}

What did you expect to see?

2009/11/10 23:00:00 [EQUAL][0]: expected: "\"Loops \\ \" <eren@jaeger.com>", got: "\"Loops \\ \" <eren@jaeger.com>"
Loops \   eren@jaeger.com
2009/11/10 23:00:00 [EQUAL][1]: expected "\"Loops \\\\ \" <eren@jaeger.com>", got "\"Loops \\\\ \" <eren@jaeger.com>"
Loops \\  eren@jaeger.com
2009/11/10 23:00:00 [EQUAL][2]: expected "\"Loops \\\" <eren@jaeger.com>", got "\"Loops \\\" <eren@jaeger.com>"
Loops \  eren@jaeger.com

What did you see instead?

2009/11/10 23:00:00 [NOT EQUAL][0]: expected: "\"Loops \\ \" <eren@jaeger.com>", got: "\"Loops  \" <eren@jaeger.com>"
Loops   eren@jaeger.com
2009/11/10 23:00:00 [EQUAL][1]: expected "\"Loops \\\\ \" <eren@jaeger.com>", got "\"Loops \\\\ \" <eren@jaeger.com>"
Loops \  eren@jaeger.com
[ERROR][2]: mail: missing word in phrase: mail: unclosed quoted-stringpanic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x10 pc=0x4c6437]

goroutine 1 [running]:
net/mail.(*Address).String(0x0, 0xc000010018, 0x4f80fc)
	/usr/local/go-faketime/src/net/mail/message.go:207 +0x37
main.main()
	/tmp/sandbox192696518/prog.go:26 +0x192

This needs to be fixed as some servers faced/ might face this error { Ours did :( }

@seankhliao seankhliao added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Feb 24, 2021
@ianlancetaylor
Copy link
Contributor

In your test program mail.ParseAddress returns an error, but the code just prints the error. Even if there is an error, the code calls actual.String(), and that is where the crash occurs. If I change your program to continue after printing ERROR, then I don't see a crash.

@ianlancetaylor ianlancetaylor added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Feb 24, 2021
@TVenuMadhav
Copy link
Author

TVenuMadhav commented Feb 25, 2021

Even if we continue, our email provider api gives error (email not sent)

Moreover, is this even expected,

shouldn't this not give error "\"Loops \\\" <eren@jaeger.com>" ?
Shouldn't the exact string be resolved to "Loops \" <eren@jaeger.com>? (in actual.String())

@ianlancetaylor
Copy link
Contributor

The error I get from your program is mail: missing word in phrase: mail: unclosed quoted-string. If we undo the backslash quoting, the actual string is, as you say "Loops \" <eren@jaeger.com>. That string does indeed appear to have an unclosed quoted-string. It seems to me that the error is correct.

@TVenuMadhav
Copy link
Author

TVenuMadhav commented Feb 25, 2021

so having a backslash as the last character in the address name field is the issue right?

If that is the case, are there any good practice suggestions for cases like these (name field is set by the enduser)

@ianlancetaylor
Copy link
Contributor

I don't know where the string came from. If the program took a string that ends in a backslash and added quotation marks around that string, then, yes, that is the problem.

One possible fix would be to take the string from the user, and then put a backslash before every special character. The list of special characters includes quotation mark " and backslash \. I don't know off hand if there are other special characters.

@TVenuMadhav
Copy link
Author

escaping the user input. Got it.

P.S :- I was wondering about handling cases like these far down the pipeline (eg. email service).

Thank you for the quick response(s)

@ianlancetaylor
Copy link
Contributor

You're welcome. Note that you will normally get faster responses from a forum: see https://golang.org/wiki/Questions.

@golang golang locked and limited conversation to collaborators Feb 25, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Projects
None yet
Development

No branches or pull requests

4 participants