From 720dfa4b41cbff6e700e59111fbc83456119c5cd Mon Sep 17 00:00:00 2001 From: wiggin77 Date: Thu, 15 Oct 2026 17:34:02 -0400 Subject: [PATCH] MM-71178 Catch unclosed curly quotes and split quote error messages A missing curly closing quote still swallowed the flags after it, because the next value opens with a left curly quote and the scan only looked for the right one. Treat a left curly quote found before the closing one as an unclosed quote. The unclosed quote message told users of double quotes to switch to double quotes. Split it into a plain unclosed quote message and one for a single quoted value that reaches " --". --- .../app/slashcommands/command_remote_test.go | 2 +- server/channels/app/slashcommands/util.go | 29 ++++++++++++------- .../channels/app/slashcommands/util_test.go | 5 +++- server/i18n/en.json | 9 ++++-- 4 files changed, 30 insertions(+), 15 deletions(-) diff --git a/server/channels/app/slashcommands/command_remote_test.go b/server/channels/app/slashcommands/command_remote_test.go index 56b4b39e0a4..cbd78f5ebd0 100644 --- a/server/channels/app/slashcommands/command_remote_test.go +++ b/server/channels/app/slashcommands/command_remote_test.go @@ -112,7 +112,7 @@ func TestRemoteProviderDoRemove(t *testing.T) { "empty quotes": {`/secure-connection remove --connectionID ""`, "Missing or empty `connectionID`"}, "malformed": {"/secure-connection remove --connectionID foo", "`foo` is not a valid `connectionID`"}, "stray word": {"/secure-connection remove --connectionID foo bar", "Unexpected `bar`. Wrap a value that contains spaces in quotes, and use double quotes if the value also contains an apostrophe."}, - "open quote": {`/secure-connection remove --connectionID "foo`, "The quote at `\"foo` is never closed. A value in single quotes cannot contain ` --`, so use double quotes for that."}, + "open quote": {`/secure-connection remove --connectionID "foo`, "The quote at `\"foo` is never closed."}, } { t.Run(name, func(t *testing.T) { callsBefore := wrapper.deleteCalls.Load() diff --git a/server/channels/app/slashcommands/util.go b/server/channels/app/slashcommands/util.go index 3e17376602d..e94732b8f27 100644 --- a/server/channels/app/slashcommands/util.go +++ b/server/channels/app/slashcommands/util.go @@ -84,6 +84,11 @@ func (t commandToken) value() string { // Mobile and macOS autocorrect replace straight quotes with curly ones. var closingQuotes = map[rune]rune{'"': '"', '\'': '\'', '“': '”', '‘': '’'} +var ( + errUnterminatedQuote = errors.New("unterminated quote") + errFlagInSingleQuotes = errors.New("flag in single quoted value") +) + // tokenizeCommand splits cmd on whitespace, except inside a quoted value. A quote only opens // a value at the start of a word, so apostrophes such as O'Brien are kept. A double quoted // value ends at the next matching quote. See quotedToken for single quotes. @@ -93,9 +98,12 @@ func tokenizeCommand(T i18n.TranslateFunc, cmd string) ([]commandToken, error) { for rest != "" { var tok commandToken if open, size := utf8.DecodeRuneInString(rest); closingQuotes[open] != 0 { - quoted, after, ok := quotedToken(rest[size:], closingQuotes[open]) - if !ok { + quoted, after, err := quotedToken(rest[size:], closingQuotes[open]) + if err != nil { word, _ := nextWord(rest) + if errors.Is(err, errFlagInSingleQuotes) { + return nil, errors.New(T("api.command.named_args.single_quoted_flag", map[string]any{"Value": word})) + } return nil, errors.New(T("api.command.named_args.unterminated_quote", map[string]any{"Value": word})) } if next, _ := utf8.DecodeRuneInString(after); after != "" && !unicode.IsSpace(next) { @@ -121,27 +129,26 @@ func nextWord(s string) (string, string) { } // A single quote doubles as an apostrophe, so a single quoted value only ends at a matching -// quote followed by whitespace or the end of input. It may not contain " --", so a missing -// closing quote is an error instead of swallowing the flags after it. -func quotedToken(s string, closeQuote rune) (commandToken, string, bool) { +// quote followed by whitespace or the end of input. It may not contain " --". +func quotedToken(s string, closeQuote rune) (commandToken, string, error) { if closeQuote == '"' || closeQuote == '”' { end := strings.IndexRune(s, closeQuote) - if end < 0 { - return commandToken{}, "", false + if end < 0 || closeQuote == '”' && strings.ContainsRune(s[:end], '“') { + return commandToken{}, "", errUnterminatedQuote } - return commandToken{text: s[:end], quoted: true}, s[end+utf8.RuneLen(closeQuote):], true + return commandToken{text: s[:end], quoted: true}, s[end+utf8.RuneLen(closeQuote):], nil } for i, r := range s { after := s[i+utf8.RuneLen(r):] if r == closeQuote { if next, _ := utf8.DecodeRuneInString(after); after == "" || unicode.IsSpace(next) { - return commandToken{text: s[:i], quoted: true}, after, true + return commandToken{text: s[:i], quoted: true}, after, nil } } else if unicode.IsSpace(r) && strings.HasPrefix(strings.TrimLeftFunc(after, unicode.IsSpace), "--") { - return commandToken{}, "", false + return commandToken{}, "", errFlagInSingleQuotes } } - return commandToken{}, "", false + return commandToken{}, "", errUnterminatedQuote } func trimSpaceAndQuotes(s string) string { diff --git a/server/channels/app/slashcommands/util_test.go b/server/channels/app/slashcommands/util_test.go index 779ddd3ee78..e9404e681d3 100644 --- a/server/channels/app/slashcommands/util_test.go +++ b/server/channels/app/slashcommands/util_test.go @@ -57,6 +57,7 @@ func TestParseNamedArgs(t *testing.T) { func TestParseNamedArgsErrors(t *testing.T) { const ( + singleQuotedFlag = "api.command.named_args.single_quoted_flag" unexpectedValue = "api.command.named_args.unexpected_value" unterminatedQuote = "api.command.named_args.unterminated_quote" ) @@ -72,7 +73,9 @@ func TestParseNamedArgsErrors(t *testing.T) { {"unterminated quote before flag", "remote add --arg1 \"my remote --arg2 val2", unterminatedQuote}, {"unterminated quote before quoted value", "remote add --arg1 \"foo --arg2 \"Bar Baz\" --arg3 val3", unexpectedValue}, {"text after closing double quote", "remote add --arg1 \"a\"b c\"", unexpectedValue}, - {"single quoted value containing a flag", "remote add --arg1 'use --arg2 here'", unterminatedQuote}, + {"single quoted value containing a flag", "remote add --arg1 'use --arg2 here'", singleQuotedFlag}, + {"unterminated single quote at end", "remote add --arg1 'my remote", unterminatedQuote}, + {"unterminated curly quote before curly quoted value", "remote add --arg1 \u201cfoo --arg2 \u201cBar Baz\u201d --arg3 val3", unterminatedQuote}, {"mismatched curly quotes", "remote add --arg1 \u201cmy remote\" --arg2 val2", unterminatedQuote}, } diff --git a/server/i18n/en.json b/server/i18n/en.json index 47c0039bf22..a33b27181c0 100644 --- a/server/i18n/en.json +++ b/server/i18n/en.json @@ -1154,6 +1154,11 @@ "translation": "No permission to move command", "description": "API error when a user lacks permission to move a custom slash command to another team." }, + { + "id": "api.command.named_args.single_quoted_flag", + "translation": "The single quote at `{{.Value}}` is not closed before ` --`. Close it, or use double quotes if the value contains ` --`.", + "description": "Slash command error when a single quoted argument value reaches a flag before its closing quote." + }, { "id": "api.command.named_args.unexpected_value", "translation": "Unexpected `{{.Value}}`. Wrap a value that contains spaces in quotes, and use double quotes if the value also contains an apostrophe.", @@ -1161,8 +1166,8 @@ }, { "id": "api.command.named_args.unterminated_quote", - "translation": "The quote at `{{.Value}}` is never closed. A value in single quotes cannot contain ` --`, so use double quotes for that.", - "description": "Slash command error when a quoted argument value has no closing quote, or a single quoted value contains a flag." + "translation": "The quote at `{{.Value}}` is never closed.", + "description": "Slash command error when a quoted argument value has no closing quote." }, { "id": "api.command.team_mismatch.app_error",