fix: improve code style

Signed-off-by: Junjie Gao <[email protected]>

revocation/crl/errors.go

@@ -17,3 +17,6 @@ import "errors"
 
 // ErrCacheMiss is returned when a cache miss occurs.
 var ErrCacheMiss = errors.New("cache miss")
+
+// errDeltaCRLNotFound is returned when a delta CRL is not found.
+var errDeltaCRLNotFound = errors.New("delta CRL not found")

revocation/crl/fetcher.go

@@ -127,8 +127,8 @@ func (f *HTTPFetcher) fetch(ctx context.Context, url string) (*Bundle, error) {
 	}
 
 	// fetch delta CRL from base CRL extension
-	deltaCRL, err := f.processDeltaCRL(&base.Extensions)
-	if err != nil {
+	deltaCRL, err := f.fetchDeltaCRL(&base.Extensions)
+	if err != nil && !errors.Is(err, errDeltaCRLNotFound) {
 		return nil, err
 	}
 
@@ -138,8 +138,10 @@ func (f *HTTPFetcher) fetch(ctx context.Context, url string) (*Bundle, error) {
 	}, nil
 }
 
-// processDeltaCRL processes the delta CRL from the given extensions of base CRL.
-func (f *HTTPFetcher) processDeltaCRL(extensions *[]pkix.Extension) (*x509.RevocationList, error) {
+// fetchDeltaCRL fetches the delta CRL from the given extensions of base CRL.
+//
+// It returns errDeltaCRLNotFound if the delta CRL is not found.
+func (f *HTTPFetcher) fetchDeltaCRL(extensions *[]pkix.Extension) (*x509.RevocationList, error) {
 	for _, ext := range *extensions {
 		if ext.Id.Equal(oidFreshestCRL) {
 			// RFC 5280, 4.2.1.15
@@ -151,12 +153,15 @@ func (f *HTTPFetcher) processDeltaCRL(extensions *[]pkix.Extension) (*x509.Revoc
 				return nil, fmt.Errorf("failed to parse Freshest CRL extension: %w", err)
 			}
 			if len(urls) == 0 {
-				return nil, nil
+				return nil, errDeltaCRLNotFound
 			}
 
-			var lastError error
-			var deltaCRL *x509.RevocationList
+			var (
+				lastError error
+				deltaCRL  *x509.RevocationList
+			)
 			for _, cdpURL := range urls {
+				// RFC 5280, 5.2.6
 				// Delta CRLs from the base CRL have the same scope as the base
 				// CRL, so the URLs are for redundancy and should be tried in
 				// order until one succeeds.
@@ -169,14 +174,14 @@ func (f *HTTPFetcher) processDeltaCRL(extensions *[]pkix.Extension) (*x509.Revoc
 			return nil, lastError
 		}
 	}
-	return nil, nil
+	return nil, errDeltaCRLNotFound
 }
 
 // parseCRLDistributionPoint parses the CRL extension and returns the CRL URLs
 //
 // value is the raw value of the CRL distribution point extension
 func parseCRLDistributionPoint(value []byte) ([]string, error) {
-	var cdp []string
+	var urls []string
 	// borrowed from crypto/x509: https://cs.opensource.google/go/go/+/refs/tags/go1.23.4:src/crypto/x509/parser.go;l=700-743
 	//
 	// RFC 5280, 4.2.1.13
@@ -219,10 +224,10 @@ func parseCRLDistributionPoint(value []byte) ([]string, error) {
 			if !dpNameDER.ReadASN1(&uri, cryptobyte_asn1.Tag(6).ContextSpecific()) {
 				return nil, errors.New("x509: invalid CRL distribution point")
 			}
-			cdp = append(cdp, string(uri))
+			urls = append(urls, string(uri))
 		}
 	}
-	return cdp, nil
+	return urls, nil
 }
 
 func fetchCRL(ctx context.Context, crlURL string, client *http.Client) (*x509.RevocationList, error) {

revocation/crl/fetcher_test.go

@@ -380,7 +380,7 @@ func TestParseFreshestCRL(t *testing.T) {
 	})
 }
 
-func TestProcessDeltaCRL(t *testing.T) {
+func TestFetchDeltaCRL(t *testing.T) {
 	loadExtentsion := func(certPath string) *[]pkix.Extension {
 		certData, err := os.ReadFile(certPath)
 		if err != nil {
@@ -415,7 +415,7 @@ func TestProcessDeltaCRL(t *testing.T) {
 	t.Run("parse freshest CRL failed", func(t *testing.T) {
 		certPath := "testdata/certificateWithIncompleteFreshestCRL.cer"
 		extensions := loadExtentsion(certPath)
-		_, err := fetcher.processDeltaCRL(extensions)
+		_, err := fetcher.fetchDeltaCRL(extensions)
 		expectedErrorMsg := "failed to parse Freshest CRL extension: x509: invalid CRL distribution point"
 		if err == nil || err.Error() != expectedErrorMsg {
 			t.Fatalf("expected error %q, got %v", expectedErrorMsg, err)
@@ -425,19 +425,17 @@ func TestProcessDeltaCRL(t *testing.T) {
 	t.Run("zero freshest CRL URL", func(t *testing.T) {
 		certPath := "testdata/certificateWithZeroDeltaCRLURL.cer"
 		extensions := loadExtentsion(certPath)
-		deltaCRL, err := fetcher.processDeltaCRL(extensions)
-		if err != nil {
-			t.Fatalf("failed to process delta CRL: %v", err)
-		}
-		if deltaCRL != nil {
-			t.Fatalf("expected nil delta CRL, got %v", deltaCRL)
+		_, err := fetcher.fetchDeltaCRL(extensions)
+		expectedErr := errDeltaCRLNotFound
+		if err == nil || !errors.Is(err, expectedErr) {
+			t.Fatalf("expected error %v, got %v", expectedErr, err)
 		}
 	})
 
 	t.Run("one freshest CRL URL", func(t *testing.T) {
 		certPath := "testdata/certificateWithDeltaCRL.cer"
 		extensions := loadExtentsion(certPath)
-		deltaCRL, err := fetcher.processDeltaCRL(extensions)
+		deltaCRL, err := fetcher.fetchDeltaCRL(extensions)
 		if err != nil {
 			t.Fatalf("failed to process delta CRL: %v", err)
 		}
@@ -455,7 +453,7 @@ func TestProcessDeltaCRL(t *testing.T) {
 		}
 		certPath := "testdata/certificateWith2DeltaCRL.cer"
 		extensions := loadExtentsion(certPath)
-		_, err = fetcherWithError.processDeltaCRL(extensions)
+		_, err = fetcherWithError.fetchDeltaCRL(extensions)
 		expectedErrorMsg := "request failed"
 		if err == nil || !strings.Contains(err.Error(), expectedErrorMsg) {
 			t.Fatalf("expected error %q, got %v", expectedErrorMsg, err)
@@ -465,7 +463,7 @@ func TestProcessDeltaCRL(t *testing.T) {
 	t.Run("process delta crl from certificate extension failed", func(t *testing.T) {
 		certPath := "testdata/certificateWithIncompleteFreshestCRL.cer"
 		extensions := loadExtentsion(certPath)
-		_, err := fetcher.processDeltaCRL(extensions)
+		_, err := fetcher.fetchDeltaCRL(extensions)
 		expectedErrorMsg := "failed to parse Freshest CRL extension: x509: invalid CRL distribution point"
 		if err == nil || err.Error() != expectedErrorMsg {
 			t.Fatalf("expected error %q, got %v", expectedErrorMsg, err)

revocation/internal/crl/crl.go

@@ -107,23 +107,19 @@ func CertCheckStatus(ctx context.Context, cert, issuer *x509.Certificate, opts C
 	// point with one CRL URI, which will be cached, so checking all the URIs is
 	// not a performance issue.
 	for _, crlURL = range cert.CRLDistributionPoints {
-		var (
-			bundle *crl.Bundle
-			err    error
-		)
-		bundle, err = opts.Fetcher.Fetch(ctx, crlURL)
+		bundle, err := opts.Fetcher.Fetch(ctx, crlURL)
 		if err != nil {
 			lastErr = fmt.Errorf("failed to download CRL from %s: %w", crlURL, err)
 			break
 		}
 
 		if hasFreshestCRLInCertificate && bundle.DeltaCRL == nil {
-			// deltaCRL URL in cert        deltaCRL URL in baseCRL support it?
-			// --------------------------- ----------------------- ---------
-			//        True                      True                 Yes
-			//        True                      False                No
-			//        False                     True                 Yes
-			//        False                     False                Yes
+			// deltaCRL URL in cert        deltaCRL URL in baseCRL  support it?
+			// --------------------------- ----------------------- -------------
+			//        True                      True                    Yes
+			//        True                      False                   No
+			//        False                     True                    Yes
+			//        False                     False                   Yes
 			//
 			// if only the certificate has the freshest CRL, the bundle.DeltaCRL
 			// should be nil. We don't support this case now because the delta

revocation/internal/crl/crl_test.go

@@ -332,12 +332,6 @@ func TestCertCheckStatus(t *testing.T) {
 	})
 }
 
-type fetcherMock struct{}
-
-func (f *fetcherMock) Fetch(ctx context.Context, url string) (*crlutils.Bundle, error) {
-	return nil, fmt.Errorf("fetch error")
-}
-
 func TestValidate(t *testing.T) {
 	t.Run("expired CRL", func(t *testing.T) {
 		chain := testhelper.GetRevokableRSAChainWithRevocations(1, false, true)