This is an automated email from the ASF dual-hosted git repository. sjanc pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/mynewt-newtmgr.git
commit f698e60c3acacf438e83044c4f3f677c407b6b63 Author: Szymon Janc <[email protected]> AuthorDate: Thu Oct 22 00:23:54 2026 +0200 newtmgr/cli: Fix panic in conn add on a variable without '=' "newtmgr conn add p1 type" (or "connstring" without a value) indexed the missing value of the split key=value token and crashed with "index out of range". Report a usage error instead, like for other bad input. To make this testable without going through nmUsage(), which exits the process, move the parsing of the variable list into a small helper, connProfileParseVars(), that returns an error. Assisted-by: Claude:claude-opus-5-5 --- newtmgr/cli/connprofile.go | 43 +++++++++++++------- newtmgr/cli/connprofile_test.go | 89 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 118 insertions(+), 14 deletions(-) diff --git a/newtmgr/cli/connprofile.go b/newtmgr/cli/connprofile.go index c3a614d..d69fc72 100644 --- a/newtmgr/cli/connprofile.go +++ b/newtmgr/cli/connprofile.go @@ -30,6 +30,33 @@ import ( "github.com/spf13/cobra" ) +// connProfileParseVars applies "key=value" variable definitions from the +// "conn add" command line to the given connection profile. +func connProfileParseVars(cp *config.ConnProfile, vdefs []string) error { + for _, vdef := range vdefs { + s := strings.SplitN(vdef, "=", 2) + if len(s) != 2 { + return util.FmtNewtError( + "Invalid variable definition (expected key=value): %s", vdef) + } + + switch s[0] { + case "type": + var err error + cp.Type, err = config.ConnTypeFromString(s[1]) + if err != nil { + return err + } + case "connstring": + cp.ConnString = s[1] + default: + return util.NewNewtError("Unknown variable " + s[0]) + } + } + + return nil +} + func connProfileAddCmd(cmd *cobra.Command, args []string) { cpm := config.GlobalConnProfileMgr() @@ -43,20 +70,8 @@ func connProfileAddCmd(cmd *cobra.Command, args []string) { cp.Name = name cp.Type = config.CONN_TYPE_NONE - for _, vdef := range args[1:] { - s := strings.SplitN(vdef, "=", 2) - switch s[0] { - case "type": - var err error - cp.Type, err = config.ConnTypeFromString(s[1]) - if err != nil { - nmUsage(cmd, err) - } - case "connstring": - cp.ConnString = s[1] - default: - nmUsage(cmd, util.NewNewtError("Unknown variable "+s[0])) - } + if err := connProfileParseVars(cp, args[1:]); err != nil { + nmUsage(cmd, err) } // Check that a type is specified. diff --git a/newtmgr/cli/connprofile_test.go b/newtmgr/cli/connprofile_test.go new file mode 100644 index 0000000..6daec36 --- /dev/null +++ b/newtmgr/cli/connprofile_test.go @@ -0,0 +1,89 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package cli + +import ( + "testing" + + "mynewt.apache.org/newtmgr/newtmgr/config" +) + +func TestConnProfileParseVars(t *testing.T) { + tests := []struct { + name string + vdefs []string + want config.ConnProfile + }{ + {"none", nil, config.ConnProfile{Name: "p"}}, + {"type", []string{"type=serial"}, + config.ConnProfile{Name: "p", Type: config.CONN_TYPE_SERIAL_PLAIN}}, + {"type and connstring", + []string{"type=oic_ble", "connstring=peer_name=x,conn_timeout=5"}, + config.ConnProfile{ + Name: "p", + Type: config.CONN_TYPE_BLL_OIC, + ConnString: "peer_name=x,conn_timeout=5", + }}, + {"empty connstring", []string{"connstring="}, + config.ConnProfile{Name: "p"}}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cp := &config.ConnProfile{Name: "p"} + if err := connProfileParseVars(cp, tt.vdefs); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if *cp != tt.want { + t.Errorf("got %+v, want %+v", *cp, tt.want) + } + }) + } +} + +func TestConnProfileParseVarsErrors(t *testing.T) { + tests := []struct { + vdefs []string + err string + }{ + // Regression: a token without '=' used to panic. + {[]string{"type"}, + "Invalid variable definition (expected key=value): type"}, + {[]string{"type=serial", "connstring"}, + "Invalid variable definition (expected key=value): connstring"}, + {[]string{""}, + "Invalid variable definition (expected key=value): "}, + {[]string{"type=bogus"}, "Invalid connection type: bogus"}, + {[]string{"type="}, "Invalid connection type: "}, + {[]string{"foo=bar"}, "Unknown variable foo"}, + } + + for _, tt := range tests { + cp := config.NewConnProfile() + err := connProfileParseVars(cp, tt.vdefs) + if err == nil { + t.Errorf("%q: expected error", tt.vdefs) + continue + } + if err.Error() != tt.err { + t.Errorf("%q: error = %q, want %q", tt.vdefs, err.Error(), tt.err) + } + } +}
