Skip to content

Fix segfault: guard nil OBA SDK responses (null JSON body) #3

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.

Sign up for GitHub

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

Merged
aaronbrethorst merged 2 commits into from
May 25, 2026
Merged

Fix segfault: guard nil OBA SDK responses (null JSON body) #3

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4a68412
fix: guard nil OBA SDK responses to prevent segfault
aaronbrethorst May 25, 2026
d0e0348
refactor: DRY up checks and tests to cut duplication
aaronbrethorst May 25, 2026
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
2 changes: 1 addition & 1 deletion validator/check_agencies.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ func (agencyUnionCheck) Name() string { return "agency-union" }
func (agencyUnionCheck) Run(ctx context.Context, vc *ValidationContext) []Result {
const name = "agency-union"
if vc.AgenciesErr != nil || vc.Agencies == nil {
return []Result{{Check: name, Status: Fail, Message: "agencies-with-coverage unavailable: " + redact(vc.AgenciesErr, vc.Config.APIKey)}}
return []Result{{Check: name, Status: Fail, Message: withReason("agencies-with-coverage unavailable", vc.AgenciesErr, vc.Config.APIKey)}}
}

apiSet := map[string]bool{}
Expand Down
26 changes: 22 additions & 4 deletions validator/check_alerts.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,9 @@ func (serviceAlertCheck) Run(ctx context.Context, vc *ValidationContext, src *So
var out []Result
for _, s := range sample {
obaStop := PrefixedID(agency, s.rawStop)
ad, err := vc.Client.ArrivalAndDeparture.List(ctx, obaStop, onebusaway.ArrivalAndDepartureListParams{})
if err != nil {
out = append(out, Result{Check: name, Source: src.Label, Status: Warn,
Message: fmt.Sprintf("could not query stop %q (agency prefix may be wrong): %s", obaStop, redact(err, key))})
ad, bad := queryArrivals(ctx, vc, name, src.Label, obaStop)
if bad != nil {
out = append(out, *bad)
continue
}
anySituation := false
Expand Down Expand Up @@ -86,3 +85,22 @@ func (serviceAlertCheck) Run(ctx context.Context, vc *ValidationContext, src *So
}
return out
}

// queryArrivals fetches arrivals-and-departures for a stop. It returns a non-nil
// *Result — a Warn the caller should record before skipping the stop — when the
// call errors or the server returns a null body, so the two cross-reference
// checks (alerts, trip-updates) share identical "couldn't read this stop"
// handling. A null body must never be read as "stop confirmed empty".
func queryArrivals(ctx context.Context, vc *ValidationContext, check, label, obaStop string) (*onebusaway.ArrivalAndDepartureListResponse, *Result) {
ad, err := vc.Client.ArrivalAndDeparture.List(ctx, obaStop, onebusaway.ArrivalAndDepartureListParams{})
switch {
case err != nil:
return nil, &Result{Check: check, Source: label, Status: Warn,
Message: fmt.Sprintf("could not query stop %q (agency prefix may be wrong): %s", obaStop, redact(err, vc.Config.APIKey))}
case ad == nil:
return nil, &Result{Check: check, Source: label, Status: Warn,
Message: fmt.Sprintf("arrivals query for stop %q returned a null response", obaStop),
Details: map[string]any{"stopId": obaStop}}
}
return ad, nil
}
140 changes: 28 additions & 112 deletions validator/check_alerts_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package validator
import (
"context"
"net/http"
"strings"
"testing"

gtfs "github.com/OneBusAway/go-gtfs"
Expand All @@ -12,29 +11,19 @@ import (
)

func TestServiceAlertFoundInSituationIDs(t *testing.T) {
client := newTestClient(t, func(w http.ResponseWriter, r *http.Request) {
if strings.Contains(r.URL.Path, "arrivals-and-departures-for-stop") {
w.Header().Set("Content-Type", "application/json")
w.Write([]byte(`{"data":{"entry":{"arrivalsAndDepartures":[{"stopId":"1_ST1","tripId":"1_T1","situationIds":["1_ALERT1"]}]}}}`))
return
}
t.Errorf("unexpected path %s", r.URL.Path)
})
src := &SourceContext{
Label: "ds0",
Config: config.DataSource{AgencyMapping: map[string]string{"KCM": "1"}},
PrepErrors: map[string]error{},
Static: staticForVehicle(),
ServiceAlerts: &gtfs.Realtime{Alerts: []gtfs.Alert{{
ID: "ALERT1",
InformedEntities: []gtfs.AlertInformedEntity{{StopID: strp("ST1")}},
}}},
}
client := arrivalsClient(t, `{"data":{"entry":{"arrivalsAndDepartures":[{"stopId":"1_ST1","tripId":"1_T1","situationIds":["1_ALERT1"]}]}}}`)
vc := &ValidationContext{Config: cfgForTest("test"), Client: client}
results := serviceAlertCheck{}.Run(context.Background(), vc, src)
if len(results) == 0 || results[0].Status != Pass {
t.Errorf("want Pass, got %+v", results)
}
results := serviceAlertCheck{}.Run(context.Background(), vc, alertSrcForStop())
assertFirstStatus(t, results, Pass, "alert in situationIds")
}

// A `null` arrivals response (nil SDK response, nil error) must not be mistaken
// for "stop has no situations" and Fail — it is an unconfirmed query, so Warn.
func TestServiceAlertNullArrivalsResponseWarns(t *testing.T) {
client := arrivalsClient(t, `null`)
vc := &ValidationContext{Config: cfgForTest("test"), Client: client}
results := serviceAlertCheck{}.Run(context.Background(), vc, alertSrcForStop())
assertFirstStatus(t, results, Warn, "null arrivals response")
}

func TestServiceAlertNoSamplableWarns(t *testing.T) {
Expand All @@ -47,109 +36,36 @@ func TestServiceAlertNoSamplableWarns(t *testing.T) {
}
vc := &ValidationContext{Config: cfgForTest("test")}
results := serviceAlertCheck{}.Run(context.Background(), vc, src)
if results[0].Status != Warn {
t.Errorf("agency-only alert not stop-referenceable: want Warn got %v", results[0].Status)
}
assertFirstStatus(t, results, Warn, "agency-only alert not stop-referenceable")
}

func TestServiceAlertNoSituationsFails(t *testing.T) {
client := newTestClient(t, func(w http.ResponseWriter, r *http.Request) {
if strings.Contains(r.URL.Path, "arrivals-and-departures-for-stop") {
w.Header().Set("Content-Type", "application/json")
// Arrivals present but NO situations at all, though the feed says this stop is affected.
w.Write([]byte(`{"data":{"entry":{"arrivalsAndDepartures":[{"stopId":"1_ST1","tripId":"1_T1"}]}}}`))
return
}
t.Errorf("unexpected path %s", r.URL.Path)
})
src := &SourceContext{
Label: "ds0",
Config: config.DataSource{AgencyMapping: map[string]string{"KCM": "1"}},
PrepErrors: map[string]error{},
Static: staticForVehicle(),
ServiceAlerts: &gtfs.Realtime{Alerts: []gtfs.Alert{{
ID: "ALERT1",
InformedEntities: []gtfs.AlertInformedEntity{{StopID: strp("ST1")}},
}}},
}
// Arrivals present but NO situations at all, though the feed says this stop is affected.
client := arrivalsClient(t, `{"data":{"entry":{"arrivalsAndDepartures":[{"stopId":"1_ST1","tripId":"1_T1"}]}}}`)
vc := &ValidationContext{Config: cfgForTest("test"), Client: client}
results := serviceAlertCheck{}.Run(context.Background(), vc, src)
if len(results) == 0 || results[0].Status != Fail {
t.Errorf("affected stop with no situations should Fail, got %+v", results)
}
results := serviceAlertCheck{}.Run(context.Background(), vc, alertSrcForStop())
assertFirstStatus(t, results, Fail, "affected stop with no situations")
}

func TestServiceAlertSituationsButNoMatchWarns(t *testing.T) {
client := newTestClient(t, func(w http.ResponseWriter, r *http.Request) {
if strings.Contains(r.URL.Path, "arrivals-and-departures-for-stop") {
w.Header().Set("Content-Type", "application/json")
// Situations exist but none match the feed alert id.
w.Write([]byte(`{"data":{"entry":{"arrivalsAndDepartures":[{"stopId":"1_ST1","tripId":"1_T1","situationIds":["1_DIFFERENT"]}]}}}`))
return
}
t.Errorf("unexpected path %s", r.URL.Path)
})
src := &SourceContext{
Label: "ds0",
Config: config.DataSource{AgencyMapping: map[string]string{"KCM": "1"}},
PrepErrors: map[string]error{},
Static: staticForVehicle(),
ServiceAlerts: &gtfs.Realtime{Alerts: []gtfs.Alert{{
ID: "ALERT1",
InformedEntities: []gtfs.AlertInformedEntity{{StopID: strp("ST1")}},
}}},
}
// Situations exist but none match the feed alert id.
client := arrivalsClient(t, `{"data":{"entry":{"arrivalsAndDepartures":[{"stopId":"1_ST1","tripId":"1_T1","situationIds":["1_DIFFERENT"]}]}}}`)
vc := &ValidationContext{Config: cfgForTest("test"), Client: client}
results := serviceAlertCheck{}.Run(context.Background(), vc, src)
if len(results) == 0 || results[0].Status != Warn {
t.Errorf("situations present but no match should Warn, got %+v", results)
}
results := serviceAlertCheck{}.Run(context.Background(), vc, alertSrcForStop())
assertFirstStatus(t, results, Warn, "situations present but no match")
}

func TestServiceAlertFoundInGlobalReferences(t *testing.T) {
client := newTestClient(t, func(w http.ResponseWriter, r *http.Request) {
if strings.Contains(r.URL.Path, "arrivals-and-departures-for-stop") {
w.Header().Set("Content-Type", "application/json")
// situationIds empty on the arrival, but the alert IS in references.situations
w.Write([]byte(`{"data":{"entry":{"arrivalsAndDepartures":[{"stopId":"1_ST1","tripId":"1_T1"}]},"references":{"situations":[{"id":"1_ALERT1"}]}}}`))
return
}
t.Errorf("unexpected path %s", r.URL.Path)
})
src := &SourceContext{
Label: "ds0",
Config: config.DataSource{AgencyMapping: map[string]string{"KCM": "1"}},
PrepErrors: map[string]error{},
Static: staticForVehicle(),
ServiceAlerts: &gtfs.Realtime{Alerts: []gtfs.Alert{{
ID: "ALERT1",
InformedEntities: []gtfs.AlertInformedEntity{{StopID: strp("ST1")}},
}}},
}
// situationIds empty on the arrival, but the alert IS in references.situations.
client := arrivalsClient(t, `{"data":{"entry":{"arrivalsAndDepartures":[{"stopId":"1_ST1","tripId":"1_T1"}]},"references":{"situations":[{"id":"1_ALERT1"}]}}}`)
vc := &ValidationContext{Config: cfgForTest("test"), Client: client}
results := serviceAlertCheck{}.Run(context.Background(), vc, src)
if len(results) == 0 || results[0].Status != Pass {
t.Errorf("alert in global references.situations should Pass, got %+v", results)
}
results := serviceAlertCheck{}.Run(context.Background(), vc, alertSrcForStop())
assertFirstStatus(t, results, Pass, "alert in global references.situations")
}

func TestServiceAlert404StopWarns(t *testing.T) {
client := newTestClient(t, func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusNotFound)
})
src := &SourceContext{
Label: "ds0",
Config: config.DataSource{AgencyMapping: map[string]string{"KCM": "1"}},
PrepErrors: map[string]error{},
Static: staticForVehicle(),
ServiceAlerts: &gtfs.Realtime{Alerts: []gtfs.Alert{{
ID: "ALERT1",
InformedEntities: []gtfs.AlertInformedEntity{{StopID: strp("ST1")}},
}}},
}
client := newTestClient(t, func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusNotFound) })
vc := &ValidationContext{Config: cfgForTest("test"), Client: client}
results := serviceAlertCheck{}.Run(context.Background(), vc, src)
if len(results) == 0 || results[0].Status != Warn {
t.Errorf("404 on stop should Warn (not Fail), got %+v", results)
}
results := serviceAlertCheck{}.Run(context.Background(), vc, alertSrcForStop())
assertFirstStatus(t, results, Warn, "404 on stop should Warn not Fail")
}
26 changes: 13 additions & 13 deletions validator/check_endpoints.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ func (endpointsCheck) Run(ctx context.Context, vc *ValidationContext) []Result {

// 1. current-time
ct, err := vc.Client.CurrentTime.Get(ctx)
if err != nil {
add("current-time", Fail, "current-time failed: "+redact(err, key), nil)
if err != nil || ct == nil {
add("current-time", Fail, withReason("current-time failed", err, key), nil)
skipRest("current-time failed")
return out
}
Expand All @@ -48,7 +48,7 @@ func (endpointsCheck) Run(ctx context.Context, vc *ValidationContext) []Result {

// 2. agencies-with-coverage (pre-fetched into the context)
if vc.Agencies == nil || vc.AgenciesErr != nil {
add("agencies-with-coverage", Fail, "agencies-with-coverage failed: "+redact(vc.AgenciesErr, key), nil)
add("agencies-with-coverage", Fail, withReason("agencies-with-coverage failed", vc.AgenciesErr, key), nil)
pop()
skipRest("agencies-with-coverage failed")
return out
Expand All @@ -65,8 +65,8 @@ func (endpointsCheck) Run(ctx context.Context, vc *ValidationContext) []Result {

// 3. routes-for-agency
routes, err := vc.Client.RoutesForAgency.List(ctx, agencyID)
if err != nil || len(routes.Data.List) == 0 {
add("routes-for-agency", Fail, "routes-for-agency empty/failed: "+redact(err, key), map[string]any{"agencyId": agencyID})
if err != nil || routes == nil || len(routes.Data.List) == 0 {
add("routes-for-agency", Fail, withReason("routes-for-agency empty/failed", err, key), map[string]any{"agencyId": agencyID})
pop()
skipRest("routes-for-agency failed")
return out
Expand All @@ -77,8 +77,8 @@ func (endpointsCheck) Run(ctx context.Context, vc *ValidationContext) []Result {

// 4. stops-for-route
sfr, err := vc.Client.StopsForRoute.List(ctx, routeID, onebusaway.StopsForRouteListParams{})
if err != nil || len(sfr.Data.Entry.StopIDs) == 0 {
add("stops-for-route", Fail, "stops-for-route empty/failed: "+redact(err, key), map[string]any{"routeId": routeID})
if err != nil || sfr == nil || len(sfr.Data.Entry.StopIDs) == 0 {
add("stops-for-route", Fail, withReason("stops-for-route empty/failed", err, key), map[string]any{"routeId": routeID})
pop()
skipRest("stops-for-route failed")
return out
Expand All @@ -89,8 +89,8 @@ func (endpointsCheck) Run(ctx context.Context, vc *ValidationContext) []Result {

// 5. stop
st, err := vc.Client.Stop.Get(ctx, stopID)
if err != nil || st.Data.Entry.ID != stopID {
add("stop", Fail, "stop lookup failed/mismatch: "+redact(err, key), map[string]any{"stopId": stopID})
if err != nil || st == nil || st.Data.Entry.ID != stopID {
add("stop", Fail, withReason("stop lookup failed/mismatch", err, key), map[string]any{"stopId": stopID})
pop()
skipRest("stop failed")
return out
Expand All @@ -104,8 +104,8 @@ func (endpointsCheck) Run(ctx context.Context, vc *ValidationContext) []Result {
Lat: onebusaway.Float(lat),
Lon: onebusaway.Float(lon),
})
if err != nil || loc.Data.OutOfRange || len(loc.Data.List) == 0 {
add("stops-for-location", Fail, "stops-for-location empty/out-of-range/failed: "+redact(err, key), nil)
if err != nil || loc == nil || loc.Data.OutOfRange || len(loc.Data.List) == 0 {
add("stops-for-location", Fail, withReason("stops-for-location empty/out-of-range/failed", err, key), nil)
pop()
skipRest("stops-for-location failed")
return out
Expand All @@ -115,8 +115,8 @@ func (endpointsCheck) Run(ctx context.Context, vc *ValidationContext) []Result {

// 7. arrivals-and-departures-for-stop
ad, err := vc.Client.ArrivalAndDeparture.List(ctx, stopID, onebusaway.ArrivalAndDepartureListParams{})
if err != nil {
add("arrivals-and-departures-for-stop", Fail, "arrivals failed: "+redact(err, key), map[string]any{"stopId": stopID})
if err != nil || ad == nil {
add("arrivals-and-departures-for-stop", Fail, withReason("arrivals failed", err, key), map[string]any{"stopId": stopID})
return out
}
n := len(ad.Data.Entry.ArrivalsAndDepartures)
Expand Down
Loading