Handle special characters in metric name

[posener]

I noticed that having a metric with name that contains colons result in headers that are not valid (the parser confuses them to be the headers colons, I guess. I guess that there are more special characters.

One option to solve it is to replace them with other character? Another is to fix the parser?

[mitchellh]

So according to the HTTP spec (https://tools.ietf.org/html/rfc7230), the set of delimiters are simply not valid. I propose we accumulate an error but build-up as much of the header as we can.

[mitchellh]

Actually, I may have misunderstood your issue initially. If the quoted name value isn't parsing colons properly then that would be a separate bug. I thought I tested that but that may have to be reported upstream since the value parsing I do is from the godoc libraries.

[posener]

What I mean is, that if i'll add a test case in header_test.go:

	{
		[]*Metric{
			{
				Name:     "sql:1",
				Duration: 100 * time.Millisecond,
				Desc:     "MySQL lookup Server",
				Extra:    map[string]string{},
			},
		},
		`sql:1;desc="MySQL lookup Server";dur=100`,
	},

This case will fail, because of the colons in Name:

$ go test ./...
--- FAIL: TestParseHeader (0.00s)
    --- FAIL: TestParseHeader/sql:1;desc="MySQL_lookup_Server";dur=100 (0.00s)
    	header_test.go:90: received, expected:
    		
    		[]*servertiming.Metric{*servertiming.Metric{Name:"sql", Duration:0, Desc:"", Extra:map[string]string{}, startTime:time.Time{wall:0x0, ext:0, loc:(*time.Location)(nil)}}}
    		
    		[]*servertiming.Metric{*servertiming.Metric{Name:"sql:1", Duration:100000000, Desc:"MySQL lookup Server", Extra:map[string]string{}, startTime:time.Time{wall:0x0, ext:0, loc:(*time.Location)(nil)}}}
FAIL
FAIL	github.com/mitchellh/go-server-timing	0.055s
?   	github.com/mitchellh/go-server-timing/example	[no test files]

So the gddo header package is the one to blame, it will stop reading the "name" on any of: \t\"(),:;<=>?@[]\\{}, characters that are lower than or equal to 31, and the 127th character. then, since the current character is not ; it does not continue to read key-value pairs.

I could not understand if this is the right behavior from the rfc, section 3.2.6. Field Value Components looks relevant, but no specific rules. Might it be that the gddo had this rule just because it suited the use case of content-type headers?

Anyway, currently, if the name contains special character, even a space, it breaks the parsing of the metric.

Do you suggest to open an issue in gddo and see what I get there?

[posener]

Opened an issue in gddo: https://github.com/golang/gddo/issues/539