Skip to content

Commit

Permalink
DE-1350 Add errcheck, ineffassign and staticcheck linters (#341)
Browse files Browse the repository at this point in the history
  • Loading branch information
vtopc authored Nov 9, 2024
1 parent 8619573 commit b08f773
Show file tree
Hide file tree
Showing 6 changed files with 81 additions and 39 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ jobs:
run: make nilaway

- name: lint
run: make lint-new
run: make lint

- name: Test
run: go test -v -race -p=1 -count=1
12 changes: 11 additions & 1 deletion .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,15 @@ linters-settings:
checks: ["all"]

issues:
# Which files to exclude: they will be analyzed, but issues from them won't be reported.
# There is no need to include all autogenerated files,
# we confidently recognize autogenerated files.
# If it's not, please let us know.
# "/" will be replaced by current OS file path separator to properly work on Windows.
# Default: []
exclude-files:
- "^mock_*"

# Maximum issues count per one linter. Set to 0 to disable. Default is 50.
max-issues-per-linter: 0

Expand All @@ -67,7 +76,8 @@ issues:

run:
# include test files or not, default is true
tests: true
# TODO(DE-1139): Enable
tests: false

# Timeout for analysis, e.g. 30s, 5m.
# Default: 1m
Expand Down
4 changes: 0 additions & 4 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,6 @@ GOLINT = $(GOPATH)/bin/golangci-lint
$(GOLINT):
curl -sfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(GOPATH)/bin v1.61.0

.PHONY: lint-new
lint-new: $(GOLINT)
$(GOLINT) run -new-from-rev=master

.PHONY: lint
lint: $(GOLINT)
$(GOLINT) run
1 change: 0 additions & 1 deletion events.go
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,6 @@ func (ep *EventPoller) Poll(ctx context.Context, events *[]Event) bool {
// If we have events to return
if len(results) != 0 {
*events = results
results = nil
return true
}

Expand Down
78 changes: 50 additions & 28 deletions httphelpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ type httpResponse struct {

type payload interface {
getPayloadBuffer() (*bytes.Buffer, error)
getContentType() string
getContentType() (string, error)
getValues() []keyValuePair
}

Expand Down Expand Up @@ -109,8 +109,8 @@ func (j *jsonEncodedPayload) getPayloadBuffer() (*bytes.Buffer, error) {
return bytes.NewBuffer(b), nil
}

func (j *jsonEncodedPayload) getContentType() string {
return "application/json"
func (j *jsonEncodedPayload) getContentType() (string, error) {
return "application/json", nil
}

func (j *jsonEncodedPayload) getValues() []keyValuePair {
Expand All @@ -134,8 +134,8 @@ func (f *urlEncodedPayload) getPayloadBuffer() (*bytes.Buffer, error) {
return bytes.NewBufferString(data.Encode()), nil
}

func (f *urlEncodedPayload) getContentType() string {
return "application/x-www-form-urlencoded"
func (f *urlEncodedPayload) getContentType() (string, error) {
return "application/x-www-form-urlencoded", nil
}

func (f *urlEncodedPayload) getValues() []keyValuePair {
Expand Down Expand Up @@ -177,24 +177,31 @@ func (f *formDataPayload) getPayloadBuffer() (*bytes.Buffer, error) {

for _, keyVal := range f.Values {
if tmp, err := writer.CreateFormField(keyVal.key); err == nil {
// TODO(DE-1139): handle error:
tmp.Write([]byte(keyVal.value))
_, err := tmp.Write([]byte(keyVal.value))
if err != nil {
return nil, err
}
} else {
return nil, err
}
}

for _, file := range f.Files {
if tmp, err := writer.CreateFormFile(file.key, path.Base(file.value)); err == nil {
if fp, err := os.Open(file.value); err == nil {
// TODO(DE-1139): defer in a loop:
defer fp.Close()
// TODO(DE-1139): handle error:
io.Copy(tmp, fp)
} else {
return nil, err
}
} else {
tmp, err := writer.CreateFormFile(file.key, path.Base(file.value))
if err != nil {
return nil, err
}

fp, err := os.Open(file.value)
if err != nil {
return nil, err
}

// TODO(DE-1139): defer in a loop:
defer fp.Close()

_, err = io.Copy(tmp, fp)
if err != nil {
return nil, err
}
}
Expand All @@ -203,8 +210,11 @@ func (f *formDataPayload) getPayloadBuffer() (*bytes.Buffer, error) {
if tmp, err := writer.CreateFormFile(file.key, file.name); err == nil {
// TODO(DE-1139): defer in a loop:
defer file.value.Close()
// TODO(DE-1139): handle error:
io.Copy(tmp, file.value)

_, err := io.Copy(tmp, file.value)
if err != nil {
return nil, err
}
} else {
return nil, err
}
Expand All @@ -213,25 +223,31 @@ func (f *formDataPayload) getPayloadBuffer() (*bytes.Buffer, error) {
for _, buff := range f.Buffers {
if tmp, err := writer.CreateFormFile(buff.key, buff.name); err == nil {
r := bytes.NewReader(buff.value)
// TODO(DE-1139): handle error:
io.Copy(tmp, r)

_, err := io.Copy(tmp, r)
if err != nil {
return nil, err
}
} else {
return nil, err
}
}

// TODO(vtopc): getPayloadBuffer is not just a getter, it also sets the content type
f.contentType = writer.FormDataContentType()

return data, nil
}

func (f *formDataPayload) getContentType() string {
func (f *formDataPayload) getContentType() (string, error) {
if f.contentType == "" {
// TODO(DE-1139): handle error:
f.getPayloadBuffer()
_, err := f.getPayloadBuffer()
if err != nil {
return "", err
}
}

return f.contentType
return f.contentType, nil
}

func (r *httpRequest) addHeader(name, value string) {
Expand Down Expand Up @@ -277,8 +293,13 @@ func (r *httpRequest) NewRequest(ctx context.Context, method string, payload pay
return nil, err
}

if payload != nil && payload.getContentType() != "" {
req.Header.Add("Content-Type", payload.getContentType())
if payload != nil {
contentType, err := payload.getContentType()
if err != nil {
return nil, err
}

req.Header.Add("Content-Type", contentType)
}

if r.BasicAuthUser != "" && r.BasicAuthPassword != "" {
Expand Down Expand Up @@ -381,7 +402,8 @@ func (r *httpRequest) curlString(req *http.Request, p payload) string {
// parts = append(parts, fmt.Sprintf(" --user '%s:%s'", r.BasicAuthUser, r.BasicAuthPassword))

if p != nil {
if p.getContentType() == "application/json" {
contentType, _ := p.getContentType()
if contentType == "application/json" {
b, err := p.getPayloadBuffer()
if err != nil {
return "Unable to get payload buffer: " + err.Error()
Expand Down
23 changes: 19 additions & 4 deletions webhooks.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,8 +124,15 @@ type WebhookPayload struct {
// Use this method to parse the webhook signature given as JSON in the webhook response
func (mg *MailgunImpl) VerifyWebhookSignature(sig Signature) (verified bool, err error) {
h := hmac.New(sha256.New, []byte(mg.APIKey()))
io.WriteString(h, sig.TimeStamp)
io.WriteString(h, sig.Token)

_, err = io.WriteString(h, sig.TimeStamp)
if err != nil {
return false, err
}
_, err = io.WriteString(h, sig.Token)
if err != nil {
return false, err
}

calculatedSignature := h.Sum(nil)
signature, err := hex.DecodeString(sig.Signature)
Expand All @@ -143,8 +150,16 @@ func (mg *MailgunImpl) VerifyWebhookSignature(sig Signature) (verified bool, err
// version of WebHooks from mailgun
func (mg *MailgunImpl) VerifyWebhookRequest(req *http.Request) (verified bool, err error) {
h := hmac.New(sha256.New, []byte(mg.APIKey()))
io.WriteString(h, req.FormValue("timestamp"))
io.WriteString(h, req.FormValue("token"))

_, err = io.WriteString(h, req.FormValue("timestamp"))
if err != nil {
return false, err
}

_, err = io.WriteString(h, req.FormValue("token"))
if err != nil {
return false, err
}

calculatedSignature := h.Sum(nil)
signature, err := hex.DecodeString(req.FormValue("signature"))
Expand Down

0 comments on commit b08f773

Please sign in to comment.