Skip to content

Commit bd29cc4

Browse files
authored
Allow reading incident rows with NULL columns (#6961)
Fixes #6960
1 parent 08017e4 commit bd29cc4

File tree

6 files changed

+87
-24
lines changed

6 files changed

+87
-24
lines changed

sa/model.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -850,10 +850,10 @@ func incidentModelToPB(i incidentModel) sapb.Incident {
850850

851851
// incidentSerialModel represents a row in an 'incident_*' table.
852852
type incidentSerialModel struct {
853-
Serial string `db:"serial"`
854-
RegistrationID int64 `db:"registrationID"`
855-
OrderID int64 `db:"orderID"`
856-
LastNoticeSent time.Time `db:"lastNoticeSent"`
853+
Serial string `db:"serial"`
854+
RegistrationID *int64 `db:"registrationID"`
855+
OrderID *int64 `db:"orderID"`
856+
LastNoticeSent *time.Time `db:"lastNoticeSent"`
857857
}
858858

859859
// crlEntryModel has just the certificate status fields necessary to construct

sa/model_test.go

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616
"github.com/letsencrypt/boulder/db"
1717
"github.com/letsencrypt/boulder/grpc"
1818
"github.com/letsencrypt/boulder/probs"
19+
"github.com/letsencrypt/boulder/test/vars"
1920

2021
"github.com/letsencrypt/boulder/core"
2122
corepb "github.com/letsencrypt/boulder/core/proto"
@@ -346,3 +347,52 @@ func makeKey() rsa.PrivateKey {
346347
q := bigIntFromB64("uKE2dh-cTf6ERF4k4e_jy78GfPYUIaUyoSSJuBzp3Cubk3OCqs6grT8bR_cu0Dm1MZwWmtdqDyI95HrUeq3MP15vMMON8lHTeZu2lmKvwqW7anV5UzhM1iZ7z4yMkuUwFWoBvyY898EXvRD-hdqRxHlSqAZ192zB3pVFJ0s7pFc=")
347348
return rsa.PrivateKey{PublicKey: rsa.PublicKey{N: n, E: e}, D: d, Primes: []*big.Int{p, q}}
348349
}
350+
351+
func TestIncidentSerialModel(t *testing.T) {
352+
testIncidentsDbMap, err := DBMapForTest(vars.DBConnIncidentsFullPerms)
353+
test.AssertNotError(t, err, "Couldn't create test dbMap")
354+
defer test.ResetIncidentsTestDatabase(t)
355+
356+
// Inserting and retrieving a row with only the serial populated should work.
357+
_, err = testIncidentsDbMap.Exec(
358+
"INSERT INTO incident_foo (serial) VALUES (?)",
359+
"1337",
360+
)
361+
test.AssertNotError(t, err, "inserting row with only serial")
362+
363+
var res1 incidentSerialModel
364+
err = testIncidentsDbMap.SelectOne(
365+
&res1,
366+
"SELECT * FROM incident_foo WHERE serial = ?",
367+
"1337",
368+
)
369+
test.AssertNotError(t, err, "selecting row with only serial")
370+
371+
test.AssertEquals(t, res1.Serial, "1337")
372+
test.AssertBoxedNil(t, res1.RegistrationID, "registrationID should be NULL")
373+
test.AssertBoxedNil(t, res1.OrderID, "orderID should be NULL")
374+
test.AssertBoxedNil(t, res1.LastNoticeSent, "lastNoticeSent should be NULL")
375+
376+
// Inserting and retrieving a row with all columns populated should work.
377+
_, err = testIncidentsDbMap.Exec(
378+
"INSERT INTO incident_foo (serial, registrationID, orderID, lastNoticeSent) VALUES (?, ?, ?, ?)",
379+
"1338",
380+
1,
381+
2,
382+
time.Date(2023, 06, 29, 16, 9, 00, 00, time.UTC),
383+
)
384+
test.AssertNotError(t, err, "inserting row with only serial")
385+
386+
var res2 incidentSerialModel
387+
err = testIncidentsDbMap.SelectOne(
388+
&res2,
389+
"SELECT * FROM incident_foo WHERE serial = ?",
390+
"1338",
391+
)
392+
test.AssertNotError(t, err, "selecting row with only serial")
393+
394+
test.AssertEquals(t, res2.Serial, "1338")
395+
test.AssertEquals(t, *res2.RegistrationID, int64(1))
396+
test.AssertEquals(t, *res2.OrderID, int64(2))
397+
test.AssertEquals(t, *res2.LastNoticeSent, time.Date(2023, 06, 29, 16, 9, 00, 00, time.UTC))
398+
}

sa/proto/sa.pb.go

Lines changed: 3 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

sa/proto/sa.proto

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -334,9 +334,9 @@ message SerialsForIncidentRequest {
334334

335335
message IncidentSerial {
336336
string serial = 1;
337-
int64 registrationID = 2;
338-
int64 orderID = 3;
339-
int64 lastNoticeSent = 4; // Unix timestamp (nanoseconds)
337+
int64 registrationID = 2; // May be 0 (NULL)
338+
int64 orderID = 3; // May be 0 (NULL)
339+
int64 lastNoticeSent = 4; // Unix timestamp (nanoseconds), may be 0 (NULL)
340340
}
341341

342342
message GetRevokedCertsRequest {

sa/sa_test.go

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2926,6 +2926,8 @@ func TestIncidentsForSerial(t *testing.T) {
29262926
test.AssertNotError(t, err, "Couldn't create test dbMap")
29272927
defer test.ResetIncidentsTestDatabase(t)
29282928

2929+
weekAgo := sa.clk.Now().Add(-time.Hour * 24 * 7)
2930+
29292931
// Add a disabled incident.
29302932
err = testSADbMap.Insert(&incidentModel{
29312933
SerialTable: "incident_foo",
@@ -2950,11 +2952,12 @@ func TestIncidentsForSerial(t *testing.T) {
29502952
test.AssertNotError(t, err, "Failed to insert enabled incident")
29512953

29522954
// Add a row to the incident table with serial '1338'.
2955+
one := int64(1)
29532956
affectedCertA := incidentSerialModel{
29542957
Serial: "1338",
2955-
RegistrationID: 1,
2956-
OrderID: 1,
2957-
LastNoticeSent: sa.clk.Now().Add(time.Hour * 24 * 7),
2958+
RegistrationID: &one,
2959+
OrderID: &one,
2960+
LastNoticeSent: &weekAgo,
29582961
}
29592962
_, err = testIncidentsDbMap.Exec(
29602963
fmt.Sprintf("INSERT INTO incident_bar (%s) VALUES ('%s', %d, %d, '%s')",
@@ -2973,11 +2976,12 @@ func TestIncidentsForSerial(t *testing.T) {
29732976
test.AssertEquals(t, len(result.Incidents), 0)
29742977

29752978
// Add a row to the incident table with serial '1337'.
2979+
two := int64(2)
29762980
affectedCertB := incidentSerialModel{
29772981
Serial: "1337",
2978-
RegistrationID: 2,
2979-
OrderID: 2,
2980-
LastNoticeSent: sa.clk.Now().Add(time.Hour * 24 * 7),
2982+
RegistrationID: &two,
2983+
OrderID: &two,
2984+
LastNoticeSent: &weekAgo,
29812985
}
29822986
_, err = testIncidentsDbMap.Exec(
29832987
fmt.Sprintf("INSERT INTO incident_bar (%s) VALUES ('%s', %d, %d, '%s')",

sa/saro.go

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1205,7 +1205,9 @@ func (ssa *SQLStorageAuthority) IncidentsForSerial(ctx context.Context, req *sap
12051205
// SerialsForIncident queries the provided incident table and returns the
12061206
// resulting rows as a stream of `*sapb.IncidentSerial`s. An `io.EOF` error
12071207
// signals that there are no more serials to send. If the incident table in
1208-
// question contains zero rows, only an `io.EOF` error is returned.
1208+
// question contains zero rows, only an `io.EOF` error is returned. The
1209+
// IncidentSerial messages returned may have the zero-value for their OrderID,
1210+
// RegistrationID, and LastNoticeSent fields, if those are NULL in the database.
12091211
func (ssa *SQLStorageAuthorityRO) SerialsForIncident(req *sapb.SerialsForIncidentRequest, stream sapb.StorageAuthorityReadOnly_SerialsForIncidentServer) error {
12101212
if req.IncidentTable == "" {
12111213
return errIncompleteRequest
@@ -1235,13 +1237,20 @@ func (ssa *SQLStorageAuthorityRO) SerialsForIncident(req *sapb.SerialsForInciden
12351237
return err
12361238
}
12371239

1238-
err = stream.Send(
1239-
&sapb.IncidentSerial{
1240-
Serial: ism.Serial,
1241-
RegistrationID: ism.RegistrationID,
1242-
OrderID: ism.OrderID,
1243-
LastNoticeSent: ism.LastNoticeSent.UnixNano(),
1244-
})
1240+
ispb := &sapb.IncidentSerial{
1241+
Serial: ism.Serial,
1242+
}
1243+
if ism.RegistrationID != nil {
1244+
ispb.RegistrationID = *ism.RegistrationID
1245+
}
1246+
if ism.OrderID != nil {
1247+
ispb.OrderID = *ism.OrderID
1248+
}
1249+
if ism.LastNoticeSent != nil {
1250+
ispb.LastNoticeSent = ism.LastNoticeSent.UnixNano()
1251+
}
1252+
1253+
err = stream.Send(ispb)
12451254
if err != nil {
12461255
return err
12471256
}

0 commit comments

Comments
 (0)