From eea9021771f09b8808bcecd7745a7e6206c824ae Mon Sep 17 00:00:00 2001
From: Sven Rebhan <36194019+srebhan@users.noreply.github.com>
Date: Tue, 6 Dec 2022 15:47:58 +0100
Subject: [PATCH] fix(inputs.modbus): Fix Windows COM-port path (#12339)

---
 plugins/inputs/modbus/README.md               |  9 ++-
 plugins/inputs/modbus/modbus.go               | 11 ++-
 plugins/inputs/modbus/modbus_test.go          | 70 +++++++++++++++++++
 .../inputs/modbus/sample_general_begin.conf   |  9 ++-
 4 files changed, 90 insertions(+), 9 deletions(-)

diff --git a/plugins/inputs/modbus/README.md b/plugins/inputs/modbus/README.md
index 9e22895e2..fa66b54b5 100644
--- a/plugins/inputs/modbus/README.md
+++ b/plugins/inputs/modbus/README.md
@@ -41,15 +41,18 @@ See the [CONFIGURATION.md][CONFIGURATION.md] for more details.
   controller = "tcp://localhost:502"
 
   ## Serial (RS485; RS232)
+  ## For unix-like operating systems use:
   # controller = "file:///dev/ttyUSB0"
+  ## For Windows operating systems use:
+  # controller = "COM1"
   # baud_rate = 9600
   # data_bits = 8
   # parity = "N"
   # stop_bits = 1
 
-  ## For Modbus over TCP you can choose between "TCP", "RTUoverTCP" and "ASCIIoverTCP"
-  ## default behaviour is "TCP" if the controller is TCP
-  ## For Serial you can choose between "RTU" and "ASCII"
+  ## For Modbus over TCP you can choose between "TCP", "RTUoverTCP" and
+  ## "ASCIIoverTCP". The default behaviour is "TCP" for ModbusTCP controllers.
+  ## For Serial controllers you can choose between "RTU" and "ASCII".
   # transmission_mode = "RTU"
 
   ## Trace the connection to the modbus device as debug messages
diff --git a/plugins/inputs/modbus/modbus.go b/plugins/inputs/modbus/modbus.go
index 2dcad559a..e1d64826e 100644
--- a/plugins/inputs/modbus/modbus.go
+++ b/plugins/inputs/modbus/modbus.go
@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"net"
 	"net/url"
+	"path/filepath"
 	"strconv"
 	"time"
 
@@ -256,10 +257,14 @@ func (m *Modbus) initClient() error {
 			}
 			m.handler = handler
 		}
-	case "file":
+	case "", "file":
+		path := filepath.Join(u.Host, u.Path)
+		if path == "" {
+			return fmt.Errorf("invalid path for controller %q", m.Controller)
+		}
 		switch m.TransmissionMode {
 		case "RTU":
-			handler := mb.NewRTUClientHandler(u.Path)
+			handler := mb.NewRTUClientHandler(path)
 			handler.Timeout = time.Duration(m.Timeout)
 			handler.BaudRate = m.BaudRate
 			handler.DataBits = m.DataBits
@@ -270,7 +275,7 @@ func (m *Modbus) initClient() error {
 			}
 			m.handler = handler
 		case "ASCII":
-			handler := mb.NewASCIIClientHandler(u.Path)
+			handler := mb.NewASCIIClientHandler(path)
 			handler.Timeout = time.Duration(m.Timeout)
 			handler.BaudRate = m.BaudRate
 			handler.DataBits = m.DataBits
diff --git a/plugins/inputs/modbus/modbus_test.go b/plugins/inputs/modbus/modbus_test.go
index d4a4d49bf..53689c422 100644
--- a/plugins/inputs/modbus/modbus_test.go
+++ b/plugins/inputs/modbus/modbus_test.go
@@ -21,6 +21,76 @@ import (
 	"github.com/influxdata/telegraf/testutil"
 )
 
+func TestControllers(t *testing.T) {
+	var tests = []struct {
+		name       string
+		controller string
+		errmsg     string
+	}{
+		{
+			name:       "TCP host",
+			controller: "tcp://localhost:502",
+		},
+		{
+			name:       "invalid TCP host",
+			controller: "tcp://localhost",
+			errmsg:     "initializing client failed: address localhost: missing port in address",
+		},
+		{
+			name:       "absolute file path",
+			controller: "file:///dev/ttyUSB0",
+		},
+		{
+			name:       "relative file path",
+			controller: "file://dev/ttyUSB0",
+		},
+		{
+			name:       "relative file path with dot",
+			controller: "file://./dev/ttyUSB0",
+		},
+		{
+			name:       "Windows COM-port",
+			controller: "COM2",
+		},
+		{
+			name:       "Windows COM-port file path",
+			controller: "file://com2",
+		},
+		{
+			name:       "empty file path",
+			controller: "file://",
+			errmsg:     "initializing client failed: invalid path for controller",
+		},
+		{
+			name:       "empty controller",
+			controller: "",
+			errmsg:     "initializing client failed: invalid path for controller",
+		},
+		{
+			name:       "invalid scheme",
+			controller: "foo://bar",
+			errmsg:     "initializing client failed: invalid controller",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			plugin := Modbus{
+				Name:             "dummy",
+				Controller:       tt.controller,
+				TransmissionMode: "RTU",
+				Log:              testutil.Logger{},
+			}
+			err := plugin.Init()
+			if tt.errmsg != "" {
+				require.ErrorContains(t, err, tt.errmsg)
+			} else {
+				require.NoError(t, err)
+			}
+		})
+	}
+}
+
 func TestCoils(t *testing.T) {
 	var coilTests = []struct {
 		name     string
diff --git a/plugins/inputs/modbus/sample_general_begin.conf b/plugins/inputs/modbus/sample_general_begin.conf
index b3c5e677e..54bc7e7d9 100644
--- a/plugins/inputs/modbus/sample_general_begin.conf
+++ b/plugins/inputs/modbus/sample_general_begin.conf
@@ -24,15 +24,18 @@
   controller = "tcp://localhost:502"
 
   ## Serial (RS485; RS232)
+  ## For unix-like operating systems use:
   # controller = "file:///dev/ttyUSB0"
+  ## For Windows operating systems use:
+  # controller = "COM1"
   # baud_rate = 9600
   # data_bits = 8
   # parity = "N"
   # stop_bits = 1
 
-  ## For Modbus over TCP you can choose between "TCP", "RTUoverTCP" and "ASCIIoverTCP"
-  ## default behaviour is "TCP" if the controller is TCP
-  ## For Serial you can choose between "RTU" and "ASCII"
+  ## For Modbus over TCP you can choose between "TCP", "RTUoverTCP" and
+  ## "ASCIIoverTCP". The default behaviour is "TCP" for ModbusTCP controllers.
+  ## For Serial controllers you can choose between "RTU" and "ASCII".
   # transmission_mode = "RTU"
 
   ## Trace the connection to the modbus device as debug messages