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.

Sign up for GitHub

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

Jump to bottom

Fix IPv6 parsing errors in semconv.NetAttributesFromHTTPRequest #2285

Merged
merged 9 commits into from Oct 18, 2021
Merged

Fix IPv6 parsing errors in semconv.NetAttributesFromHTTPRequest #2285

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
996504b
Fix IPv6 handling errors in semconv.NetAttributesFromHTTPRequest
evantorrie Oct 13, 2021
e674a99
Enter PR number in CHANGELOG
evantorrie Oct 13, 2021
d4017e2
Remove unnecessary creation and then assignment
evantorrie Oct 13, 2021
1335150
Assume happy path when parsing host and port
evantorrie Oct 14, 2021
1958448
Get rid of uint64 for port
evantorrie Oct 14, 2021
523c66d
Merge branch 'main' into http_attribute_fix
MrAlias Oct 18, 2021
5b100f8
Merge branch 'main' into http_attribute_fix
MrAlias Oct 18, 2021
4ee3142
Merge branch 'main' into http_attribute_fix
MrAlias Oct 18, 2021
69b88ab
Fix git merge of main by adding back strings import
MrAlias Oct 18, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Expand Up @@ -23,6 +23,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)
- Added a new `schema` module to help parse Schema Files in OTEP 0152 format. (#2267)

### Fixed

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

## [1.0.1] - 2021-10-01

### Fixed
Expand Down
76 changes: 29 additions & 47 deletions semconv/v1.4.0/http.go
View file
Open in desktop
Expand Up @@ -51,64 +51,22 @@ func NetAttributesFromHTTPRequest(network string, request *http.Request) []attri
attrs = append(attrs, NetTransportOther)
}

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 peerIP != "" {
attrs = append(attrs, NetPeerIPKey.String(peerIP))
}
if peerName != "" {
attrs = append(attrs, NetPeerNameKey.String(peerName))
}
if peerIP != "" {
attrs = append(attrs, NetPeerIPKey.String(peerIP))
}
if peerPort != 0 {
attrs = append(attrs, NetPeerPortKey.Int(peerPort))
}

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 +82,30 @@ 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) {
var (
hostPart, portPart string
parsedPort uint64
err error
)
if hostPart, portPart, err = net.SplitHostPort(hostWithPort); err != nil {
hostPart, portPart = hostWithPort, ""
}
if parsedIP := net.ParseIP(hostPart); parsedIP != nil {
ip = parsedIP.String()
} else {
name = hostPart
}
if parsedPort, err = strconv.ParseUint(portPart, 10, 16); err == nil {
port = int(parsedPort)
}
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
Open in desktop
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