Overhauling error handling of the different APIs as part of #24

- More error generalization
- Fixed PwnedPasswords API errors
- Added SHA1 hash validation with corresponding error
- More tests for error handling

hibp.go

@@ -16,9 +16,12 @@ import (
 // Version represents the version of this package
 const Version = "1.0.5"
 
-// BaseURL is the base URL for the majority of API calls
+// BaseURL is the base URL for the majority of API endpoints
 const BaseURL = "https://haveibeenpwned.com/api/v3"
 
+// PasswdBaseURL is the base URL for the pwned passwords API endpoints
+const PasswdBaseURL = "https://api.pwnedpasswords.com"
+
 // DefaultUserAgent defines the default UA string for the HTTP client
 // Currently the URL in the UA string is comment out, as there is a bug in the HIBP API
 // not allowing multiple slashes
@@ -37,6 +40,17 @@ var (
 
 	// ErrNonPositiveResponse should be returned if a HTTP request failed with a non HTTP-200 status
 	ErrNonPositiveResponse = errors.New("non HTTP-200 response for HTTP request")
+
+	// ErrPrefixLengthMismatch should be used if a given prefix does not match the
+	// expected length
+	ErrPrefixLengthMismatch = errors.New("password hash prefix must be 5 characters long")
+
+	// ErrSHA1LengthMismatch should be used if a given SHA1 checksum does not match the
+	// expected length
+	ErrSHA1LengthMismatch = errors.New("SHA1 hash size needs to be 160 bits")
+
+	// ErrSHA1Invalid should be used if a given string does not represent a valid SHA1 hash
+	ErrSHA1Invalid = errors.New("not a valid SHA1 hash")
 )
 
 // Client is the HIBP client object

password.go

@@ -3,22 +3,13 @@ package hibp
 import (
 	"bufio"
 	"crypto/sha1"
+	"encoding/hex"
 	"fmt"
 	"net/http"
 	"strconv"
 	"strings"
 )
 
-const (
-	// ErrPrefixLengthMismatch should be used if a given prefix does not match the
-	// expected length
-	ErrPrefixLengthMismatch = "password hash prefix must be 5 characters long"
-
-	// ErrSHA1LengthMismatch should be used if a given SHA1 checksum does not match the
-	// expected length
-	ErrSHA1LengthMismatch = "SHA1 hash size needs to be 160 bits"
-)
-
 // PwnedPassAPI is a HIBP Pwned Passwords API client
 type PwnedPassAPI struct {
 	hibp *Client // References back to the parent HIBP client
@@ -46,7 +37,7 @@ func (p *PwnedPassAPI) CheckPassword(pw string) (*Match, *http.Response, error)
 // CheckSHA1 checks the Pwned Passwords database against a given SHA1 checksum of a password string
 func (p *PwnedPassAPI) CheckSHA1(h string) (*Match, *http.Response, error) {
 	if len(h) != 40 {
-		return nil, nil, fmt.Errorf(ErrSHA1LengthMismatch)
+		return nil, nil, ErrSHA1LengthMismatch
 	}
 
 	pwMatches, hr, err := p.ListHashesPrefix(h[:5])
@@ -79,7 +70,11 @@ func (p *PwnedPassAPI) ListHashesPassword(pw string) ([]Match, *http.Response, e
 // contain junk data
 func (p *PwnedPassAPI) ListHashesSHA1(h string) ([]Match, *http.Response, error) {
 	if len(h) != 40 {
-		return nil, nil, fmt.Errorf(ErrSHA1LengthMismatch)
+		return nil, nil, ErrSHA1LengthMismatch
+	}
+	dst := make([]byte, hex.DecodedLen(len(h)))
+	if _, err := hex.Decode(dst, []byte(h)); err != nil {
+		return nil, nil, ErrSHA1Invalid
 	}
 	return p.ListHashesPrefix(h[:5])
 }
@@ -91,28 +86,29 @@ func (p *PwnedPassAPI) ListHashesSHA1(h string) ([]Match, *http.Response, error)
 // contain junk data
 func (p *PwnedPassAPI) ListHashesPrefix(pf string) ([]Match, *http.Response, error) {
 	if len(pf) != 5 {
-		return nil, nil, fmt.Errorf(ErrPrefixLengthMismatch)
+		return nil, nil, ErrPrefixLengthMismatch
 	}
-	hreq, err := p.hibp.HTTPReq(http.MethodGet, fmt.Sprintf("https://api.pwnedpasswords.com/range/%s", pf),
+
-		nil)
+	au := fmt.Sprintf("%s/range/%s", PasswdBaseURL, pf)
+	hreq, err := p.hibp.HTTPReq(http.MethodGet, au, nil)
 	if err != nil {
 		return nil, nil, err
 	}
 	hr, err := p.hibp.hc.Do(hreq)
 	if err != nil {
-		return nil, nil, err
+		return nil, hr, err
-	}
-	if hr.StatusCode != 200 {
-		return nil, hr, fmt.Errorf("API responded with non HTTP-200: %s", hr.Status)
 	}
 	defer func() {
 		_ = hr.Body.Close()
 	}()
+	if hr.StatusCode != 200 {
+		return nil, hr, fmt.Errorf("HTTP %s: %w", hr.Status, ErrNonPositiveResponse)
+	}
 
-	var pwMatches []Match
+	var pm []Match
-	scanObj := bufio.NewScanner(hr.Body)
+	so := bufio.NewScanner(hr.Body)
-	for scanObj.Scan() {
+	for so.Scan() {
-		hp := strings.SplitN(scanObj.Text(), ":", 2)
+		hp := strings.SplitN(so.Text(), ":", 2)
 		fh := fmt.Sprintf("%s%s", strings.ToLower(pf), strings.ToLower(hp[0]))
 		hc, err := strconv.ParseInt(hp[1], 10, 64)
 		if err != nil {
@@ -121,15 +117,15 @@ func (p *PwnedPassAPI) ListHashesPrefix(pf string) ([]Match, *http.Response, err
 		if hc == 0 {
 			continue
 		}
-		pwMatches = append(pwMatches, Match{
+		pm = append(pm, Match{
 			Hash:  fh,
 			Count: hc,
 		})
 	}
 
-	if err := scanObj.Err(); err != nil {
+	if err := so.Err(); err != nil {
-		return nil, nil, err
+		return nil, hr, err
 	}
 
-	return pwMatches, hr, nil
+	return pm, hr, nil
 }

password_test.go

@@ -1,6 +1,7 @@
 package hibp
 
 import (
+	"errors"
 	"fmt"
 	"testing"
 )
@@ -119,6 +120,74 @@ func TestPwnedPassAPI_ListHashesPrefix(t *testing.T) {
 	}
 }
 
+// TestPwnedPassAPI_ListHashesPrefix_Errors tests the ListHashesPrefix method's errors
+func TestPwnedPassAPI_ListHashesPrefix_Errors(t *testing.T) {
+	hc := New()
+
+	// Empty prefix
+	t.Run("empty prefix", func(t *testing.T) {
+		_, _, err := hc.PwnedPassAPI.ListHashesPrefix("")
+		if err == nil {
+			t.Errorf("ListHashesPrefix with empty prefix should fail but didn't")
+			return
+		}
+		if !errors.Is(err, ErrPrefixLengthMismatch) {
+			t.Errorf("ListHashesPrefix with empty prefix should return ErrPrefixLengthMismatch error but didn't")
+			return
+		}
+	})
+
+	// Too long prefix
+	t.Run("too long prefix", func(t *testing.T) {
+		_, _, err := hc.PwnedPassAPI.ListHashesPrefix("abcdefg12345")
+		if err == nil {
+			t.Errorf("ListHashesPrefix with too long prefix should fail but didn't")
+			return
+		}
+		if !errors.Is(err, ErrPrefixLengthMismatch) {
+			t.Errorf("ListHashesPrefix with too long prefix should return ErrPrefixLengthMismatch error but didn't")
+		}
+	})
+}
+
+// TestPwnedPassAPI_ListHashesSHA1_Errors tests the ListHashesSHA1 method's errors
+func TestPwnedPassAPI_ListHashesSHA1_Errors(t *testing.T) {
+	hc := New()
+
+	// Empty hash
+	t.Run("empty hash", func(t *testing.T) {
+		_, _, err := hc.PwnedPassAPI.ListHashesSHA1("")
+		if err == nil {
+			t.Errorf("ListHashesSHA1 with empty hash should fail but didn't")
+		}
+		if !errors.Is(err, ErrSHA1LengthMismatch) {
+			t.Errorf("ListHashesSHA1 with empty hash should return ErrSHA1LengthMismatch error but didn't")
+		}
+	})
+
+	// Too long hash
+	t.Run("too long hash", func(t *testing.T) {
+		_, _, err := hc.PwnedPassAPI.ListHashesSHA1("FF36DC7D3284A39991ADA90CAF20D1E3C0DADEFAB")
+		if err == nil {
+			t.Errorf("ListHashesSHA1 with too long hash should fail but didn't")
+		}
+		if !errors.Is(err, ErrSHA1LengthMismatch) {
+			t.Errorf("ListHashesSHA1 with too long hash should return ErrSHA1LengthMismatch error but didn't")
+		}
+	})
+
+	// Invalid hash
+	t.Run("invalid hash", func(t *testing.T) {
+		_, _, err := hc.PwnedPassAPI.ListHashesSHA1("FF36DC7D3284A39991ADA90CAF20D1E3C0DADEFZ")
+		if err == nil {
+			t.Errorf("ListHashesSHA1 with invalid hash should fail but didn't")
+		}
+		if !errors.Is(err, ErrSHA1Invalid) {
+			t.Errorf("ListHashesSHA1 with invalid hash should return ErrSHA1Invalid error but didn't")
+		}
+	})
+}
+
 // TestPwnedPassApi_ListHashesSHA1 tests the PwnedPassAPI.ListHashesSHA1 metethod
 func TestPwnedPassAPI_ListHashesSHA1(t *testing.T) {
 	hc := New()