Skip to content

Commit

Permalink
cmd/govim: fix panic when using custom hover opts "col" / "line"
Browse files Browse the repository at this point in the history
Setting either ExperimentalCursorTriggeredHoverPopupOptions or
ExperimentalMouseTriggeredHoverPopupOptions fields "col" or "line"
did result in a panic.

The value was parsed as a raw json message, but since it is already
parsed once before it paniced. We now type assert the value instead.

Updates #1012
  • Loading branch information
leitzler committed Jan 8, 2021
1 parent 4c1364f commit b9d026d
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 23 deletions.
33 changes: 10 additions & 23 deletions cmd/govim/hover.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"encoding/json"
"fmt"
"math"
"strings"

"github.com/govim/govim/cmd/govim/config"
Expand Down Expand Up @@ -78,11 +79,13 @@ func (v *vimstate) showHover(posExpr string, opts map[string]interface{}, userOp
formatPopupline := func(msg, source string, severity types.Severity) types.PopupLine {
srcProp := string(config.HighlightHoverDiagSrc)
msgProp := string(types.SeverityHoverHighlight[severity])
return types.PopupLine{Text: fmt.Sprintf("%s %s", msg, source),
return types.PopupLine{
Text: fmt.Sprintf("%s %s", msg, source),
Props: []types.PopupProp{
{Type: msgProp, Col: 1, Len: len(msg) + 1 + len(source)}, // Diagnostic message
{Type: srcProp, Col: len(msg) + 2, Len: len(source)}, // Source
}}
},
}
}

var lines []types.PopupLine
Expand Down Expand Up @@ -113,16 +116,12 @@ func (v *vimstate) showHover(posExpr string, opts map[string]interface{}, userOp
opts[k] = v
}
var line, col int64
var err error
if lv, ok := opts["line"]; ok {
if line, err = rawToInt(lv); err != nil {
return nil, fmt.Errorf("failed to parse line option: %v", err)
}
// TODO: we should use json.Decoder.UseNumber() instead of treating ints as floats.
if lv, ok := opts["line"].(float64); ok {
line = int64(math.Round(lv))
}
if cv, ok := opts["col"]; ok {
if col, err = rawToInt(cv); err != nil {
return nil, fmt.Errorf("failed to parse col option: %v", err)
}
if cv, ok := opts["col"].(float64); ok {
col = int64(math.Round(cv))
}
opts["line"] = line + int64(vpos.ScreenPos.Row)
opts["col"] = col + int64(vpos.ScreenPos.Col)
Expand All @@ -140,15 +139,3 @@ func (v *vimstate) showHover(posExpr string, opts map[string]interface{}, userOp
v.ChannelRedraw(false)
return "", nil
}

func rawToInt(i interface{}) (int64, error) {
var n json.Number
if err := json.Unmarshal(i.(json.RawMessage), &n); err != nil {
return 0, fmt.Errorf("failed to parse number: %v", err)
}
v, err := n.Int64()
if err != nil {
return 0, fmt.Errorf("failed to parse integer from line option: %v", err)
}
return v, nil
}
38 changes: 38 additions & 0 deletions cmd/govim/testdata/scenario_bugs/bug_govim_1013.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
# Test that custom hover opts col & line aren't causing a panic (govim/govim#1013)

[!vim] [!gvim] skip 'Test only known to work in Vim and GVim'

[!vim:v8.1.1649] skip 'New style popup tests do not work on gvim https://github.com/govim/govim/issues/351'

vim ex 'call govim#config#Set(\"ExperimentalMouseTriggeredHoverPopupOptions\", { \"pos\": \"topright\", \"col\": 9999, \"line\": -9999 })'

vim ex 'e main.go'
vim ex 'call test_setmouse(screenpos(bufwinid(\"main.go\"),6,13)[\"row\"],screenpos(bufwinid(\"main.go\"),6,13)[\"col\"])'
vim ex 'call feedkeys(\"\\<MouseMove>\\<Ignore>\", \"xt\")'
sleep 500ms
vim -stringout expr 'GOVIM_internal_DumpPopups()'
cmp stdout popup.golden
! stderr .+

# Assert that we have received no error (Type: 1) or warning (Type: 2) log messages
# Disabled pending resolution to https://github.com/golang/go/issues/34103
# errlogmatch -start -count=0 'LogMessage callback: &protocol\.LogMessageParams\{Type:(1|2), Message:".*'

-- go.mod --
module mod.com

go 1.12

-- main.go --
package main

import "fmt"

func main() {
fmt.Println("Hello, world")
}
-- popup.golden --
func fmt.Println(a ...interface{}) (n int, err error)
Println formats using the default formats for its operands and writes to standard output.
Spaces are always added between operands and a newline is appended.
It returns the number of bytes written and any write error encountered.

0 comments on commit b9d026d

Please sign in to comment.