diff --git a/cmd/contact-exporter/main_test.go b/cmd/contact-exporter/main_test.go index c5eb89906..e352f96e2 100644 --- a/cmd/contact-exporter/main_test.go +++ b/cmd/contact-exporter/main_test.go @@ -124,10 +124,10 @@ type testCtx struct { } func (c testCtx) addRegistrations(t *testing.T) { - emailA, _ := core.ParseAcmeURL("mailto:" + emailARaw) - emailB, _ := core.ParseAcmeURL("mailto:" + emailBRaw) - emailC, _ := core.ParseAcmeURL("mailto:" + emailCRaw) - tel, _ := core.ParseAcmeURL("tel:" + telNum) + emailA := "mailto:" + emailARaw + emailB := "mailto:" + emailBRaw + emailC := "mailto:" + emailCRaw + tel := "tel:" + telNum // Every registration needs a unique JOSE key jsonKeyA := []byte(`{ @@ -167,7 +167,7 @@ func (c testCtx) addRegistrations(t *testing.T) { // Regs A through C have `mailto:` contact ACME URL's regA = core.Registration{ ID: 1, - Contact: &[]*core.AcmeURL{ + Contact: &[]string{ emailA, }, Key: keyA, @@ -175,7 +175,7 @@ func (c testCtx) addRegistrations(t *testing.T) { } regB = core.Registration{ ID: 2, - Contact: &[]*core.AcmeURL{ + Contact: &[]string{ emailB, }, Key: keyB, @@ -183,7 +183,7 @@ func (c testCtx) addRegistrations(t *testing.T) { } regC = core.Registration{ ID: 3, - Contact: &[]*core.AcmeURL{ + Contact: &[]string{ emailC, }, Key: keyC, @@ -192,7 +192,7 @@ func (c testCtx) addRegistrations(t *testing.T) { // Reg D has a `tel:` contact ACME URL regD = core.Registration{ ID: 4, - Contact: &[]*core.AcmeURL{ + Contact: &[]string{ tel, }, Key: keyD, diff --git a/cmd/expiration-mailer/main.go b/cmd/expiration-mailer/main.go index f4969e700..3a46d2f5f 100644 --- a/cmd/expiration-mailer/main.go +++ b/cmd/expiration-mailer/main.go @@ -9,6 +9,7 @@ import ( "io/ioutil" "math" netmail "net/mail" + "net/url" "os" "sort" "strings" @@ -55,7 +56,7 @@ type mailer struct { clk clock.Clock } -func (m *mailer) sendNags(contacts []*core.AcmeURL, certs []*x509.Certificate) error { +func (m *mailer) sendNags(contacts []string, certs []*x509.Certificate) error { if len(contacts) == 0 { return nil } @@ -64,8 +65,14 @@ func (m *mailer) sendNags(contacts []*core.AcmeURL, certs []*x509.Certificate) e } emails := []string{} for _, contact := range contacts { - if contact.Scheme == "mailto" { - emails = append(emails, contact.Opaque) + parsed, err := url.Parse(contact) + if err != nil { + m.log.AuditErr(fmt.Sprintf("parsing contact email %s: %s", + contact, err)) + continue + } + if parsed.Scheme == "mailto" { + emails = append(emails, parsed.Opaque) } } if len(emails) == 0 { diff --git a/cmd/expiration-mailer/main_test.go b/cmd/expiration-mailer/main_test.go index af22f442d..818ab5ca4 100644 --- a/cmd/expiration-mailer/main_test.go +++ b/cmd/expiration-mailer/main_test.go @@ -78,9 +78,9 @@ const emailARaw = "rolandshoemaker@gmail.com" const emailBRaw = "test@gmail.com" var ( - emailA, _ = core.ParseAcmeURL("mailto:" + emailARaw) - emailB, _ = core.ParseAcmeURL("mailto:" + emailBRaw) - jsonKeyA = []byte(`{ + emailA = "mailto:" + emailARaw + emailB = "mailto:" + emailBRaw + jsonKeyA = []byte(`{ "kty":"RSA", "n":"0vx7agoebGcQSuuPiLJXZptN9nndrQmbXEps2aiAFbWhM78LhWx4cbbfAAtVT86zwu1RK7aPFFxuhDR1L6tSoc_BJECPebWKRXjBZCiFV4n3oknjhMstn64tZ_2W-5JsGY4Hc5n9yBXArwl93lqt7_RN5w6Cf0h4QyQ5v-65YGjQR0_FDW2QvzqY368QQMicAtaSqzs8KJZgnYb9c7d0zgdAZHzu6qMQvRL5hajrn1n91CbOpbISD08qNLyrdkt-bFTWhAI4vMQFh6WeZu0fM4lFd2NcRwr3XPksINHaQ-G_xBniIqbw0Ls1jF44-csFCur-kEgU8awapJzKnqDKgw", "e":"AQAB" @@ -124,7 +124,7 @@ func TestSendNags(t *testing.T) { DNSNames: []string{"example.com"}, } - err := m.sendNags([]*core.AcmeURL{emailA}, []*x509.Certificate{cert}) + err := m.sendNags([]string{emailA}, []*x509.Certificate{cert}) test.AssertNotError(t, err, "Failed to send warning messages") test.AssertEquals(t, len(mc.Messages), 1) test.AssertEquals(t, mocks.MailerMessage{ @@ -134,7 +134,7 @@ func TestSendNags(t *testing.T) { }, mc.Messages[0]) mc.Clear() - err = m.sendNags([]*core.AcmeURL{emailA, emailB}, []*x509.Certificate{cert}) + err = m.sendNags([]string{emailA, emailB}, []*x509.Certificate{cert}) test.AssertNotError(t, err, "Failed to send warning messages") test.AssertEquals(t, len(mc.Messages), 2) test.AssertEquals(t, mocks.MailerMessage{ @@ -149,7 +149,7 @@ func TestSendNags(t *testing.T) { }, mc.Messages[1]) mc.Clear() - err = m.sendNags([]*core.AcmeURL{}, []*x509.Certificate{cert}) + err = m.sendNags([]string{}, []*x509.Certificate{cert}) test.AssertNotError(t, err, "Not an error to pass no email contacts") test.AssertEquals(t, len(mc.Messages), 0) @@ -253,7 +253,7 @@ func addExpiringCerts(t *testing.T, ctx *testCtx) []core.Certificate { test.AssertNotError(t, err, "Failed to unmarshal public JWK") regA := core.Registration{ ID: 1, - Contact: &[]*core.AcmeURL{ + Contact: &[]string{ emailA, }, Key: keyA, @@ -261,7 +261,7 @@ func addExpiringCerts(t *testing.T, ctx *testCtx) []core.Certificate { } regB := core.Registration{ ID: 2, - Contact: &[]*core.AcmeURL{ + Contact: &[]string{ emailB, }, Key: keyB, @@ -269,7 +269,7 @@ func addExpiringCerts(t *testing.T, ctx *testCtx) []core.Certificate { } regC := core.Registration{ ID: 3, - Contact: &[]*core.AcmeURL{ + Contact: &[]string{ emailB, }, Key: keyC, @@ -595,7 +595,7 @@ func TestLifetimeOfACert(t *testing.T) { regA := core.Registration{ ID: 1, - Contact: &[]*core.AcmeURL{ + Contact: &[]string{ emailA, }, Key: keyA, @@ -696,11 +696,11 @@ func TestDontFindRevokedCert(t *testing.T) { err := json.Unmarshal(jsonKeyA, &keyA) test.AssertNotError(t, err, "Failed to unmarshal public JWK") - emailA, _ := core.ParseAcmeURL("mailto:one@mail.com") + emailA := "mailto:one@mail.com" regA := core.Registration{ ID: 1, - Contact: &[]*core.AcmeURL{ + Contact: &[]string{ emailA, }, Key: keyA, @@ -756,7 +756,7 @@ func TestDedupOnRegistration(t *testing.T) { regA := core.Registration{ ID: 1, - Contact: &[]*core.AcmeURL{ + Contact: &[]string{ emailA, }, Key: keyA, diff --git a/cmd/expiration-mailer/send_test.go b/cmd/expiration-mailer/send_test.go index 8174b368a..ba5aef834 100644 --- a/cmd/expiration-mailer/send_test.go +++ b/cmd/expiration-mailer/send_test.go @@ -8,24 +8,15 @@ import ( "testing" "time" - "github.com/letsencrypt/boulder/core" "github.com/letsencrypt/boulder/mocks" "github.com/letsencrypt/boulder/test" ) var ( - email1 = mustParseAcmeURL("mailto:one@example.com") - email2 = mustParseAcmeURL("mailto:two@example.com") + email1 = "mailto:one@example.com" + email2 = "mailto:two@example.com" ) -func mustParseAcmeURL(acmeURL string) *core.AcmeURL { - c, err := core.ParseAcmeURL(acmeURL) - if err != nil { - panic(fmt.Sprintf("unable to parse as AcmeURL %#v: %s", acmeURL, err)) - } - return c -} - func TestSendEarliestCertInfo(t *testing.T) { expiresIn := 24 * time.Hour ctx := setup(t, []time.Duration{expiresIn}) @@ -42,7 +33,7 @@ func TestSendEarliestCertInfo(t *testing.T) { serial2, ) - err := ctx.m.sendNags([]*core.AcmeURL{email1, email2}, []*x509.Certificate{rawCertA, rawCertB}) + err := ctx.m.sendNags([]string{email1, email2}, []*x509.Certificate{rawCertA, rawCertB}) if err != nil { t.Fatal(err) } diff --git a/core/core_test.go b/core/core_test.go index 5571d75e9..d03ee1c49 100644 --- a/core/core_test.go +++ b/core/core_test.go @@ -3,7 +3,6 @@ package core import ( "encoding/base64" "encoding/json" - "fmt" "testing" "github.com/letsencrypt/boulder/test" @@ -131,38 +130,3 @@ func TestFingerprint(t *testing.T) { t.Errorf("Incorrect SHA-256 fingerprint: %v", digest) } } - -func TestURL(t *testing.T) { - scheme := "https" - host := "example.com" - path := "/acme/test" - query := "foo" - jsonURL := fmt.Sprintf(`{"URL":"%s://%s%s?%s"}`, scheme, host, path, query) - badJSON := `{"URL":666}` - - url := struct{ URL *AcmeURL }{URL: &AcmeURL{}} - err := json.Unmarshal([]byte(jsonURL), &url) - if err != nil { - t.Errorf("Error in json unmarshal: %v", err) - } - if url.URL.Scheme != scheme || url.URL.Host != host || - url.URL.Path != path || url.URL.RawQuery != query { - t.Errorf("Improper URL contents: %v", url.URL) - } - if s := url.URL.PathSegments(); len(s) != 2 { - t.Errorf("Path segments failed to parse properly: %v", s) - } - - err = json.Unmarshal([]byte(badJSON), &url) - if err == nil { - t.Errorf("Failed to catch bad JSON") - } - - marshaledURL, err := json.Marshal(url) - if err != nil { - t.Errorf("Error in json marshal: %v", err) - } - if string(marshaledURL) != jsonURL { - t.Errorf("Expected marshaled url %#v, got %#v", jsonURL, string(marshaledURL)) - } -} diff --git a/core/objects.go b/core/objects.go index 37757e8b7..e237aec25 100644 --- a/core/objects.go +++ b/core/objects.go @@ -147,7 +147,7 @@ type Registration struct { Key jose.JsonWebKey `json:"key"` // Contact URIs - Contact *[]*AcmeURL `json:"contact,omitempty"` + Contact *[]string `json:"contact,omitempty"` // Agreement with terms of service Agreement string `json:"agreement,omitempty"` diff --git a/core/util.go b/core/util.go index 5a2fb82c3..93c67af5d 100644 --- a/core/util.go +++ b/core/util.go @@ -7,7 +7,6 @@ import ( "crypto/x509" "encoding/base64" "encoding/hex" - "encoding/json" "encoding/pem" "errors" "expvar" @@ -17,7 +16,6 @@ import ( "math/big" mrand "math/rand" "net/http" - "net/url" "regexp" "sort" "strings" @@ -212,52 +210,6 @@ func KeyDigestEquals(j, k crypto.PublicKey) bool { return digestJ == digestK } -// AcmeURL is a URL that automatically marshal/unmarshal to JSON strings -type AcmeURL url.URL - -// ParseAcmeURL is just a wrapper around url.Parse that returns an *AcmeURL -func ParseAcmeURL(s string) (*AcmeURL, error) { - u, err := url.Parse(s) - if err != nil { - return nil, err - } - return (*AcmeURL)(u), nil -} - -func (u *AcmeURL) String() string { - uu := (*url.URL)(u) - return uu.String() -} - -// PathSegments splits an AcmeURL into segments on the '/' characters -func (u *AcmeURL) PathSegments() (segments []string) { - segments = strings.Split(u.Path, "/") - if len(segments) > 0 && len(segments[0]) == 0 { - segments = segments[1:] - } - return -} - -// MarshalJSON encodes an AcmeURL for transfer -func (u *AcmeURL) MarshalJSON() ([]byte, error) { - return json.Marshal(u.String()) -} - -// UnmarshalJSON decodes an AcmeURL from transfer -func (u *AcmeURL) UnmarshalJSON(data []byte) error { - var str string - if err := json.Unmarshal(data, &str); err != nil { - return err - } - - uu, err := url.Parse(str) - if err != nil { - return err - } - *u = AcmeURL(*uu) - return nil -} - // SerialToString converts a certificate serial number (big.Int) to a String // consistently. func SerialToString(serial *big.Int) string { diff --git a/core/util_test.go b/core/util_test.go index eb2002890..79a2d283a 100644 --- a/core/util_test.go +++ b/core/util_test.go @@ -5,7 +5,6 @@ import ( "fmt" "math" "math/big" - "net/url" "reflect" "sort" "testing" @@ -106,27 +105,12 @@ func TestKeyDigestEquals(t *testing.T) { test.Assert(t, !KeyDigestEquals(struct{}{}, struct{}{}), "Unknown key types should not match anything") } -func TestAcmeURL(t *testing.T) { - s := "http://example.invalid" - u, _ := url.Parse(s) - a := (*AcmeURL)(u) - test.AssertEquals(t, s, a.String()) -} - func TestUniqueLowerNames(t *testing.T) { u := UniqueLowerNames([]string{"foobar.com", "fooBAR.com", "baz.com", "foobar.com", "bar.com", "bar.com", "a.com"}) sort.Strings(u) test.AssertDeepEquals(t, []string{"a.com", "bar.com", "baz.com", "foobar.com"}, u) } -func TestUnmarshalAcmeURL(t *testing.T) { - var u AcmeURL - err := u.UnmarshalJSON([]byte(`":"`)) - if err == nil { - t.Errorf("Expected error parsing ':', but got nil err.") - } -} - func TestProblemDetailsFromError(t *testing.T) { testCases := []struct { err error diff --git a/mocks/mocks.go b/mocks/mocks.go index dc5b6a914..491ce2065 100644 --- a/mocks/mocks.go +++ b/mocks/mocks.go @@ -114,8 +114,7 @@ func (sa *StorageAuthority) GetRegistrationByKey(_ context.Context, jwk jose.Jso panic(err) } - contactURL, _ := core.ParseAcmeURL("mailto:person@mail.com") - contacts := []*core.AcmeURL{contactURL} + contacts := []string{"mailto:person@mail.com"} if core.KeyDigestEquals(jwk, test1KeyPublic) { return core.Registration{ID: 1, Key: jwk, Agreement: agreementURL, Contact: &contacts}, nil diff --git a/ra/ra.go b/ra/ra.go index 71fc112a1..c20ef4252 100644 --- a/ra/ra.go +++ b/ra/ra.go @@ -7,6 +7,7 @@ import ( "fmt" "net" "net/mail" + "net/url" "reflect" "sort" "strings" @@ -283,7 +284,7 @@ func (ra *RegistrationAuthorityImpl) NewRegistration(ctx context.Context, init c return } -func (ra *RegistrationAuthorityImpl) validateContacts(ctx context.Context, contacts *[]*core.AcmeURL) error { +func (ra *RegistrationAuthorityImpl) validateContacts(ctx context.Context, contacts *[]string) error { if contacts == nil || len(*contacts) == 0 { return nil // Nothing to validate } @@ -293,20 +294,24 @@ func (ra *RegistrationAuthorityImpl) validateContacts(ctx context.Context, conta } for _, contact := range *contacts { - if contact == nil { + if contact == "" { + return core.MalformedRequestError("Empty contact") + } + parsed, err := url.Parse(contact) + if err != nil { return core.MalformedRequestError("Invalid contact") } - if contact.Scheme != "mailto" { - return core.MalformedRequestError(fmt.Sprintf("Contact method %s is not supported", contact.Scheme)) + if parsed.Scheme != "mailto" { + return core.MalformedRequestError(fmt.Sprintf("Contact method %s is not supported", parsed.Scheme)) } - if !core.IsASCII(contact.String()) { + if !core.IsASCII(contact) { return core.MalformedRequestError( - fmt.Sprintf("Contact email [%s] contains non-ASCII characters", contact.String())) + fmt.Sprintf("Contact email [%s] contains non-ASCII characters", contact)) } start := ra.clk.Now() ra.stats.Inc("RA.ValidateEmail.Calls", 1, 1.0) - problem := validateEmail(ctx, contact.Opaque, ra.DNSResolver) + problem := validateEmail(ctx, parsed.Opaque, ra.DNSResolver) ra.stats.TimingDuration("RA.ValidateEmail.Latency", ra.clk.Now().Sub(start), 1.0) if problem != nil { ra.stats.Inc("RA.ValidateEmail.Errors", 1, 1.0) @@ -804,29 +809,17 @@ func contactsEqual(r *core.Registration, other core.Registration) bool { } // If there is an existing contact slice and it has the same length as the - // new contact slice we need to look at each AcmeURL to determine if there - // is a change being made - // - // TODO(cpu): After #1966 is merged and the `AcmeURL`s are just `String`s we - // should use `sort.Strings` here to ensure a consistent - // comparison. + // new contact slice we need to look at each contact to determine if there + // is a change being made. Use `sort.Strings` here to ensure a consistent + // comparison a := *other.Contact b := *r.Contact + sort.Strings(a) + sort.Strings(b) for i := 0; i < len(a); i++ { - // In order to call .String() we need to make sure the AcmeURL pointers - // aren't nil. In practice `a[i]` should never be nil but `b[i]` might be - if a[i] == nil || b[i] == nil { - // We return false, allowing MergeUpdate to use `other` to overwrite. The - // nil value in the Contact slice will be turned into a user-facing error - // when the RA validates the contacts post-merge. - return false - } - // If the contact's string representation differs at any index they aren't // equal - contactA := (*a[i]).String() - contactB := (*b[i]).String() - if contactA != contactB { + if a[i] != b[i] { return false } } diff --git a/ra/ra_test.go b/ra/ra_test.go index 3ecf98220..1c762f2e0 100644 --- a/ra/ra_test.go +++ b/ra/ra_test.go @@ -284,31 +284,31 @@ func TestValidateContacts(t *testing.T) { _, _, ra, _, cleanUp := initAuthorities(t) defer cleanUp() - ansible, _ := core.ParseAcmeURL("ansible:earth.sol.milkyway.laniakea/letsencrypt") - validEmail, _ := core.ParseAcmeURL("mailto:admin@email.com") - otherValidEmail, _ := core.ParseAcmeURL("mailto:other-admin@email.com") - malformedEmail, _ := core.ParseAcmeURL("mailto:admin.com") - nonASCII, _ := core.ParseAcmeURL("mailto:señor@email.com") + ansible := "ansible:earth.sol.milkyway.laniakea/letsencrypt" + validEmail := "mailto:admin@email.com" + otherValidEmail := "mailto:other-admin@email.com" + malformedEmail := "mailto:admin.com" + nonASCII := "mailto:señor@email.com" - err := ra.validateContacts(context.Background(), &[]*core.AcmeURL{}) + err := ra.validateContacts(context.Background(), &[]string{}) test.AssertNotError(t, err, "No Contacts") - err = ra.validateContacts(context.Background(), &[]*core.AcmeURL{validEmail, otherValidEmail}) + err = ra.validateContacts(context.Background(), &[]string{validEmail, otherValidEmail}) test.AssertError(t, err, "Too Many Contacts") - err = ra.validateContacts(context.Background(), &[]*core.AcmeURL{validEmail}) + err = ra.validateContacts(context.Background(), &[]string{validEmail}) test.AssertNotError(t, err, "Valid Email") - err = ra.validateContacts(context.Background(), &[]*core.AcmeURL{malformedEmail}) + err = ra.validateContacts(context.Background(), &[]string{malformedEmail}) test.AssertError(t, err, "Malformed Email") - err = ra.validateContacts(context.Background(), &[]*core.AcmeURL{ansible}) + err = ra.validateContacts(context.Background(), &[]string{ansible}) test.AssertError(t, err, "Unknown scheme") - err = ra.validateContacts(context.Background(), &[]*core.AcmeURL{nil}) - test.AssertError(t, err, "Nil AcmeURL") + err = ra.validateContacts(context.Background(), &[]string{""}) + test.AssertError(t, err, "Empty URL") - err = ra.validateContacts(context.Background(), &[]*core.AcmeURL{nonASCII}) + err = ra.validateContacts(context.Background(), &[]string{nonASCII}) test.AssertError(t, err, "Non ASCII email") } @@ -350,9 +350,9 @@ func TestValidateEmail(t *testing.T) { func TestNewRegistration(t *testing.T) { _, sa, ra, _, cleanUp := initAuthorities(t) defer cleanUp() - mailto, _ := core.ParseAcmeURL("mailto:foo@letsencrypt.org") + mailto := "mailto:foo@letsencrypt.org" input := core.Registration{ - Contact: &[]*core.AcmeURL{mailto}, + Contact: &[]string{mailto}, Key: AccountKeyB, InitialIP: net.ParseIP("7.6.6.5"), } @@ -364,8 +364,7 @@ func TestNewRegistration(t *testing.T) { test.Assert(t, core.KeyDigestEquals(result.Key, AccountKeyB), "Key didn't match") test.Assert(t, len(*result.Contact) == 1, "Wrong number of contacts") - test.Assert(t, mailto.String() == (*result.Contact)[0].String(), - "Contact didn't match") + test.Assert(t, mailto == (*result.Contact)[0], "Contact didn't match") test.Assert(t, result.Agreement == "", "Agreement didn't default empty") reg, err := sa.GetRegistration(ctx, result.ID) @@ -376,11 +375,11 @@ func TestNewRegistration(t *testing.T) { func TestNewRegistrationNoFieldOverwrite(t *testing.T) { _, _, ra, _, cleanUp := initAuthorities(t) defer cleanUp() - mailto, _ := core.ParseAcmeURL("mailto:foo@letsencrypt.org") + mailto := "mailto:foo@letsencrypt.org" input := core.Registration{ ID: 23, Key: AccountKeyC, - Contact: &[]*core.AcmeURL{mailto}, + Contact: &[]string{mailto}, Agreement: "I agreed", InitialIP: net.ParseIP("5.0.5.0"), } @@ -405,9 +404,9 @@ func TestNewRegistrationNoFieldOverwrite(t *testing.T) { func TestNewRegistrationBadKey(t *testing.T) { _, _, ra, _, cleanUp := initAuthorities(t) defer cleanUp() - mailto, _ := core.ParseAcmeURL("mailto:foo@letsencrypt.org") + mailto := "mailto:foo@letsencrypt.org" input := core.Registration{ - Contact: &[]*core.AcmeURL{mailto}, + Contact: &[]string{mailto}, Key: ShortKey, } @@ -426,12 +425,12 @@ func (sa NoUpdateSA) UpdateRegistration(_ context.Context, _ core.Registration) func TestUpdateRegistrationSame(t *testing.T) { _, _, ra, _, cleanUp := initAuthorities(t) defer cleanUp() - mailto, _ := core.ParseAcmeURL("mailto:foo@letsencrypt.org") + mailto := "mailto:foo@letsencrypt.org" // Make a new registration with AccountKeyC and a Contact input := core.Registration{ Key: AccountKeyC, - Contact: &[]*core.AcmeURL{mailto}, + Contact: &[]string{mailto}, Agreement: "I agreed", InitialIP: net.ParseIP("5.0.5.0"), } @@ -446,7 +445,7 @@ func TestUpdateRegistrationSame(t *testing.T) { updateSame := core.Registration{ ID: id, Key: AccountKeyC, - Contact: &[]*core.AcmeURL{mailto}, + Contact: &[]string{mailto}, Agreement: "I agreed", } @@ -1088,15 +1087,15 @@ func TestCheckCertificatesPerNameLimit(t *testing.T) { } func TestRegistrationUpdate(t *testing.T) { - oldURL, _ := core.ParseAcmeURL("http://old.invalid") - newURL, _ := core.ParseAcmeURL("http://new.invalid") + oldURL := "http://old.invalid" + newURL := "http://new.invalid" reg := core.Registration{ ID: 1, - Contact: &[]*core.AcmeURL{oldURL}, + Contact: &[]string{oldURL}, Agreement: "", } update := core.Registration{ - Contact: &[]*core.AcmeURL{newURL}, + Contact: &[]string{newURL}, Agreement: "totally!", } @@ -1105,21 +1104,21 @@ func TestRegistrationUpdate(t *testing.T) { test.Assert(t, len(*reg.Contact) == 1 && (*reg.Contact)[0] == (*update.Contact)[0], "Contact was not updated %v != %v") test.Assert(t, reg.Agreement == update.Agreement, "Agreement was not updated") - // Make sure that a `MergeUpdate` call with a nil entry doesn't produce an + // Make sure that a `MergeUpdate` call with an empty string doesn't produce an // error and results in a change to the base reg. - nilUpdate := core.Registration{ - Contact: &[]*core.AcmeURL{nil}, + emptyUpdate := core.Registration{ + Contact: &[]string{""}, Agreement: "totally!", } - changed = mergeUpdate(®, nilUpdate) + changed = mergeUpdate(®, emptyUpdate) test.AssertEquals(t, changed, true) } func TestRegistrationContactUpdate(t *testing.T) { - contactURL, _ := core.ParseAcmeURL("mailto://example@example.com") + contactURL := "mailto://example@example.com" fullReg := core.Registration{ ID: 1, - Contact: &[]*core.AcmeURL{contactURL}, + Contact: &[]string{contactURL}, Agreement: "totally!", } @@ -1165,7 +1164,7 @@ func TestRegistrationContactUpdate(t *testing.T) { changed = mergeUpdate(®, contactSameUpdate) test.AssertEquals(t, changed, false) test.Assert(t, len(*reg.Contact) == 1, "len(Contact) was updated unexpectedly") - test.Assert(t, (*reg.Contact)[0].String() == "mailto://example@example.com", "Contact was changed unexpectedly") + test.Assert(t, (*reg.Contact)[0] == "mailto://example@example.com", "Contact was changed unexpectedly") } // A mockSAWithFQDNSet is a mock StorageAuthority that supports diff --git a/sa/model.go b/sa/model.go index a8d7ffd72..b83459565 100644 --- a/sa/model.go +++ b/sa/model.go @@ -23,11 +23,11 @@ type issuedNameModel struct { // regModel is the description of a core.Registration in the database. type regModel struct { - ID int64 `db:"id"` - Key []byte `db:"jwk"` - KeySHA256 string `db:"jwk_sha256"` - Contact []*core.AcmeURL `db:"contact"` - Agreement string `db:"agreement"` + ID int64 `db:"id"` + Key []byte `db:"jwk"` + KeySHA256 string `db:"jwk_sha256"` + Contact []string `db:"contact"` + Agreement string `db:"agreement"` // InitialIP is stored as sixteen binary bytes, regardless of whether it // represents a v4 or v6 IP address. InitialIP []byte `db:"initialIp"` @@ -82,7 +82,7 @@ func registrationToModel(r *core.Registration) (*regModel, error) { return nil, fmt.Errorf("initialIP was nil") } if r.Contact == nil { - r.Contact = &[]*core.AcmeURL{} + r.Contact = &[]string{} } rm := ®Model{ ID: r.ID, @@ -103,12 +103,12 @@ func modelToRegistration(rm *regModel) (core.Registration, error) { err = fmt.Errorf("unable to unmarshal JsonWebKey in db: %s", err) return core.Registration{}, err } - var contact *[]*core.AcmeURL + var contact *[]string // Contact can be nil when the DB contains the literal string "null". We // prefer to represent this in memory as a pointer to an empty slice rather // than a nil pointer. if rm.Contact == nil { - contact = &[]*core.AcmeURL{} + contact = &[]string{} } else { contact = &rm.Contact } diff --git a/sa/model_test.go b/sa/model_test.go index a47d06f47..8a94c4162 100644 --- a/sa/model_test.go +++ b/sa/model_test.go @@ -2,8 +2,6 @@ package sa import ( "testing" - - "github.com/letsencrypt/boulder/core" ) func TestModelToRegistrationNilContact(t *testing.T) { @@ -25,7 +23,7 @@ func TestModelToRegistrationNilContact(t *testing.T) { func TestModelToRegistrationNonNilContact(t *testing.T) { reg, err := modelToRegistration(®Model{ Key: []byte(`{"kty":"RSA","n":"AQAB","e":"AQAB"}`), - Contact: []*core.AcmeURL{}, + Contact: []string{}, }) if err != nil { t.Errorf("Got error from modelToRegistration: %s", err) diff --git a/sa/sa_test.go b/sa/sa_test.go index 5b4e4a35a..b68d7893d 100644 --- a/sa/sa_test.go +++ b/sa/sa_test.go @@ -11,7 +11,6 @@ import ( "io/ioutil" "math/big" "net" - "net/url" "reflect" "testing" "time" @@ -66,11 +65,8 @@ func TestAddRegistration(t *testing.T) { jwk := satest.GoodJWK() - contact, err := core.ParseAcmeURL("mailto:foo@example.com") - if err != nil { - t.Fatalf("unable to parse contact link: %s", err) - } - contacts := &[]*core.AcmeURL{contact} + contact := "mailto:foo@example.com" + contacts := &[]string{contact} reg, err := sa.NewRegistration(ctx, core.Registration{ Key: jwk, Contact: contacts, @@ -97,12 +93,10 @@ func TestAddRegistration(t *testing.T) { test.AssertEquals(t, dbReg.ID, expectedReg.ID) test.Assert(t, core.KeyDigestEquals(dbReg.Key, expectedReg.Key), "Stored key != expected") - u, _ := core.ParseAcmeURL("test.com") - newReg := core.Registration{ ID: reg.ID, Key: jwk, - Contact: &[]*core.AcmeURL{u}, + Contact: &[]string{"test.com"}, InitialIP: net.ParseIP("72.72.72.72"), Agreement: "yes", } @@ -589,26 +583,23 @@ func TestCountRegistrationsByIP(t *testing.T) { sa, fc, cleanUp := initSA(t) defer cleanUp() - contact := core.AcmeURL(url.URL{ - Scheme: "mailto", - Opaque: "foo@example.com", - }) + contact := "mailto:foo@example.com" _, err := sa.NewRegistration(ctx, core.Registration{ Key: jose.JsonWebKey{Key: &rsa.PublicKey{N: big.NewInt(1), E: 1}}, - Contact: &[]*core.AcmeURL{&contact}, + Contact: &[]string{contact}, InitialIP: net.ParseIP("43.34.43.34"), }) test.AssertNotError(t, err, "Couldn't insert registration") _, err = sa.NewRegistration(ctx, core.Registration{ Key: jose.JsonWebKey{Key: &rsa.PublicKey{N: big.NewInt(2), E: 1}}, - Contact: &[]*core.AcmeURL{&contact}, + Contact: &[]string{contact}, InitialIP: net.ParseIP("2001:cdba:1234:5678:9101:1121:3257:9652"), }) test.AssertNotError(t, err, "Couldn't insert registration") _, err = sa.NewRegistration(ctx, core.Registration{ Key: jose.JsonWebKey{Key: &rsa.PublicKey{N: big.NewInt(3), E: 1}}, - Contact: &[]*core.AcmeURL{&contact}, + Contact: &[]string{contact}, InitialIP: net.ParseIP("2001:cdba:1234:5678:9101:1121:3257:9653"), }) test.AssertNotError(t, err, "Couldn't insert registration") diff --git a/sa/satest/satest.go b/sa/satest/satest.go index 7cf0e5de6..baa135fc8 100644 --- a/sa/satest/satest.go +++ b/sa/satest/satest.go @@ -40,11 +40,8 @@ func GoodJWK() jose.JsonWebKey { // CreateWorkingRegistration, this and CreateWorkingRegistration can // be pushed back into the SA tests proper. func CreateWorkingRegistration(t *testing.T, sa core.StorageAdder) core.Registration { - contact, err := core.ParseAcmeURL("mailto:foo@example.com") - if err != nil { - t.Fatalf("unable to parse contact link: %s", err) - } - contacts := &[]*core.AcmeURL{contact} + contact := "mailto:foo@example.com" + contacts := &[]string{contact} reg, err := sa.NewRegistration(context.Background(), core.Registration{ Key: GoodJWK(), Contact: contacts, diff --git a/sa/type-converter.go b/sa/type-converter.go index a7afcaf41..a33a7c219 100644 --- a/sa/type-converter.go +++ b/sa/type-converter.go @@ -17,7 +17,7 @@ type BoulderTypeConverter struct{} // ToDb converts a Boulder object to one suitable for the DB representation. func (tc BoulderTypeConverter) ToDb(val interface{}) (interface{}, error) { switch t := val.(type) { - case core.AcmeIdentifier, []core.Challenge, []*core.AcmeURL, [][]int: + case core.AcmeIdentifier, []core.Challenge, []string, [][]int: jsonBytes, err := json.Marshal(t) if err != nil { return nil, err @@ -41,7 +41,7 @@ func (tc BoulderTypeConverter) ToDb(val interface{}) (interface{}, error) { // FromDb converts a DB representation back into a Boulder object. func (tc BoulderTypeConverter) FromDb(target interface{}) (gorp.CustomScanner, bool) { switch target.(type) { - case *core.AcmeIdentifier, *[]core.Challenge, *[]*core.AcmeURL, *[][]int: + case *core.AcmeIdentifier, *[]core.Challenge, *[]string, *[][]int: binder := func(holder, target interface{}) error { s, ok := holder.(*string) if !ok { diff --git a/sa/type-converter_test.go b/sa/type-converter_test.go index 887a1e0e2..fd58f0de2 100644 --- a/sa/type-converter_test.go +++ b/sa/type-converter_test.go @@ -103,9 +103,9 @@ func TestOCSPStatus(t *testing.T) { test.AssertMarshaledEquals(t, os, out) } -func TestAcmeURLSlice(t *testing.T) { +func TestStringSlice(t *testing.T) { tc := BoulderTypeConverter{} - var au, out []*core.AcmeURL + var au, out []string marshaledI, err := tc.ToDb(au) test.AssertNotError(t, err, "Could not ToDb") diff --git a/wfe/context.go b/wfe/context.go index d904de2f9..1b1e5acdb 100644 --- a/wfe/context.go +++ b/wfe/context.go @@ -23,7 +23,7 @@ type requestEvent struct { ResponseTime time.Time `json:",omitempty"` Errors []string Requester int64 `json:",omitempty"` - Contacts *[]*core.AcmeURL `json:",omitempty"` + Contacts *[]string `json:",omitempty"` RequestNonce string `json:",omitempty"` ResponseNonce string `json:",omitempty"` UserAgent string `json:",omitempty"` diff --git a/wfe/wfe_test.go b/wfe/wfe_test.go index 593b8a0be..e93d48dbc 100644 --- a/wfe/wfe_test.go +++ b/wfe/wfe_test.go @@ -879,7 +879,7 @@ func TestNewECDSARegistration(t *testing.T) { err = json.Unmarshal([]byte(responseWriter.Body.String()), ®) test.AssertNotError(t, err, "Couldn't unmarshal returned registration object") test.Assert(t, len(*reg.Contact) >= 1, "No contact field in registration") - test.AssertEquals(t, (*reg.Contact)[0].String(), "mailto:person@mail.com") + test.AssertEquals(t, (*reg.Contact)[0], "mailto:person@mail.com") test.AssertEquals(t, reg.Agreement, "http://example.invalid/terms") test.AssertEquals(t, reg.InitialIP.String(), "1.1.1.1") @@ -944,7 +944,7 @@ func TestEmptyRegistration(t *testing.T) { err = json.Unmarshal([]byte(responseWriter.Body.String()), ®) test.AssertNotError(t, err, "Couldn't unmarshal returned registration object") test.Assert(t, len(*reg.Contact) >= 1, "No contact field in registration") - test.AssertEquals(t, (*reg.Contact)[0].String(), "mailto:person@mail.com") + test.AssertEquals(t, (*reg.Contact)[0], "mailto:person@mail.com") test.AssertEquals(t, reg.Agreement, "http://example.invalid/terms") responseWriter.Body.Reset() } @@ -1044,7 +1044,7 @@ func TestNewRegistration(t *testing.T) { err = json.Unmarshal([]byte(responseWriter.Body.String()), ®) test.AssertNotError(t, err, "Couldn't unmarshal returned registration object") test.Assert(t, len(*reg.Contact) >= 1, "No contact field in registration") - test.AssertEquals(t, (*reg.Contact)[0].String(), "mailto:person@mail.com") + test.AssertEquals(t, (*reg.Contact)[0], "mailto:person@mail.com") test.AssertEquals(t, reg.Agreement, "http://example.invalid/terms") test.AssertEquals(t, reg.InitialIP.String(), "1.1.1.1")