Skip to content

Commit

Permalink
fix unused value assignments caught by static checker (cloudflare#718)
Browse files Browse the repository at this point in the history
- also update static checker's official import path
- various bugfixes regarding bad error handling or bad logic.
  • Loading branch information
lziest authored and kisom committed Jan 25, 2017
1 parent 8a9cc9c commit 4bfe535
Show file tree
Hide file tree
Showing 18 changed files with 73 additions and 24 deletions.
12 changes: 6 additions & 6 deletions api/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,22 +120,22 @@ func TestRigidHandle(t *testing.T) {
obj = map[string]interface{}{}
obj["critique"] = "it's OK"
obj["compliment"] = "it's not bad"
resp, body = post(t, obj, ts)
resp, _ = post(t, obj, ts)

if resp.StatusCode != http.StatusBadRequest {
t.Errorf("Test expected 400, have %d", resp.StatusCode)
}

// reject empty review
obj = map[string]interface{}{}
resp, body = post(t, obj, ts)
resp, _ = post(t, obj, ts)

if resp.StatusCode != http.StatusBadRequest {
t.Errorf("Test expected 400, have %d", resp.StatusCode)
}

// reject GET
resp, body = get(t, ts)
resp, _ = get(t, ts)

if resp.StatusCode != http.StatusMethodNotAllowed {
t.Errorf("Test expected 405, have %d", resp.StatusCode)
Expand Down Expand Up @@ -190,7 +190,7 @@ func TestCleverHandle(t *testing.T) {
obj = map[string]interface{}{}
obj["critique"] = "it's OK"
obj["compliment"] = "it's not bad"
resp, body = post(t, obj, ts)
_, body = post(t, obj, ts)

message = new(Response)
err = json.Unmarshal(body, message)
Expand All @@ -205,14 +205,14 @@ func TestCleverHandle(t *testing.T) {

// reject empty review
obj = map[string]interface{}{}
resp, body = post(t, obj, ts)
resp, _ = post(t, obj, ts)

if resp.StatusCode != http.StatusBadRequest {
t.Errorf("Test expected 400, have %d", resp.StatusCode)
}

// reject GET
resp, body = get(t, ts)
resp, _ = get(t, ts)

if resp.StatusCode != http.StatusMethodNotAllowed {
t.Errorf("Test expected 405, have %d", resp.StatusCode)
Expand Down
4 changes: 4 additions & 0 deletions api/crl/crl.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,10 @@ func crlHandler(w http.ResponseWriter, r *http.Request) error {
}

result, err := cert.CreateCRL(rand.Reader, key, revokedCerts, time.Now(), newExpiryTime)
if err != nil {
log.Debug("Unable to create CRL: %v", err)
return err
}

return api.SendResponse(w, result)
}
Expand Down
Binary file modified certdb/testdb/certstore_development.db
Binary file not shown.
2 changes: 1 addition & 1 deletion cli/cli_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ func TestReadStdin(t *testing.T) {
t.Fatal(err)
}

fn, err = ReadStdin("-")
_, err = ReadStdin("-")
if err != nil {
t.Fatal(err)
}
Expand Down
4 changes: 4 additions & 0 deletions cli/gencert/gencert.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,10 @@ func gencertMain(args []string, c cli.Config) error {
return err
}

if len(args) > 0 {
return errors.New("only one argument is accepted, please check with usage")
}

csrJSONFileBytes, err := cli.ReadStdin(csrJSONFile)
if err != nil {
return err
Expand Down
3 changes: 3 additions & 0 deletions cli/genkey/genkey.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ func genkeyMain(args []string, c cli.Config) (err error) {
if err != nil {
return
}
if len(args) > 0 {
return errors.New("only one argument is accepted, please check with usage")
}

csrFileBytes, err := cli.ReadStdin(csrFile)
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion cli/printdefault/printdefault.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ func printAvailable() {
}

func printDefaults(args []string, c cli.Config) (err error) {
arg, args, err := cli.PopFirstArgument(args)
arg, _, err := cli.PopFirstArgument(args)
if err != nil {
return
}
Expand Down
5 changes: 5 additions & 0 deletions cli/selfsign/selfsign.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package selfsign

import (
"encoding/json"
"errors"
"fmt"
"os"
"time"
Expand Down Expand Up @@ -49,6 +50,10 @@ func selfSignMain(args []string, c cli.Config) (err error) {
return
}

if len(args) > 0 {
return errors.New("too many arguments are provided, please check with usage")
}

csrFileBytes, err := cli.ReadStdin(csrFile)
if err != nil {
return
Expand Down
4 changes: 4 additions & 0 deletions cli/sign/sign.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package sign

import (
"encoding/json"
"errors"
"io/ioutil"

"github.com/cloudflare/cfssl/certdb/dbconf"
Expand Down Expand Up @@ -121,6 +122,9 @@ func signerMain(args []string, c cli.Config) (err error) {
if err != nil {
return
}
if len(args) > 0 {
return errors.New("too many arguments are provided, please check with usage")
}

var subjectJSON []byte
subjectJSON, err = ioutil.ReadFile(subjectFile)
Expand Down
4 changes: 4 additions & 0 deletions config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -447,6 +447,10 @@ func TestNeedLocalSigner(t *testing.T) {
if localConfig.Signing.NeedsRemoteSigner() != false {
t.Fatal("incorrect NeedsRemoteSigner().")
}

if err != nil {
t.Fatal(err)
}
}

func TestOverrideRemotes(t *testing.T) {
Expand Down
8 changes: 4 additions & 4 deletions csr/csr_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -634,7 +634,7 @@ func TestReGenerate(t *testing.T) {
KeyRequest: &BasicKeyRequest{"ecdsa", 256},
}

csr, key, err := ParseRequest(req)
_, key, err := ParseRequest(req)
if err != nil {
t.Fatalf("%v", err)
}
Expand All @@ -644,7 +644,7 @@ func TestReGenerate(t *testing.T) {
t.Fatalf("%v", err)
}

csr, err = Generate(priv, req)
csr, err := Generate(priv, req)
if err != nil {
t.Fatalf("%v", err)
}
Expand Down Expand Up @@ -677,7 +677,7 @@ func TestBadReGenerate(t *testing.T) {
KeyRequest: &BasicKeyRequest{"ecdsa", 256},
}

csr, key, err := ParseRequest(req)
_, key, err := ParseRequest(req)
if err != nil {
t.Fatalf("%v", err)
}
Expand All @@ -687,7 +687,7 @@ func TestBadReGenerate(t *testing.T) {
t.Fatalf("%v", err)
}

csr, err = Generate(priv, req)
csr, err := Generate(priv, req)
if err != nil {
t.Fatalf("%v", err)
}
Expand Down
4 changes: 2 additions & 2 deletions helpers/helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -428,12 +428,12 @@ const clientCertFile = "testdata/ca.pem"
const clientKeyFile = "testdata/ca_key.pem"

func TestClientCertParams(t *testing.T) {
cert, err := LoadClientCertificate(testCertFile, testPrivateRSAKey)
_, err := LoadClientCertificate(testCertFile, testPrivateRSAKey)
if err == nil {
t.Fatal("Unmatched cert/key should generate error")
}

cert, err = LoadClientCertificate("", "")
cert, err := LoadClientCertificate("", "")
if err != nil || cert != nil {
t.Fatal("Certificate atempted to loaded with missing key and cert")
}
Expand Down
5 changes: 5 additions & 0 deletions helpers/testsuite/testing_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -284,10 +284,15 @@ func SignCertificate(request csr.CertificateRequest, signerCert, signerKey []byt
tempCSRFile,
)
CLIOutput, err = command.CombinedOutput()
if err != nil {
return nil, nil, fmt.Errorf("%v - CLI output: %s", err, string(CLIOutput))
}

err = checkCLIOutput(CLIOutput)
if err != nil {
return nil, nil, fmt.Errorf("%v - CLI output: %s", err, string(CLIOutput))
}

encodedCert, err = cleanCLIOutput(CLIOutput, "cert")
if err != nil {
return nil, nil, err
Expand Down
18 changes: 9 additions & 9 deletions initca/initca_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,12 @@ import (
)

var validKeyParams = []csr.BasicKeyRequest{
{"rsa", 2048},
{"rsa", 3072},
{"rsa", 4096},
{"ecdsa", 256},
{"ecdsa", 384},
{"ecdsa", 521},
{A: "rsa", S: 2048},
{A: "rsa", S: 3072},
{A: "rsa", S: 4096},
{A: "ecdsa", S: 256},
{A: "ecdsa", S: 384},
{A: "ecdsa", S: 521},
}

var validCAConfigs = []csr.CAConfig{
Expand Down Expand Up @@ -54,10 +54,10 @@ var testECDSACAKeyFile = "testdata/5min-ecdsa-key.pem"

var invalidCryptoParams = []csr.BasicKeyRequest{
// Weak Key
{"rsa", 1024},
{A: "rsa", S: 1024},
// Bad param
{"rsaCrypto", 2048},
{"ecdsa", 2000},
{A: "rsaCrypto", S: 2048},
{A: "ecdsa", S: 2000},
}

func TestInitCA(t *testing.T) {
Expand Down
13 changes: 13 additions & 0 deletions multiroot/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,19 @@ func parsePrivateKeySpec(spec string, cfg map[string]string) (crypto.Signer, err
return nil, err
}

log.Debug("attempting to load PEM-encoded private key")
priv, err = helpers.ParsePrivateKeyPEM(in)
if err != nil {
log.Debug("file is not a PEM-encoded private key")
log.Debug("attempting to load DER-encoded private key")
priv, err = derhelpers.ParsePrivateKeyDER(in)
if err != nil {
return nil, err
}
}

log.Debug("loaded private key")

return priv, nil
default:
return nil, ErrUnsupportedScheme
Expand Down
4 changes: 4 additions & 0 deletions ocsp/ocsp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,10 @@ func TestSign(t *testing.T) {
}

sMismatch, err := NewSignerFromFile(wrongServerCertFile, otherCertFile, wrongServerKeyFile, dur)
if err != nil {
t.Fatal("NewSigner failed:", err)
}

_, err = sMismatch.Sign(req)
if err == nil {
t.Fatal("Signed a certificate from the wrong issuer")
Expand Down
3 changes: 3 additions & 0 deletions signer/signer.go
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,9 @@ func ComputeSKI(template *x509.Certificate) ([]byte, error) {
// and SKI.
func FillTemplate(template *x509.Certificate, defaultProfile, profile *config.SigningProfile) error {
ski, err := ComputeSKI(template)
if err != nil {
return err
}

var (
eku []x509.ExtKeyUsage
Expand Down
2 changes: 1 addition & 1 deletion test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ do
done

if ! command -v staticcheck > /dev/null ; then
go get github.com/dominikh/go-staticcheck/cmd/staticcheck
go get honnef.co/go/tools/cmd/staticcheck
fi

for package in $PACKAGES
Expand Down

0 comments on commit 4bfe535

Please sign in to comment.