Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
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
2 changes: 1 addition & 1 deletion docs/http.md
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ possible. In that case, only the `code` field will be returned.

- `code` if the request was successful `Ok` otherwise see the service dependent and general status codes.
- `waypoints` array of `Waypoint` objects sorted by distance to the input coordinate. Each object has at least the following additional properties:
- `nodes`: Array of OpenStreetMap node ids.
- `nodes`: Array of OpenStreetMap node ids. Each id is a 64-bit unsigned integer (encoded as a JSON number for the `json` format, and as `ulong` for the `flatbuffers` format).

#### Example Requests

Expand Down
10 changes: 5 additions & 5 deletions features/support/fbresult_generated.js
Original file line number Diff line number Diff line change
Expand Up @@ -110,15 +110,15 @@ var Annotation = class _Annotation {
}
nodes(index) {
const offset = this.bb.__offset(this.bb_pos, 10);
return offset ? this.bb.readUint32(this.bb.__vector(this.bb_pos + offset) + index * 4) : 0;
return offset ? this.bb.readUint64(this.bb.__vector(this.bb_pos + offset) + index * 8) : BigInt(0);
}
nodesLength() {
const offset = this.bb.__offset(this.bb_pos, 10);
return offset ? this.bb.__vector_len(this.bb_pos + offset) : 0;
}
nodesArray() {
const offset = this.bb.__offset(this.bb_pos, 10);
return offset ? new Uint32Array(this.bb.bytes().buffer, this.bb.bytes().byteOffset + this.bb.__vector(this.bb_pos + offset), this.bb.__vector_len(this.bb_pos + offset)) : null;
return offset ? new BigUint64Array(this.bb.bytes().buffer, this.bb.bytes().byteOffset + this.bb.__vector(this.bb_pos + offset), this.bb.__vector_len(this.bb_pos + offset)) : null;
}
weight(index) {
const offset = this.bb.__offset(this.bb_pos, 12);
Expand Down Expand Up @@ -194,14 +194,14 @@ var Annotation = class _Annotation {
builder.addFieldOffset(3, nodesOffset, 0);
}
static createNodesVector(builder, data) {
builder.startVector(4, data.length, 4);
builder.startVector(8, data.length, 8);
for (let i = data.length - 1; i >= 0; i--) {
builder.addInt32(data[i]);
builder.addInt64(BigInt(data[i]));
}
return builder.endVector();
}
static startNodesVector(builder, numElems) {
builder.startVector(4, numElems, 4);
builder.startVector(8, numElems, 8);
}
static addWeight(builder, weightOffset) {
builder.addFieldOffset(4, weightOffset, 0);
Expand Down
2 changes: 1 addition & 1 deletion include/engine/api/flatbuffers/route.fbs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ table Annotation {
distance: [uint];
duration: [uint];
datasources: [uint];
nodes: [uint];
nodes: [ulong];
weight: [uint];
speed: [float];
metadata: Metadata;
Expand Down
2 changes: 1 addition & 1 deletion include/engine/api/route_api.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -522,7 +522,7 @@ class RouteAPI : public BaseAPI
[](const guidance::LegGeometry::Annotation &anno)
{ return anno.datasource; });
}
std::vector<uint32_t> nodes;
std::vector<uint64_t> nodes;
if (requested_annotations & RouteParameters::AnnotationsType::Nodes)
{
nodes.reserve(leg_geometry.node_ids.size());
Expand Down
2 changes: 2 additions & 0 deletions include/extractor/files.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

#include "extractor/edge_based_edge.hpp"
#include "extractor/node_data_container.hpp"
#include "extractor/packed_osm_ids.hpp"
#include "extractor/profile_properties.hpp"
#include "extractor/query_node.hpp"
#include "extractor/serialization.hpp"
Expand Down Expand Up @@ -453,6 +454,7 @@ void readRawNBGraph(const std::filesystem::path &path,
{
coordinates[index].lon = current_node.lon;
coordinates[index].lat = current_node.lat;
checkPackedOSMNodeIdFits(current_node.node_id);
osm_node_ids.push_back(current_node.node_id);
index++;
};
Expand Down
31 changes: 30 additions & 1 deletion include/extractor/packed_osm_ids.hpp
Original file line number Diff line number Diff line change
@@ -1,19 +1,48 @@
#ifndef OSRM_EXTRACTOR_PACKED_OSM_IDS_HPP
#define OSRM_EXTRACTOR_PACKED_OSM_IDS_HPP

#include "util/exception.hpp"
#include "util/packed_vector.hpp"
#include "util/typedefs.hpp"

#include <string>

namespace osrm::extractor
{
namespace detail
{
// Bit width for packed OSM node IDs. Real-world OSM node IDs already exceed
// 2^33 and grow over time, so 34 bits is no longer sufficient and would
// silently truncate IDs >= 2^34 when written into PackedVector storage.
// 40 bits supports IDs up to ~1.1 * 10^12, providing decades of headroom.
// See https://github.com/Project-OSRM/osrm-backend/issues/7069
inline constexpr std::size_t PACKED_OSM_ID_BITS = 40;

template <storage::Ownership Ownership>
using PackedOSMIDs = util::detail::PackedVector<OSMNodeID, 34, Ownership>;
using PackedOSMIDs = util::detail::PackedVector<OSMNodeID, PACKED_OSM_ID_BITS, Ownership>;
} // namespace detail

using PackedOSMIDsView = detail::PackedOSMIDs<storage::Ownership::View>;
using PackedOSMIDs = detail::PackedOSMIDs<storage::Ownership::Container>;

// Maximum OSM node ID that can be stored in PackedOSMIDs without truncation.
inline constexpr std::uint64_t MAX_PACKED_OSM_NODE_ID =
(std::uint64_t{1} << detail::PACKED_OSM_ID_BITS) - 1;

// Throws util::exception if `id` is too large to fit into PackedOSMIDs.
// Use this at every call site that pushes OSM node IDs into PackedOSMIDs to
// fail loudly during extraction rather than silently truncating IDs.
inline void checkPackedOSMNodeIdFits(const OSMNodeID id)
{
if (static_cast<std::uint64_t>(id) > MAX_PACKED_OSM_NODE_ID)
{
throw util::exception("OSM node ID " + std::to_string(static_cast<std::uint64_t>(id)) +
" exceeds the " + std::to_string(detail::PACKED_OSM_ID_BITS) +
"-bit packed storage limit (" +
std::to_string(MAX_PACKED_OSM_NODE_ID) +
"). Increase PACKED_OSM_ID_BITS in packed_osm_ids.hpp.");
}
}
} // namespace osrm::extractor

#endif
1 change: 1 addition & 0 deletions src/extractor/extractor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -632,6 +632,7 @@ Extractor::ParsedOSMData Extractor::ParseOSMData(ScriptingEnvironment &scripting
const auto &current_node = extraction_containers.used_nodes[index];
osm_coordinates[index].lon = current_node.lon;
osm_coordinates[index].lat = current_node.lat;
checkPackedOSMNodeIdFits(current_node.node_id);
osm_node_ids.push_back(current_node.node_id);
}

Expand Down
47 changes: 47 additions & 0 deletions unit_tests/util/packed_vector.cpp
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
#include "util/packed_vector.hpp"
#include "util/typedefs.hpp"

#include "extractor/packed_osm_ids.hpp"
#include "util/exception.hpp"

#include "common/range_tools.hpp"

#include <algorithm>
Expand Down Expand Up @@ -202,4 +205,48 @@ BOOST_AUTO_TEST_CASE(packed_vector_33bit_continious)
}
}

// Regression test for https://github.com/Project-OSRM/osrm-backend/issues/7069
// OSM node IDs above 2^34 must round-trip through PackedOSMIDs without
// truncation. The previous 34-bit packing silently masked these values.
BOOST_AUTO_TEST_CASE(packed_osm_ids_above_2pow34_roundtrip)
{
osrm::extractor::PackedOSMIDs ids;

const std::vector<std::uint64_t> inputs = {
0ull,
1ull,
(1ull << 33), // 2^33
(1ull << 34), // 2^34, the old truncation boundary
(1ull << 34) + 1, // just above 2^34
20'000'000'000ull, // realistic projected OSM node ID
(1ull << 35), // 2^35
osrm::extractor::MAX_PACKED_OSM_NODE_ID - 1, // just below the new limit
osrm::extractor::MAX_PACKED_OSM_NODE_ID, // exactly at the new limit
};

for (auto raw : inputs)
{
ids.push_back(OSMNodeID{raw});
}

for (std::size_t i = 0; i < inputs.size(); ++i)
{
BOOST_CHECK_EQUAL(static_cast<std::uint64_t>(ids[i]), inputs[i]);
}
}

// Verify that the overflow guard fires for IDs above the packed-storage limit
// instead of silently masking them on push_back.
BOOST_AUTO_TEST_CASE(packed_osm_ids_overflow_guard_throws)
{
const OSMNodeID just_above_limit{osrm::extractor::MAX_PACKED_OSM_NODE_ID + 1};
BOOST_CHECK_THROW(osrm::extractor::checkPackedOSMNodeIdFits(just_above_limit),
osrm::util::exception);

// Sanity: values at and below the limit must not throw.
BOOST_CHECK_NO_THROW(osrm::extractor::checkPackedOSMNodeIdFits(
OSMNodeID{osrm::extractor::MAX_PACKED_OSM_NODE_ID}));
BOOST_CHECK_NO_THROW(osrm::extractor::checkPackedOSMNodeIdFits(OSMNodeID{1ull << 34}));
}

BOOST_AUTO_TEST_SUITE_END()
Loading