Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

execinquery panics in the presence of url.URL.Query #2835

Closed
4 tasks done
favonia opened this issue May 9, 2022 · 8 comments
Closed
4 tasks done

execinquery panics in the presence of url.URL.Query #2835

favonia opened this issue May 9, 2022 · 8 comments
Labels
bug Something isn't working dependencies Relates to an upstream dependency

Comments

@favonia
Copy link

favonia commented May 9, 2022

Welcome

  • Yes, I'm using a binary release within 2 latest major releases. Only such installations are supported.
  • Yes, I've searched similar issues on GitHub and didn't find any.
  • Yes, I've included all information below (version, config, etc).
  • Yes, I've tried with the standalone linter if available. (https://golangci-lint.run/usage/linters/)

Description of the problem

The linter execinquery panics when url.URL.Query is used. The core issue is that execinquery assumes any method with a name containing Query is a method call to DB.Query (or its friends), and then it further assumes the number of arguments in those method calls must be at least one. If the assumption is false, it panics. URL.Query in net/url is one such example from the standard library. I think the current approach of execinquery could potentially generate too many false positives, and more importantly, it should have avoided panicking in all cases. I thus suggest disabling this linter until it is further improved.

Related issue: 1uf3/execinquery#3

Version of golangci-lint

$ golangci-lint --version
golangci-lint has version 1.46.0 built from 7c1d8b75 on 2022-05-09T11:16:24Z

Configuration file

$ cat .golangci.yml
linters:
  disable-all: true
  enable:
    - execinquery

Go environment

$ go version && go env
go version go1.18.1 linux/amd64
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="<redacted>/.cache/go-build"
GOENV="<redacted>/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="<redacted>/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="<redacted>/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/lib/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/lib/go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.18.1"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/dev/null"
GOWORK=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build2475000282=/tmp/go-build -gno-record-gcc-switches"

Verbose output of running

$ golangci-lint cache clean
$ golangci-lint run -v url.go
INFO [config_reader] Config search paths: <redacted>
INFO [config_reader] Used config file .golangci.yaml 
INFO [lintersdb] Active 1 linters: [execinquery]  
INFO [loader] Go packages loading at mode 7 (name|compiled_files|files) took 69.780366ms 
INFO [runner/filename_unadjuster] Pre-built 0 adjustments in 88.822µs 
INFO [linters context/goanalysis] analyzers took 41.963µs with top 10 stages: inspect: 41.963µs 
ERRO [runner] Panic: execinquery: package "main" (isInitialPkg: true, needAnalyzeSource: true): runtime error: index out of range [0] with length 0: goroutine 554 [running]:
runtime/debug.Stack()
	runtime/debug/stack.go:24 +0x65
github.com/golangci/golangci-lint/pkg/golinters/goanalysis.(*action).analyzeSafe.func1()
	github.com/golangci/golangci-lint/pkg/golinters/goanalysis/runner_action.go:101 +0x155
panic({0xfb6880, 0xc001570bb8})
	runtime/panic.go:838 +0x207
github.com/lufeee/execinquery.run.func1({0x1263f20?, 0xc000704080})
	github.com/lufeee/execinquery@v1.0.0/execinquery.go:45 +0x3ae
golang.org/x/tools/go/ast/inspector.(*Inspector).Preorder(0xc000205b78, {0x0?, 0x191ee60?, 0x0?}, 0xc00071ed28)
	golang.org/x/tools@v0.1.11-0.20220316014157-77aa08bb151a/go/ast/inspector/inspector.go:77 +0x9a
github.com/lufeee/execinquery.run(0xc000617040)
	github.com/lufeee/execinquery@v1.0.0/execinquery.go:27 +0x77
github.com/golangci/golangci-lint/pkg/golinters/goanalysis.(*action).analyze(0xc0005feb40)
	github.com/golangci/golangci-lint/pkg/golinters/goanalysis/runner_action.go:187 +0x9c4
