From 10962b685ef08579846342b3ab80507306ccd494 Mon Sep 17 00:00:00 2001
From: meowsbits <b5c6@protonmail.com>
Date: Tue, 25 May 2021 16:22:46 -0500
Subject: [PATCH] ethstats: fix URL parser for '@' or ':' in node name/password
 (#21640)

Fixes the case (example below) where the value passed
to --ethstats flag would be parsed wrongly because the
node name and/or password value contained the special
characters '@' or ':'

    --ethstats "ETC Labs Metrics @meowsbits":mypass@ws://mordor.dash.fault.dev:3000
---
 ethstats/ethstats.go      | 39 +++++++++++++++++------
 ethstats/ethstats_test.go | 67 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 97 insertions(+), 9 deletions(-)
 create mode 100644 ethstats/ethstats_test.go

diff --git a/ethstats/ethstats.go b/ethstats/ethstats.go
index c7acb9481c..ef83e5a4eb 100644
--- a/ethstats/ethstats.go
+++ b/ethstats/ethstats.go
@@ -24,7 +24,6 @@ import (
 	"fmt"
 	"math/big"
 	"net/http"
-	"regexp"
 	"runtime"
 	"strconv"
 	"strings"
@@ -144,21 +143,43 @@ func (w *connWrapper) Close() error {
 	return w.conn.Close()
 }
 
+// parseEthstatsURL parses the netstats connection url.
+// URL argument should be of the form <nodename:secret@host:port>
+// If non-erroring, the returned slice contains 3 elements: [nodename, pass, host]
+func parseEthstatsURL(url string) (parts []string, err error) {
+	err = fmt.Errorf("invalid netstats url: \"%s\", should be nodename:secret@host:port", url)
+
+	hostIndex := strings.LastIndex(url, "@")
+	if hostIndex == -1 || hostIndex == len(url)-1 {
+		return nil, err
+	}
+	preHost, host := url[:hostIndex], url[hostIndex+1:]
+
+	passIndex := strings.LastIndex(preHost, ":")
+	if passIndex == -1 {
+		return []string{preHost, "", host}, nil
+	}
+	nodename, pass := preHost[:passIndex], ""
+	if passIndex != len(preHost)-1 {
+		pass = preHost[passIndex+1:]
+	}
+
+	return []string{nodename, pass, host}, nil
+}
+
 // New returns a monitoring service ready for stats reporting.
 func New(node *node.Node, backend backend, engine consensus.Engine, url string) error {
-	// Parse the netstats connection url
-	re := regexp.MustCompile("([^:@]*)(:([^@]*))?@(.+)")
-	parts := re.FindStringSubmatch(url)
-	if len(parts) != 5 {
-		return fmt.Errorf("invalid netstats url: \"%s\", should be nodename:secret@host:port", url)
+	parts, err := parseEthstatsURL(url)
+	if err != nil {
+		return err
 	}
 	ethstats := &Service{
 		backend: backend,
 		engine:  engine,
 		server:  node.Server(),
-		node:    parts[1],
-		pass:    parts[3],
-		host:    parts[4],
+		node:    parts[0],
+		pass:    parts[1],
+		host:    parts[2],
 		pongCh:  make(chan struct{}),
 		histCh:  make(chan []uint64, 1),
 	}
diff --git a/ethstats/ethstats_test.go b/ethstats/ethstats_test.go
new file mode 100644
index 0000000000..92cec50c4d
--- /dev/null
+++ b/ethstats/ethstats_test.go
@@ -0,0 +1,67 @@
+package ethstats
+
+import (
+	"strconv"
+	"testing"
+)
+
+func TestParseEthstatsURL(t *testing.T) {
+	cases := []struct {
+		url              string
+		node, pass, host string
+	}{
+		{
+			url:  `"debug meowsbits":mypass@ws://mordor.dash.fault.dev:3000`,
+			node: "debug meowsbits", pass: "mypass", host: "ws://mordor.dash.fault.dev:3000",
+		},
+		{
+			url:  `"debug @meowsbits":mypass@ws://mordor.dash.fault.dev:3000`,
+			node: "debug @meowsbits", pass: "mypass", host: "ws://mordor.dash.fault.dev:3000",
+		},
+		{
+			url:  `"debug: @meowsbits":mypass@ws://mordor.dash.fault.dev:3000`,
+			node: "debug: @meowsbits", pass: "mypass", host: "ws://mordor.dash.fault.dev:3000",
+		},
+		{
+			url:  `name:@ws://mordor.dash.fault.dev:3000`,
+			node: "name", pass: "", host: "ws://mordor.dash.fault.dev:3000",
+		},
+		{
+			url:  `name@ws://mordor.dash.fault.dev:3000`,
+			node: "name", pass: "", host: "ws://mordor.dash.fault.dev:3000",
+		},
+		{
+			url:  `:mypass@ws://mordor.dash.fault.dev:3000`,
+			node: "", pass: "mypass", host: "ws://mordor.dash.fault.dev:3000",
+		},
+		{
+			url:  `:@ws://mordor.dash.fault.dev:3000`,
+			node: "", pass: "", host: "ws://mordor.dash.fault.dev:3000",
+		},
+	}
+
+	for i, c := range cases {
+		parts, err := parseEthstatsURL(c.url)
+		if err != nil {
+			t.Fatal(err)
+		}
+		node, pass, host := parts[0], parts[1], parts[2]
+
+		// unquote because the value provided will be used as a CLI flag value, so unescaped quotes will be removed
+		nodeUnquote, err := strconv.Unquote(node)
+		if err == nil {
+			node = nodeUnquote
+		}
+
+		if node != c.node {
+			t.Errorf("case=%d mismatch node value, got: %v ,want: %v", i, node, c.node)
+		}
+		if pass != c.pass {
+			t.Errorf("case=%d mismatch pass value, got: %v ,want: %v", i, pass, c.pass)
+		}
+		if host != c.host {
+			t.Errorf("case=%d mismatch host value, got: %v ,want: %v", i, host, c.host)
+		}
+	}
+
+}