v1: fix command always LOCAL

[bohanyang]

The changes in #61 overrides all header.Command to LOCAL (which was set to PROXY by initVersion1) unconditionally.

go-proxyproto/v1.go

Lines 55 to 60 in b6f440c

	// Allocation only happens when a signature is found.
	header := initVersion1()
	// If UNKNOWN is present, set Command to LOCAL.
	// Command is not present in v1 but set it for other parts of
	// this library to rely on it for determining connection details.
	header.Command = LOCAL

This makes these functions returning the original addresses rather than the addresses contained in the proxy header.

go-proxyproto/protocol.go

Lines 127 to 155 in b6f440c

	// LocalAddr returns the address of the server if the proxy
	// protocol is being used, otherwise just returns the address of
	// the socket server. In case an error happens on reading the
	// proxy header the original LocalAddr is returned, not the one
	// from the proxy header even if the proxy header itself is
	// syntactically correct.
	func (p *Conn) LocalAddr() net.Addr {
	p.once.Do(func() { p.readErr = p.readHeader() })
	if p.header == nil || p.header.Command.IsLocal() || p.readErr != nil {
	return p.conn.LocalAddr()
	}
	return p.header.DestinationAddr
	}
	// RemoteAddr returns the address of the client if the proxy
	// protocol is being used, otherwise just returns the address of
	// the socket peer. In case an error happens on reading the
	// proxy header the original RemoteAddr is returned, not the one
	// from the proxy header even if the proxy header itself is
	// syntactically correct.
	func (p *Conn) RemoteAddr() net.Addr {
	p.once.Do(func() { p.readErr = p.readHeader() })
	if p.header == nil || p.header.Command.IsLocal() || p.readErr != nil {
	return p.conn.RemoteAddr()
	}
	return p.header.SourceAddr
	}

Unfortunately, the problem was not revealed, since the EqualsTo function, which is used by the tests to compare actual and expected headers, straightly returns true when the command is LOCAL, and the command is not being compared.

go-proxyproto/header.go

Lines 150 to 170 in b6f440c

	// EqualsTo returns true if headers are equivalent, false otherwise.
	func (header *Header) EqualsTo(otherHeader *Header) bool {
	if otherHeader == nil {
	return false
	}
	if header.Command.IsLocal() {
	return true
	}
	// TLVs only exist for version 2
	if header.Version == 0x02 && !bytes.Equal(header.rawTLVs, otherHeader.rawTLVs) {
	return false
	}
	if header.Version != otherHeader.Version || header.TransportProtocol != otherHeader.TransportProtocol {
	return false
	}
	if header.TransportProtocol == UNSPEC {
	return true
	}
	return header.SourceAddr.String() == otherHeader.SourceAddr.String() &&
	header.DestinationAddr.String() == otherHeader.DestinationAddr.String()
	}

This PR also updated the condition that checks the token length, made it to make sense.

The comments for ProtocolVersionAndCommand are also updated, made it less confusing.

[coveralls]

Coverage increased (+0.007%) to 94.223% when pulling 43ce4ef on bohanyang:fix-v1-command into b6f440c on pires:main.

[pires]

Overall LGTM, but left a couple nits.

[bohanyang]

Done fixing the comments!

[pires]

Thanks a lot for this contribution.

[pires]

https://github.com/pires/go-proxyproto/releases/tag/v0.4.2