github.com/golangci/golangci-lint/pkg/golinters/goanalysis.(*action).analyzeSafe.func2()
	github.com/golangci/golangci-lint/pkg/golinters/goanalysis/runner_action.go:105 +0x1d
github.com/golangci/golangci-lint/pkg/timeutils.(*Stopwatch).TrackStage(0xc0000c7270, {0x105dc80, 0xb}, 0xc000076748)
	github.com/golangci/golangci-lint/pkg/timeutils/stopwatch.go:111 +0x4a
github.com/golangci/golangci-lint/pkg/golinters/goanalysis.(*action).analyzeSafe(0xc002006c00?)
	github.com/golangci/golangci-lint/pkg/golinters/goanalysis/runner_action.go:104 +0x85
github.com/golangci/golangci-lint/pkg/golinters/goanalysis.(*loadingPackage).analyze.func2(0xc0005feb40)
	github.com/golangci/golangci-lint/pkg/golinters/goanalysis/runner_loadingpackage.go:80 +0xb4
created by github.com/golangci/golangci-lint/pkg/golinters/goanalysis.(*loadingPackage).analyze
	github.com/golangci/golangci-lint/pkg/golinters/goanalysis/runner_loadingpackage.go:75 +0x1eb 
WARN [runner] Can't run linter execinquery: execinquery: execinquery: package "main" (isInitialPkg: true, needAnalyzeSource: true): runtime error: index out of range [0] with length 0 
INFO [runner] processing took 2.701µs with stages: max_same_issues: 386ns, skip_dirs: 303ns, nolint: 273ns, max_from_linter: 250ns, autogenerated_exclude: 159ns, uniq_by_line: 137ns, cgo: 132ns, path_prettifier: 131ns, max_per_file_from_linter: 129ns, exclude: 128ns, filename_unadjuster: 123ns, skip_files: 117ns, source_code: 60ns, diff: 57ns, exclude-rules: 57ns, identifier_marker: 55ns, severity-rules: 54ns, path_shortener: 52ns, sort_results: 51ns, path_prefixer: 47ns 
INFO [runner] linters took 448.439µs with stages: execinquery: 411.509µs 
ERRO Running error: 1 error occurred:
	* can't run linter execinquery: execinquery: execinquery: package "main" (isInitialPkg: true, needAnalyzeSource: true): runtime error: index out of range [0] with length 0
 
INFO Memory: 2 samples, avg is 49.0MB, max is 49.0MB 
INFO Execution took 75.509391ms                   

Code example or link to a public repository

package main

import "net/url"

func main() {
	u, _ := url.Parse("test?a=1")
	u.Query()
}
@favonia favonia added the bug Something isn't working label May 9, 2022
@boring-cyborg
Copy link

boring-cyborg bot commented May 9, 2022

Hey, thank you for opening your first Issue ! 🙂 If you would like to contribute we have a guide for contributors.

@ldez
Copy link
Member

ldez commented May 9, 2022

Hello,

It's an issue with execinquery

When a fix will be created, our bot will update the linter.

@ldez
Copy link
Member

ldez commented May 10, 2022

@lufeee can you take a look at this issue? thank you.

1uf3/execinquery#3

@ldez
Copy link
Member

ldez commented May 10, 2022

I created a fix 1uf3/execinquery#7

@1uf3
Copy link
Contributor

1uf3 commented May 10, 2022

Thanks for reporting.
I will see that issue and pull request.
wait a moment about fixing this bug

@Hades32

This comment was marked as off-topic.

@ninadingole
Copy link

I see the source repository execinquery released a new version with the fix, can we release golangci-lint version please? or what will be a quick solution with v1.46.0 to ignore this?

@favonia
Copy link
Author

favonia commented May 11, 2022

I see the source repository execinquery released a new version with the fix, can we release golangci-lint version please? or what will be a quick solution with v1.46.0 to ignore this?

@ninadingole In case it helps, I immediately disabled execinquery when 1.46.0 was out:

linters:
  disable:
    - execinquery

See configuration for more information.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working dependencies Relates to an upstream dependency
Projects
None yet
Development

No branches or pull requests

5 participants