Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pass authzModel by value, not reference #4690

Merged
merged 3 commits into from
Feb 29, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions sa/model.go
Original file line number Diff line number Diff line change
Expand Up @@ -564,7 +564,7 @@ func authzPBToModel(authz *corepb.Authorization) (*authzModel, error) {

// populateAttemptedFields takes a challenge and populates it with the validation fields status,
// validation records, and error (the latter only if the validation failed) from a authzModel.
func populateAttemptedFields(am *authzModel, challenge *corepb.Challenge) error {
func populateAttemptedFields(am authzModel, challenge *corepb.Challenge) error {
if len(am.ValidationError) != 0 {
// If the error is non-empty the challenge must be invalid.
status := string(core.StatusInvalid)
Expand Down Expand Up @@ -604,7 +604,7 @@ func populateAttemptedFields(am *authzModel, challenge *corepb.Challenge) error
return nil
}

func modelToAuthzPB(am *authzModel) (*corepb.Authorization, error) {
func modelToAuthzPB(am authzModel) (*corepb.Authorization, error) {
expires := am.Expires.UTC().UnixNano()
id := fmt.Sprintf("%d", am.ID)
status := uintToStatus[am.Status]
Expand Down
6 changes: 3 additions & 3 deletions sa/model_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ func TestV2AuthzModel(t *testing.T) {
model, err := authzPBToModel(authzPB)
test.AssertNotError(t, err, "authzPBToModel failed")

authzPBOut, err := modelToAuthzPB(model)
authzPBOut, err := modelToAuthzPB(*model)
test.AssertNotError(t, err, "modelToAuthzPB failed")
test.AssertDeepEquals(t, authzPB.Challenges, authzPBOut.Challenges)

Expand All @@ -114,7 +114,7 @@ func TestV2AuthzModel(t *testing.T) {
model, err = authzPBToModel(authzPB)
test.AssertNotError(t, err, "authzPBToModel failed")

authzPBOut, err = modelToAuthzPB(model)
authzPBOut, err = modelToAuthzPB(*model)
test.AssertNotError(t, err, "modelToAuthzPB failed")
test.AssertDeepEquals(t, authzPB.Challenges, authzPBOut.Challenges)

Expand Down Expand Up @@ -195,7 +195,7 @@ func TestPopulateAttemptedFieldsBadJSON(t *testing.T) {
}
for _, tc := range testCases {
t.Run(tc.Name, func(t *testing.T) {
err := populateAttemptedFields(tc.Model, &corepb.Challenge{})
err := populateAttemptedFields(*tc.Model, &corepb.Challenge{})
test.AssertError(t, err, "expected error from populateAttemptedFields")
badJSONErr, ok := err.(errBadJSON)
test.AssertEquals(t, ok, true)
Expand Down
6 changes: 3 additions & 3 deletions sa/sa.go
Original file line number Diff line number Diff line change
Expand Up @@ -1401,7 +1401,7 @@ func (ssa *SQLStorageAuthority) GetAuthorization2(ctx context.Context, id *sapb.
if obj == nil {
return nil, berrors.NotFoundError("authorization %d not found", *id.Id)
}
return modelToAuthzPB(obj.(*authzModel))
return modelToAuthzPB(*(obj.(*authzModel)))
}

// authzModelMapToPB converts a mapping of domain name to authzModels into a
Expand All @@ -1411,7 +1411,7 @@ func authzModelMapToPB(m map[string]authzModel) (*sapb.Authorizations, error) {
for k, v := range m {
// Make a copy of k because it will be reassigned with each loop.
kCopy := k
authzPB, err := modelToAuthzPB(&v)
authzPB, err := modelToAuthzPB(v)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -1644,7 +1644,7 @@ func (ssa *SQLStorageAuthority) GetPendingAuthorization2(ctx context.Context, re
}
return nil, err
}
return modelToAuthzPB(&am)
return modelToAuthzPB(am)
}

// CountPendingAuthorizations2 returns the number of pending, unexpired authorizations
Expand Down
10 changes: 8 additions & 2 deletions test/v1_integration.py
Original file line number Diff line number Diff line change
Expand Up @@ -438,7 +438,11 @@ def caa_recheck_setup():
# Issue a certificate with the clock set back, and save the authzs to check
# later that they are valid (200). They should however require rechecking for
# CAA purposes.
_, authzs = auth_and_issue([random_domain()], client=caa_recheck_client)
numNames = 10
# Generate numNames subdomains of a random domain
base_domain = random_domain()
domains = [ "{0}.{1}".format(str(n),base_domain) for n in range(numNames) ]
_, authzs = auth_and_issue(domains, client=caa_recheck_client)
for a in authzs:
caa_recheck_authzs.append(a)

Expand All @@ -457,7 +461,9 @@ def test_recheck_caa():
response.status_code))
domain = a.body.identifier.value
domains.append(domain)
challSrv.add_caa_issue(domain, ";")

# Set a forbidding CAA record on just one domain
challSrv.add_caa_issue(domains[3], ";")

# Request issuance for the previously-issued domain name, which should
# now be denied due to CAA.
Expand Down