Skip to content

Commit 4fe0a5a

Browse files
authored
Remove extraneous top-level struct args to IsAnyNilOrZero (#8651)
We frequently call `core.IsAnyNilOrZero(obj, obj.field)`, to check both that the object (usually a gRPC request) is not nil, and that the required field on the object is not nil. This is pointless for two reasons: 1) gRPC guarantees that the request object will never be nil, so checking the top-level object is wasted effort. 2) If the object were nil, then attempting to pass `obj.field` as the second argument to IsAnyNilOrZero will result in a nil pointer dereference panic before that function even gets to evaluate its first argument. This can be seen here: https://go.dev/play/p/duQK2ZbnxqK Remove these extraneous first arguments, and operate under the assumption that the request object itself is never nil.
1 parent 0b73047 commit 4fe0a5a

8 files changed

Lines changed: 15 additions & 15 deletions

File tree

ca/ca.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,7 @@ func (ca *certificateAuthorityImpl) IssueCertificate(ctx context.Context, req *c
219219
// Step 1: Locally process the gRPC request and its embedded CSR to extract
220220
// the relevant information, like the pubkey and SANs. Also generate
221221
// some metadata from scratch, such as the serial and validity period.
222-
if core.IsAnyNilOrZero(req, req.RegistrationID, req.OrderID, req.CertProfileName, req.Csr) {
222+
if core.IsAnyNilOrZero(req.RegistrationID, req.OrderID, req.CertProfileName, req.Csr) {
223223
return nil, berrors.InternalServerError("Incomplete issue certificate request")
224224
}
225225

ra/ra.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2096,7 +2096,7 @@ func (ra *RegistrationAuthorityImpl) DeactivateRegistration(ctx context.Context,
20962096
func (ra *RegistrationAuthorityImpl) DeactivateAuthorization(ctx context.Context, req *corepb.Authorization) (*emptypb.Empty, error) {
20972097
ident := identifier.FromProto(req.Identifier)
20982098

2099-
if core.IsAnyNilOrZero(req, req.Id, ident, req.Status, req.RegistrationID) {
2099+
if core.IsAnyNilOrZero(req.Id, ident, req.Status, req.RegistrationID) {
21002100
return nil, errIncompleteGRPCRequest
21012101
}
21022102
authzID, err := strconv.ParseInt(req.Id, 10, 64)
@@ -2413,7 +2413,7 @@ func (ra *RegistrationAuthorityImpl) UnpauseAccount(ctx context.Context, request
24132413
}
24142414

24152415
func (ra *RegistrationAuthorityImpl) GetAuthorization(ctx context.Context, req *rapb.GetAuthorizationRequest) (*corepb.Authorization, error) {
2416-
if core.IsAnyNilOrZero(req, req.Id) {
2416+
if core.IsAnyNilOrZero(req.Id) {
24172417
return nil, errIncompleteGRPCRequest
24182418
}
24192419

@@ -2437,7 +2437,7 @@ func (ra *RegistrationAuthorityImpl) GetAuthorization(ctx context.Context, req *
24372437

24382438
// AddRateLimitOverride is a pass-through to the SA's AddRateLimitOverride method.
24392439
func (ra *RegistrationAuthorityImpl) AddRateLimitOverride(ctx context.Context, req *rapb.AddRateLimitOverrideRequest) (*rapb.AddRateLimitOverrideResponse, error) {
2440-
if core.IsAnyNilOrZero(req, req.Override, req.Override.LimitEnum, req.Override.BucketKey, req.Override.Count, req.Override.Burst, req.Override.Period, req.Override.Comment) {
2440+
if core.IsAnyNilOrZero(req.Override, req.Override.LimitEnum, req.Override.BucketKey, req.Override.Count, req.Override.Burst, req.Override.Period, req.Override.Comment) {
24412441
return nil, errIncompleteGRPCRequest
24422442
}
24432443

sa/sa.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1428,7 +1428,7 @@ func overrideLowerThanExisting(new *sapb.RateLimitOverride, existing overrideMod
14281428
// re-enabled. The current status is returned in the Enabled field of the
14291429
// response. To re-enable an override, use EnableRateLimitOverride.
14301430
func (ssa *SQLStorageAuthority) AddRateLimitOverride(ctx context.Context, req *sapb.AddRateLimitOverrideRequest) (*sapb.AddRateLimitOverrideResponse, error) {
1431-
if core.IsAnyNilOrZero(req, req.Override, req.Override.LimitEnum, req.Override.BucketKey, req.Override.Count, req.Override.Burst, req.Override.Period, req.Override.Comment) {
1431+
if core.IsAnyNilOrZero(req.Override, req.Override.LimitEnum, req.Override.BucketKey, req.Override.Count, req.Override.Burst, req.Override.Period, req.Override.Comment) {
14321432
return nil, errIncompleteRequest
14331433
}
14341434

@@ -1615,7 +1615,7 @@ func (ssa *SQLStorageAuthority) updateRateLimitOverride(
16151615
// not exist, a NotFoundError is returned. If the override exists but is already
16161616
// disabled, this is a no-op.
16171617
func (ssa *SQLStorageAuthority) DisableRateLimitOverride(ctx context.Context, req *sapb.DisableRateLimitOverrideRequest) (*emptypb.Empty, error) {
1618-
if core.IsAnyNilOrZero(req, req.LimitEnum, req.BucketKey) {
1618+
if core.IsAnyNilOrZero(req.LimitEnum, req.BucketKey) {
16191619
return nil, errIncompleteRequest
16201620
}
16211621
return ssa.setRateLimitOverride(ctx, req.LimitEnum, req.BucketKey, false)
@@ -1625,7 +1625,7 @@ func (ssa *SQLStorageAuthority) DisableRateLimitOverride(ctx context.Context, re
16251625
// not exist, a NotFoundError is returned. If the override exists but is already
16261626
// enabled, this is a no-op.
16271627
func (ssa *SQLStorageAuthority) EnableRateLimitOverride(ctx context.Context, req *sapb.EnableRateLimitOverrideRequest) (*emptypb.Empty, error) {
1628-
if core.IsAnyNilOrZero(req, req.LimitEnum, req.BucketKey) {
1628+
if core.IsAnyNilOrZero(req.LimitEnum, req.BucketKey) {
16291629
return nil, errIncompleteRequest
16301630
}
16311631
return ssa.setRateLimitOverride(ctx, req.LimitEnum, req.BucketKey, true)

sa/saro.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -685,7 +685,7 @@ func (ssa *SQLStorageAuthorityRO) CountInvalidAuthorizations2(ctx context.Contex
685685
func (ssa *SQLStorageAuthorityRO) GetValidAuthorizations2(ctx context.Context, req *sapb.GetValidAuthorizationsRequest) (*sapb.Authorizations, error) {
686686
idents := identifier.FromProtoSlice(req.Identifiers)
687687

688-
if core.IsAnyNilOrZero(req, req.RegistrationID, idents, req.ValidUntil) {
688+
if core.IsAnyNilOrZero(req.RegistrationID, idents, req.ValidUntil) {
689689
return nil, errIncompleteRequest
690690
}
691691

@@ -1128,7 +1128,7 @@ func (ssa *SQLStorageAuthorityRO) GetPausedIdentifiers(ctx context.Context, req
11281128
// GetRateLimitOverride retrieves a rate limit override for the given bucket key
11291129
// and limit. If no override is found, a NotFound error is returned.
11301130
func (ssa *SQLStorageAuthorityRO) GetRateLimitOverride(ctx context.Context, req *sapb.GetRateLimitOverrideRequest) (*sapb.RateLimitOverrideResponse, error) {
1131-
if core.IsAnyNilOrZero(req, req.LimitEnum, req.BucketKey) {
1131+
if core.IsAnyNilOrZero(req.LimitEnum, req.BucketKey) {
11321132
return nil, errIncompleteRequest
11331133
}
11341134

salesforce/exporter.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ func NewExporterImpl(client SalesforceClient, cache *EmailCache, perDayLimit flo
105105
// SendContacts enqueues the provided email addresses. If the queue cannot
106106
// accommodate the new emails, an ErrQueueFull is returned.
107107
func (impl *ExporterImpl) SendContacts(ctx context.Context, req *salesforcepb.SendContactsRequest) (*emptypb.Empty, error) {
108-
if core.IsAnyNilOrZero(req, req.Emails) {
108+
if core.IsAnyNilOrZero(req.Emails) {
109109
return nil, berrors.InternalServerError("Incomplete gRPC request message")
110110
}
111111

@@ -127,7 +127,7 @@ func (impl *ExporterImpl) SendContacts(ctx context.Context, req *salesforcepb.Se
127127
// provided details. Any retries are handled internally by the SalesforceClient.
128128
// The following fields are required: Origin, Subject, ContactEmail.
129129
func (impl *ExporterImpl) SendCase(ctx context.Context, req *salesforcepb.SendCaseRequest) (*emptypb.Empty, error) {
130-
if core.IsAnyNilOrZero(req, req.Origin, req.Subject, req.ContactEmail) {
130+
if core.IsAnyNilOrZero(req.Origin, req.Subject, req.ContactEmail) {
131131
return nil, berrors.InternalServerError("incomplete gRPC request message")
132132
}
133133

va/caa.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ func (va *ValidationAuthorityImpl) checkCAA(
126126
ctx context.Context,
127127
ident identifier.ACMEIdentifier,
128128
params *caaParams) error {
129-
if core.IsAnyNilOrZero(params, params.validationMethod, params.accountURIID) {
129+
if core.IsAnyNilOrZero(params.validationMethod, params.accountURIID) {
130130
return errors.New("expected validationMethod or accountURIID not provided to checkCAA")
131131
}
132132

va/va.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -664,7 +664,7 @@ type validationLogEvent struct {
664664
// implements the DCV portion of Multi-Perspective Issuance Corroboration as
665665
// defined in BRs Sections 3.2.2.9 and 5.4.1.
666666
func (va *ValidationAuthorityImpl) DoDCV(ctx context.Context, req *vapb.PerformValidationRequest) (*vapb.ValidationResult, error) {
667-
if core.IsAnyNilOrZero(req, req.Identifier, req.Challenge, req.Authz, req.Authz.RegID, req.ExpectedKeyAuthorization) {
667+
if core.IsAnyNilOrZero(req.Identifier, req.Challenge, req.Authz, req.Authz.RegID, req.ExpectedKeyAuthorization) {
668668
return nil, berrors.InternalServerError("Incomplete validation request")
669669
}
670670

wfe2/wfe.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1348,7 +1348,7 @@ func (wfe *WebFrontEndImpl) postChallenge(
13481348
Authz: authzPB,
13491349
ChallengeIndex: int64(challengeIndex),
13501350
})
1351-
if err != nil || core.IsAnyNilOrZero(authzPB, authzPB.Id, authzPB.Identifier, authzPB.Status, authzPB.Expires) {
1351+
if err != nil || core.IsAnyNilOrZero(authzPB.Id, authzPB.Identifier, authzPB.Status, authzPB.Expires) {
13521352
wfe.sendError(response, logEvent, web.ProblemDetailsForError(err, "Unable to update challenge"), err)
13531353
return
13541354
}
@@ -2450,7 +2450,7 @@ func (wfe *WebFrontEndImpl) NewOrder(
24502450
ReplacesSerial: replacesSerial,
24512451
})
24522452

2453-
if err != nil || core.IsAnyNilOrZero(order, order.Id, order.RegistrationID, order.Identifiers, order.Created, order.Expires) {
2453+
if err != nil || core.IsAnyNilOrZero(order.Id, order.RegistrationID, order.Identifiers, order.Created, order.Expires) {
24542454
wfe.sendError(response, logEvent, web.ProblemDetailsForError(err, "Error creating new order"), err)
24552455
return
24562456
}

0 commit comments

Comments
 (0)