Skip to content

Commit

Permalink
Fix a bad error return in RA (letsencrypt#3061)
Browse files Browse the repository at this point in the history
RA.NewRegistration checked for an error return from
SA.NewRegistration, but failed to properly propagate
the error. It was just setting the err variable and falling
through to the successful return case. This fixes that
issue and adds a unittest.
  • Loading branch information
jsha authored and Roland Bracewell Shoemaker committed Sep 8, 2017
1 parent 8afec60 commit 3b6a5ff
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 3 deletions.
4 changes: 1 addition & 3 deletions ra/ra.go
Original file line number Diff line number Diff line change
Expand Up @@ -359,9 +359,7 @@ func (ra *RegistrationAuthorityImpl) NewRegistration(ctx context.Context, init c
// Store the authorization object, then return it
reg, err := ra.SA.NewRegistration(ctx, reg)
if err != nil {
// berrors.InternalServerError since the user-data was validated before being
// passed to the SA.
err = berrors.InternalServerError(err.Error())
return core.Registration{}, err
}

ra.stats.Inc("NewRegistrations", 1)
Expand Down
24 changes: 24 additions & 0 deletions ra/ra_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -394,6 +394,30 @@ func TestNewRegistration(t *testing.T) {
test.Assert(t, core.KeyDigestEquals(reg.Key, AccountKeyB), "Retrieved registration differed.")
}

type mockSAFailsNewRegistration struct {
mocks.StorageAuthority
}

func (ms *mockSAFailsNewRegistration) NewRegistration(ctx context.Context, reg core.Registration) (core.Registration, error) {
return core.Registration{}, fmt.Errorf("too bad")
}

func TestNewRegistrationSAFailure(t *testing.T) {
_, _, ra, _, cleanUp := initAuthorities(t)
defer cleanUp()
ra.SA = &mockSAFailsNewRegistration{}
input := core.Registration{
Contact: &[]string{"mailto:[email protected]"},
Key: &AccountKeyB,
InitialIP: net.ParseIP("7.6.6.5"),
}

result, err := ra.NewRegistration(ctx, input)
if err == nil {
t.Fatalf("NewRegistration should have failed when SA.NewRegistration failed %#v", result.Key)
}
}

func TestNewRegistrationNoFieldOverwrite(t *testing.T) {
_, _, ra, _, cleanUp := initAuthorities(t)
defer cleanUp()
Expand Down

0 comments on commit 3b6a5ff

Please sign in to comment.