From 25f99857cfce37e9e5979e6004052afa54d5d8ed Mon Sep 17 00:00:00 2001 From: Alisdair McDiarmid Date: Tue, 4 May 2021 08:31:51 -0400 Subject: [PATCH] cli: Fix crash with invalid JSON diagnostics If a JSON diagnostic value has a highlight end offset which is before the highlight start offset, this would previously panic. This commit adds a normalization step to prevent the crash. --- command/format/diagnostic.go | 10 ++++++ command/format/diagnostic_test.go | 55 +++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+) diff --git a/command/format/diagnostic.go b/command/format/diagnostic.go index 5156595a3..2d8522dea 100644 --- a/command/format/diagnostic.go +++ b/command/format/diagnostic.go @@ -240,6 +240,16 @@ func appendSourceSnippets(buf *bytes.Buffer, diag *viewsjson.Diagnostic, color * // Split the snippet and render the highlighted section with underlines start := snippet.HighlightStartOffset end := snippet.HighlightEndOffset + + // Only buggy diagnostics can have an end range before the start, but + // we need to ensure we don't crash here if that happens. + if end < start { + end = start + 1 + if end > len(code) { + end = len(code) + } + } + before, highlight, after := code[0:start], code[start:end], code[end:] code = fmt.Sprintf(color.Color("%s[underline]%s[reset]%s"), before, highlight, after) diff --git a/command/format/diagnostic_test.go b/command/format/diagnostic_test.go index cfbe27191..6680442d8 100644 --- a/command/format/diagnostic_test.go +++ b/command/format/diagnostic_test.go @@ -10,6 +10,8 @@ import ( "github.com/mitchellh/colorstring" "github.com/zclconf/go-cty/cty" + viewsjson "github.com/hashicorp/terraform/command/views/json" + "github.com/hashicorp/terraform/tfdiags" ) @@ -699,3 +701,56 @@ eventually make it onto multiple lines. THE END t.Fatalf("unexpected output: got:\n%s\nwant\n%s\n", output, expected) } } + +// Test cases covering invalid JSON diagnostics which should still render +// correctly. These JSON diagnostic values cannot be generated from the +// json.NewDiagnostic code path, but we may read and display JSON diagnostics +// in future from other sources. +func TestDiagnosticFromJSON_invalid(t *testing.T) { + tests := map[string]struct { + Diag *viewsjson.Diagnostic + Want string + }{ + "zero-value end range and highlight end byte": { + &viewsjson.Diagnostic{ + Severity: viewsjson.DiagnosticSeverityError, + Summary: "Bad end", + Detail: "It all went wrong.", + Range: &viewsjson.DiagnosticRange{ + Filename: "ohno.tf", + Start: viewsjson.Pos{Line: 1, Column: 23, Byte: 22}, + End: viewsjson.Pos{Line: 0, Column: 0, Byte: 0}, + }, + Snippet: &viewsjson.DiagnosticSnippet{ + Code: `resource "foo_bar "baz" {`, + StartLine: 1, + HighlightStartOffset: 22, + HighlightEndOffset: 0, + }, + }, + `[red]╷[reset] +[red]│[reset] [bold][red]Error: [reset][bold]Bad end[reset] +[red]│[reset] +[red]│[reset] on ohno.tf line 1: +[red]│[reset] 1: resource "foo_bar "baz[underline]"[reset] { +[red]│[reset] +[red]│[reset] It all went wrong. +[red]╵[reset] +`, + }, + } + + // This empty Colorize just passes through all of the formatting codes + // untouched, because it doesn't define any formatting keywords. + colorize := &colorstring.Colorize{} + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + got := strings.TrimSpace(DiagnosticFromJSON(test.Diag, colorize, 40)) + want := strings.TrimSpace(test.Want) + if got != want { + t.Errorf("wrong result\ngot:\n%s\n\nwant:\n%s\n\n", got, want) + } + }) + } +}