Skip to content
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
Original file line number Diff line number Diff line change
Expand Up @@ -471,6 +471,8 @@ public StopV2Bean getStop(StopBean stop) {

List<String> routeIds = new ArrayList<String>();
for (RouteBean route : stop.getRoutes()) {
if (route == null)
continue;
routeIds.add(route.getId());
addToReferences(route);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
/**
* Copyright (C) 2026 OneBusAway
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.onebusaway.api.model.transit;

import static org.junit.Assert.assertTrue;

import java.util.Collections;

import org.junit.Test;
import org.onebusaway.transit_data.model.RouteBean;
import org.onebusaway.transit_data.model.StopBean;

public class BeanFactoryV2Test {

/**
* A {@link StopBean} can carry a null {@link RouteBean} in its routes list
* when the bundle's stop->route-collection index references a collection that
* no longer resolves (the production trigger for issue #461). The serializer
* must skip it rather than NPE, which would otherwise unwind out of the action
* and emit a bare "null" response body.
*/
@Test
public void getStopSkipsNullRoute() {
BeanFactoryV2 factory = new BeanFactoryV2(true);

StopBean stop = new StopBean();
stop.setId("1_stop");
stop.setRoutes(Collections.singletonList((RouteBean) null));

StopV2Bean bean = factory.getStop(stop);

assertTrue(bean.getRouteIds().isEmpty());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -141,18 +141,27 @@ public DefaultHttpHeaders show() throws ServiceException {
return setResourceNotFoundResponse();
}

if (isVersion(V1)) {
// Convert data to v1 form
List<ArrivalAndDepartureBeanV1> arrivals = getArrivalsAsV1(result);
StopWithArrivalsAndDeparturesBeanV1 v1 = new StopWithArrivalsAndDeparturesBeanV1(
result.getStop(), arrivals, result.getNearbyStops());
return setOkResponse(v1);
} else if (isVersion(V2)) {
BeanFactoryV2 factory = getBeanFactoryV2();
factory.setCustomRouteSort(customRouteSort);
return setOkResponse(factory.getResponse(result));
} else {
return setUnknownVersionResponse();
// Bean conversion must stay inside a try/catch: an unexpected failure here
// (e.g. a corrupt bundle record) would otherwise unwind out of the action
// with _response unset, and the ModelDriven serializer would emit a bare
// "null" body with HTTP 200 instead of a proper error. See issue #461.
try {
if (isVersion(V1)) {
// Convert data to v1 form
List<ArrivalAndDepartureBeanV1> arrivals = getArrivalsAsV1(result);
StopWithArrivalsAndDeparturesBeanV1 v1 = new StopWithArrivalsAndDeparturesBeanV1(
result.getStop(), arrivals, result.getNearbyStops());
return setOkResponse(v1);
} else if (isVersion(V2)) {
BeanFactoryV2 factory = getBeanFactoryV2();
factory.setCustomRouteSort(customRouteSort);
return setOkResponse(factory.getResponse(result));
} else {
return setUnknownVersionResponse();
}
} catch (Exception any) {
_log.error("Failed to build arrivals-and-departures response for stop {}", _id, any);
return setExceptionResponse();
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,16 @@
import org.onebusaway.transit_data_federation.services.transit_graph.TransitGraphDao;
import org.onebusaway.utility.text.NaturalStringOrder;
import org.onebusaway.utility.text.StringLibrary;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.stereotype.Component;

@Component
class StopBeanServiceImpl implements StopBeanService {

private static final Logger _log = LoggerFactory.getLogger(StopBeanServiceImpl.class);

private static RouteBeanComparator _routeBeanComparator = new RouteBeanComparator();

private TransitGraphDao _transitGraphDao;
Expand Down Expand Up @@ -143,6 +147,12 @@ private void fillTransfersForStopBean(StopEntry stop, StopNarrative narrative, S
List<RouteBean> routeBeans = new ArrayList<>(staticRoutes.size());
for (AgencyAndId routeId : staticRoutes) {
RouteBean staticRouteBean = _routeBeanService.getRouteForId(routeId);
if (staticRouteBean == null) {
// Same orphan-route guard as fillRoutesForStopBean below; see issue #461.
_log.warn("stop {} references static route {} that does not resolve to a route; skipping",
stop.getId(), routeId);
continue;
}
routeBeans.add(staticRouteBean);
}
bean.setStaticRoutes(routeBeans);
Expand All @@ -162,6 +172,15 @@ private void fillRoutesForStopBean(StopEntry stop, StopBean sb, AgencyServiceInt

for (AgencyAndId routeCollectionId : routeCollectionIds) {
RouteBean bean = _routeBeanService.getRouteForId(routeCollectionId);
if (bean == null) {
// A corrupt stop->route-collection index entry can reference a
// collection that no longer resolves to a route. Dropping it here keeps
// a single bad entry from NPEing bean serialization and blanking the
// entire API response. See issue #461.
_log.warn("stop {} references route collection {} that does not resolve to a route; skipping",
stop.getId(), routeCollectionId);
continue;
}
routeBeans.add(bean);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,14 @@ public StopWithArrivalsAndDeparturesBean getArrivalsAndDeparturesByStopId(
List<AgencyAndId> nearbyStopIds = _nearbyStopsBeanService.getNearbyStops(
stop, 100);
List<StopBean> nearbyStops = new ArrayList<StopBean>();
for (AgencyAndId nearbyStopId : nearbyStopIds)
nearbyStops.add(_stopBeanService.getStopForId(nearbyStopId, serviceInterval));
for (AgencyAndId nearbyStopId : nearbyStopIds) {
StopBean nearbyStop = _stopBeanService.getStopForId(nearbyStopId, serviceInterval);
if (nearbyStop == null) {
_log.warn("nearby stop {} did not resolve to a stop; skipping", nearbyStopId);
continue;
}
nearbyStops.add(nearbyStop);
}

List<ServiceAlertBean> situations = _serviceAlertsBeanService.getServiceAlertsForStopId(
query.getTime(), id);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertSame;
import static org.junit.Assert.assertTrue;

import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
Expand Down Expand Up @@ -116,4 +118,79 @@ public void testGetStopForId() {

assertSame(route, routes.get(0));
}

/**
* A stop's route-collection index can reference a collection that no longer
* resolves to a route bean (e.g. a corrupt service-date entry in the bundle).
* {@link RouteBeanService#getRouteForId} then returns null. We must not let a
* null {@link RouteBean} leak into the stop's routes list: in production a
* single such orphan route survives the sort (a lone element is never
* compared) and later NPEs in bean serialization, blanking the whole API
* response with a literal "null".
*/
@Test
public void testGetStopForId_skipsUnresolvableRoute() {

AgencyAndId stopId = new AgencyAndId("29", "1109");

StopEntryImpl stopEntry = new StopEntryImpl(stopId, 47.1, -122.1);
Mockito.when(_transitGraphDao.getStopEntryForId(stopId)).thenReturn(
stopEntry);

StopNarrative.Builder builder = StopNarrative.builder();
builder.setName("stop name");
StopNarrative stop = builder.create();
Mockito.when(_narrativeService.getStopForId(stopId)).thenReturn(stop);

AgencyAndId orphanRouteId = new AgencyAndId("1", "orphan");

Set<AgencyAndId> routeIds = new HashSet<AgencyAndId>();
routeIds.add(orphanRouteId);
Mockito.when(_routeService.getRouteCollectionIdsForStop(stopId)).thenReturn(
routeIds);

// the corrupt collection id resolves to null
Mockito.when(_routeBeanService.getRouteForId(orphanRouteId)).thenReturn(null);

StopBean stopBean = _service.getStopForId(stopId, null);

assertNotNull(stopBean);
List<RouteBean> routes = stopBean.getRoutes();
assertTrue("unresolvable route must not leak into routes list", routes.isEmpty());
}

/**
* The static-routes path resolves route beans the same way as the primary
* routes path, so an unresolvable static route must likewise be dropped
* rather than leak a null into the stop's static routes list. See issue #461.
*/
@Test
public void testGetStopForId_skipsUnresolvableStaticRoute() {

AgencyAndId stopId = new AgencyAndId("29", "1109");

StopEntryImpl stopEntry = new StopEntryImpl(stopId, 47.1, -122.1);
Mockito.when(_transitGraphDao.getStopEntryForId(stopId)).thenReturn(
stopEntry);

StopNarrative.Builder builder = StopNarrative.builder();
builder.setName("stop name");
Mockito.when(_narrativeService.getStopForId(stopId)).thenReturn(builder.create());

// no active routes, so only the static-route path populates the bean
Mockito.when(_routeService.getRouteCollectionIdsForStop(stopId)).thenReturn(
new HashSet<AgencyAndId>());

AgencyAndId orphanStaticRouteId = new AgencyAndId("1", "orphan");
Mockito.when(_narrativeService.getStaticRoutes(stopId)).thenReturn(
Collections.singletonList(orphanStaticRouteId));
// the corrupt static route resolves to null
Mockito.when(_routeBeanService.getRouteForId(orphanStaticRouteId)).thenReturn(null);

StopBean stopBean = _service.getStopForId(stopId, null);

assertNotNull(stopBean);
assertTrue("unresolvable static route must not leak into static routes list",
stopBean.getStaticRoutes().isEmpty());
}
}
Loading