Skip to content

Commit

Permalink
Fix IPv6 handling errors in semconv.NetAttributesFromHTTPRequest
Browse files Browse the repository at this point in the history 
fixes open-telemetry#2283
  • Loading branch information
@evantorrie
evantorrie committed Oct 13, 2021
1 parent 8ba6da8 commit 996504b
Show file tree
Hide file tree
Showing 3 changed files with 209 additions and 54 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
View file
Expand Up @@ -16,6 +16,10 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm

- Adds `otlptracegrpc.WithGRPCConn` and `otlpmetricgrpc.WithGRPCConn` for reusing existing gRPC connection. (#2002)

### Fixed

- `semconv.NetAttributesFromHTTPRequest()` correctly handles IPv6 addresses. (#)

## [1.0.1] - 2021-10-01

### Fixed
Expand Down
80 changes: 36 additions & 44 deletions semconv/v1.4.0/http.go
View file
Expand Up @@ -52,30 +52,7 @@ func NetAttributesFromHTTPRequest(network string, request *http.Request) []attri
}

peerName, peerIP, peerPort := "", "", 0
{
hostPart := request.RemoteAddr
portPart := ""
if idx := strings.LastIndex(hostPart, ":"); idx >= 0 {
hostPart = request.RemoteAddr[:idx]
portPart = request.RemoteAddr[idx+1:]
}
if hostPart != "" {
if ip := net.ParseIP(hostPart); ip != nil {
peerIP = ip.String()
} else {
peerName = hostPart
}

if portPart != "" {
numPort, err := strconv.ParseUint(portPart, 10, 16)
if err == nil {
peerPort = (int)(numPort)
} else {
peerName, peerIP = "", ""
}
}
}
}
peerIP, peerName, peerPort = hostIPNamePort(request.RemoteAddr)
if peerName != "" {
attrs = append(attrs, NetPeerNameKey.String(peerName))
}
Expand All @@ -88,27 +65,9 @@ func NetAttributesFromHTTPRequest(network string, request *http.Request) []attri

hostIP, hostName, hostPort := "", "", 0
for _, someHost := range []string{request.Host, request.Header.Get("Host"), request.URL.Host} {
hostPart := ""
if idx := strings.LastIndex(someHost, ":"); idx >= 0 {
strPort := someHost[idx+1:]
numPort, err := strconv.ParseUint(strPort, 10, 16)
if err == nil {
hostPort = (int)(numPort)
}
hostPart = someHost[:idx]
} else {
hostPart = someHost
}
if hostPart != "" {
ip := net.ParseIP(hostPart)
if ip != nil {
hostIP = ip.String()
} else {
hostName = hostPart
}
hostIP, hostName, hostPort = hostIPNamePort(someHost)
if hostIP != "" || hostName != "" || hostPort != 0 {
break
} else {
hostPort = 0
}
}
if hostIP != "" {
Expand All @@ -124,6 +83,39 @@ func NetAttributesFromHTTPRequest(network string, request *http.Request) []attri
return attrs
}

// hostIPNamePort extracts the IP address, name and (optional) port from hostWithPort.
// It handles both IPv4 and IPv6 addresses. If the host portion is not recognized
// as a valid IPv4 or IPv6 address, the `ip` result will be empty and the
// host portion will instead be returned in `name`.
func hostIPNamePort(hostWithPort string) (ip string, name string, port int) {
hostPart, portPart := hostWithPort, ""
if idx := strings.LastIndexByte(hostWithPort, ':'); idx >= 0 {
ipv6WithPort := idx > 0 && hostWithPort[0] == '[' && hostWithPort[idx-1] == ']'
ipv4WithPort := idx > 0 && strings.IndexByte(hostWithPort[0:idx-1], ':') < 0
if ipv6WithPort || ipv4WithPort {
hostPart, portPart, _ = net.SplitHostPort(hostWithPort)
} else if idx == 0 {
hostPart, portPart = "", hostWithPort[idx+1:]
}
}
if hostPart != "" {
if parsedIP := net.ParseIP(hostPart); parsedIP != nil {
ip = parsedIP.String()
} else {
name = hostPart
}
}
if portPart != "" {
numPort, err := strconv.ParseUint(portPart, 10, 16)
if err == nil {
port = (int)(numPort)
} else {
port = 0
}
}
return
}

// EndUserAttributesFromHTTPRequest generates attributes of the
// enduser namespace as specified by the OpenTelemetry specification
// for a span.
Expand Down
179 changes: 169 additions & 10 deletions semconv/v1.4.0/http_test.go
View file
Expand Up @@ -20,6 +20,7 @@ import (
"strings"
"testing"

"github.com/google/go-cmp/cmp"
"github.com/stretchr/testify/assert"

"go.opentelemetry.io/otel/attribute"
Expand Down Expand Up @@ -131,7 +132,7 @@ func TestNetAttributesFromHTTPRequest(t *testing.T) {
},
},
{
name: "with remote ip and port",
name: "with remote ipv4 and port",
network: "tcp",
method: "GET",
requestURI: "/user/123",
Expand All @@ -148,6 +149,42 @@ func TestNetAttributesFromHTTPRequest(t *testing.T) {
attribute.Int("net.peer.port", 56),
},
},
{
name: "with remote ipv6 and port",
network: "tcp",
method: "GET",
requestURI: "/user/123",
proto: "HTTP/1.0",
remoteAddr: "[fe80::0202:b3ff:fe1e:8329]:56",
host: "",
url: &url.URL{
Path: "/user/123",
},
header: nil,
expected: []attribute.KeyValue{
attribute.String("net.transport", "ip_tcp"),
attribute.String("net.peer.ip", "fe80::202:b3ff:fe1e:8329"),
attribute.Int("net.peer.port", 56),
},
},
{
name: "with remote ipv4-in-v6 and port",
network: "tcp",
method: "GET",
requestURI: "/user/123",
proto: "HTTP/1.0",
remoteAddr: "[::ffff:192.168.0.1]:56",
host: "",
url: &url.URL{
Path: "/user/123",
},
header: nil,
expected: []attribute.KeyValue{
attribute.String("net.transport", "ip_tcp"),
attribute.String("net.peer.ip", "192.168.0.1"),
attribute.Int("net.peer.port", 56),
},
},
{
name: "with remote name and port",
network: "tcp",
Expand All @@ -167,7 +204,7 @@ func TestNetAttributesFromHTTPRequest(t *testing.T) {
},
},
{
name: "with remote ip only",
name: "with remote ipv4 only",
network: "tcp",
method: "GET",
requestURI: "/user/123",
Expand All @@ -183,6 +220,40 @@ func TestNetAttributesFromHTTPRequest(t *testing.T) {
attribute.String("net.peer.ip", "1.2.3.4"),
},
},
{
name: "with remote ipv6 only",
network: "tcp",
method: "GET",
requestURI: "/user/123",
proto: "HTTP/1.0",
remoteAddr: "fe80::0202:b3ff:fe1e:8329",
host: "",
url: &url.URL{
Path: "/user/123",
},
header: nil,
expected: []attribute.KeyValue{
attribute.String("net.transport", "ip_tcp"),
attribute.String("net.peer.ip", "fe80::202:b3ff:fe1e:8329"),
},
},
{
name: "with remote ipv4_in_v6 only",
network: "tcp",
method: "GET",
requestURI: "/user/123",
proto: "HTTP/1.0",
remoteAddr: "::ffff:192.168.0.1", // section 2.5.5.2 of RFC4291
host: "",
url: &url.URL{
Path: "/user/123",
},
header: nil,
expected: []attribute.KeyValue{
attribute.String("net.transport", "ip_tcp"),
attribute.String("net.peer.ip", "192.168.0.1"),
},
},
{
name: "with remote name only",
network: "tcp",
Expand Down Expand Up @@ -214,6 +285,7 @@ func TestNetAttributesFromHTTPRequest(t *testing.T) {
header: nil,
expected: []attribute.KeyValue{
attribute.String("net.transport", "ip_tcp"),
attribute.Int("net.peer.port", 56),
},
},
{
Expand All @@ -236,7 +308,7 @@ func TestNetAttributesFromHTTPRequest(t *testing.T) {
},
},
{
name: "with host ip only",
name: "with host ipv4 only",
network: "tcp",
method: "GET",
requestURI: "/user/123",
Expand All @@ -254,6 +326,25 @@ func TestNetAttributesFromHTTPRequest(t *testing.T) {
attribute.String("net.host.ip", "4.3.2.1"),
},
},
{
name: "with host ipv6 only",
network: "tcp",
method: "GET",
requestURI: "/user/123",
proto: "HTTP/1.0",
remoteAddr: "1.2.3.4:56",
host: "fe80::0202:b3ff:fe1e:8329",
url: &url.URL{
Path: "/user/123",
},
header: nil,
expected: []attribute.KeyValue{
attribute.String("net.transport", "ip_tcp"),
attribute.String("net.peer.ip", "1.2.3.4"),
attribute.Int("net.peer.port", 56),
attribute.String("net.host.ip", "fe80::202:b3ff:fe1e:8329"),
},
},
{
name: "with host name and port",
network: "tcp",
Expand All @@ -275,7 +366,7 @@ func TestNetAttributesFromHTTPRequest(t *testing.T) {
},
},
{
name: "with host ip and port",
name: "with host ipv4 and port",
network: "tcp",
method: "GET",
requestURI: "/user/123",
Expand All @@ -294,6 +385,26 @@ func TestNetAttributesFromHTTPRequest(t *testing.T) {
attribute.Int("net.host.port", 78),
},
},
{
name: "with host ipv6 and port",
network: "tcp",
method: "GET",
requestURI: "/user/123",
proto: "HTTP/1.0",
remoteAddr: "1.2.3.4:56",
host: "[fe80::202:b3ff:fe1e:8329]:78",
url: &url.URL{
Path: "/user/123",
},
header: nil,
expected: []attribute.KeyValue{
attribute.String("net.transport", "ip_tcp"),
attribute.String("net.peer.ip", "1.2.3.4"),
attribute.Int("net.peer.port", 56),
attribute.String("net.host.ip", "fe80::202:b3ff:fe1e:8329"),
attribute.Int("net.host.port", 78),
},
},
{
name: "with host name and bogus port",
network: "tcp",
Expand All @@ -314,7 +425,7 @@ func TestNetAttributesFromHTTPRequest(t *testing.T) {
},
},
{
name: "with host ip and bogus port",
name: "with host ipv4 and bogus port",
network: "tcp",
method: "GET",
requestURI: "/user/123",
Expand All @@ -332,6 +443,25 @@ func TestNetAttributesFromHTTPRequest(t *testing.T) {
attribute.String("net.host.ip", "4.3.2.1"),
},
},
{
name: "with host ipv6 and bogus port",
network: "tcp",
method: "GET",
requestURI: "/user/123",
proto: "HTTP/1.0",
remoteAddr: "1.2.3.4:56",
host: "[fe80::202:b3ff:fe1e:8329]:qwerty",
url: &url.URL{
Path: "/user/123",
},
header: nil,
expected: []attribute.KeyValue{
attribute.String("net.transport", "ip_tcp"),
attribute.String("net.peer.ip", "1.2.3.4"),
attribute.Int("net.peer.port", 56),
attribute.String("net.host.ip", "fe80::202:b3ff:fe1e:8329"),
},
},
{
name: "with empty host and port",
network: "tcp",
Expand All @@ -348,6 +478,7 @@ func TestNetAttributesFromHTTPRequest(t *testing.T) {
attribute.String("net.transport", "ip_tcp"),
attribute.String("net.peer.ip", "1.2.3.4"),
attribute.Int("net.peer.port", 56),
attribute.Int("net.host.port", 80),
},
},
{
Expand All @@ -373,7 +504,7 @@ func TestNetAttributesFromHTTPRequest(t *testing.T) {
},
},
{
name: "with host ip and port in url",
name: "with host ipv4 and port in url",
network: "tcp",
method: "GET",
requestURI: "http://4.3.2.1:78/user/123",
Expand All @@ -393,11 +524,39 @@ func TestNetAttributesFromHTTPRequest(t *testing.T) {
attribute.Int("net.host.port", 78),
},
},
{
name: "with host ipv6 and port in url",
network: "tcp",
method: "GET",
requestURI: "http://4.3.2.1:78/user/123",
proto: "HTTP/1.0",
remoteAddr: "1.2.3.4:56",
host: "",
url: &url.URL{
Host: "[fe80::202:b3ff:fe1e:8329]:78",
Path: "/user/123",
},
header: nil,
expected: []attribute.KeyValue{
attribute.String("net.transport", "ip_tcp"),
attribute.String("net.peer.ip", "1.2.3.4"),
attribute.Int("net.peer.port", 56),
attribute.String("net.host.ip", "fe80::202:b3ff:fe1e:8329"),
attribute.Int("net.host.port", 78),
},
},
}
for idx, tc := range testcases {
r := testRequest(tc.method, tc.requestURI, tc.proto, tc.remoteAddr, tc.host, tc.url, tc.header, noTLS)
got := NetAttributesFromHTTPRequest(tc.network, r)
assertElementsMatch(t, tc.expected, got, "testcase %d - %s", idx, tc.name)
for _, tc := range testcases {
t.Run(tc.name, func(t *testing.T) {
r := testRequest(tc.method, tc.requestURI, tc.proto, tc.remoteAddr, tc.host, tc.url, tc.header, noTLS)
got := NetAttributesFromHTTPRequest(tc.network, r)
if diff := cmp.Diff(
tc.expected,
got,
cmp.AllowUnexported(attribute.Value{})); diff != "" {
t.Fatalf("attributes differ: diff %+v,", diff)
}
})
}
}

Expand Down

0 comments on commit 996504b

Please sign in to comment.