Skip to content
Closed
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
6 changes: 3 additions & 3 deletions 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 Expand Up @@ -700,7 +700,7 @@ Annotation of the whole route leg with fine-grained information about each segme
- `distance`: The distance, in meters, between each pair of coordinates
- `duration`: The duration between each pair of coordinates, in seconds. Does not include the duration of any turns.
- `datasources`: The index of the data source for the speed between each pair of coordinates. `0` is the default profile, other values are supplied via `--segment-speed-file` to `osrm-contract` or `osrm-customize`. String-like names are in the `metadata.datasource_names` array.
- `nodes`: The OSM node ID for each coordinate along the route, excluding the first/last user-supplied coordinates
- `nodes`: Array of OpenStreetMap node ids for each coordinate along the route (excluding the first/last user-supplied coordinates). Each id is a 64-bit unsigned integer (encoded as a JSON number for the `json` format, and as `ulong` for the `flatbuffers` format). Clients consuming flatbuffers should treat these values as 64-bit integers (JS bindings expose them as BigInt).
- `weight`: The weights between each pair of coordinates. Does not include any turn costs.
- `speed`: Convenience field, calculation of `distance / duration` rounded to one decimal place
- `metadata`: Metadata related to other annotations
Expand Down Expand Up @@ -1094,7 +1094,7 @@ Almost the same as `json` StepManeuver object. The following properties differ:

### Annotation object

Exactly the same as `json` annotation object.
Almost the same as the `json` annotation object. Note: on the flatbuffers wire the `Annotation.nodes` field is a `[ulong]` (64-bit unsigned integers), and the generated JavaScript flatbuffers bindings will expose these values as BigInt / BigUint64Array. Clients that consume flatbuffers should handle 64-bit node ids accordingly.


### Position object
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
3 changes: 2 additions & 1 deletion src/util/fingerprint.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ FingerPrint FingerPrint::GetValid()
// 4 chars, 'O','S','R','N' - note the N instead of M, v1 of the fingerprint
// used M, so we add one and use N to indicate the newer fingerprint magic number.
// Bump this value if the fingerprint format ever changes.
fingerprint.magic_number = {{'O', 'S', 'R', 'N'}};
// Changed to force incompatibility for updated packed storage layout (packed_osm_ids.hpp).
fingerprint.magic_number = {{'O', 'S', 'R', 'O'}};
fingerprint.major_version = OSRM_VERSION_MAJOR;
fingerprint.minor_version = OSRM_VERSION_MINOR;
fingerprint.patch_version = OSRM_VERSION_PATCH;
Expand Down
81 changes: 81 additions & 0 deletions unit_tests/library/fb_annotation_nodes.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
#include <boost/test/unit_test.hpp>

#include <cstdint>
#include <vector>

#include "engine/api/flatbuffers/fbresult_generated.h"
#include "extractor/packed_osm_ids.hpp"

#include <flatbuffers/flatbuffers.h>

BOOST_AUTO_TEST_CASE(fb_annotation_nodes_64bit_roundtrip)
{
using namespace osrm;
using namespace osrm::engine::api;

flatbuffers::FlatBufferBuilder builder;

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
extractor::MAX_PACKED_OSM_NODE_ID - 1, // just below the new limit
extractor::MAX_PACKED_OSM_NODE_ID, // exactly at the new limit
};

// Create flatbuffers vector of uint64_t nodes
auto nodes_vector = builder.CreateVector(inputs);

// Build an Annotation with the nodes vector
fbresult::AnnotationBuilder annotation_builder(builder);
annotation_builder.add_nodes(nodes_vector);
auto annotation_off = annotation_builder.Finish();

// Create a Leg that contains the annotation
fbresult::LegBuilder leg_builder(builder);
leg_builder.add_annotations(annotation_off);
auto leg_off = leg_builder.Finish();

// Create a RouteObject that contains the leg
std::vector<flatbuffers::Offset<fbresult::Leg>> legs_vec = {leg_off};
auto legs_vector = builder.CreateVector(legs_vec);

fbresult::RouteObjectBuilder route_builder(builder);
route_builder.add_legs(legs_vector);
auto route_off = route_builder.Finish();

// Create FBResult root with the route
std::vector<flatbuffers::Offset<fbresult::RouteObject>> routes_vec = {route_off};
auto routes_vector = builder.CreateVector(routes_vec);

fbresult::FBResultBuilder result_builder(builder);
result_builder.add_routes(routes_vector);
auto result_off = result_builder.Finish();
builder.Finish(result_off);

// Read back and verify
auto fb = fbresult::GetFBResult(builder.GetBufferPointer());
BOOST_REQUIRE(fb);
auto routes = fb->routes();
BOOST_REQUIRE(routes);
auto route = routes->Get(0);
BOOST_REQUIRE(route);
auto legs = route->legs();
BOOST_REQUIRE(legs);
auto leg = legs->Get(0);
BOOST_REQUIRE(leg);
auto annotation = leg->annotations();
BOOST_REQUIRE(annotation);
auto nodes = annotation->nodes();
BOOST_REQUIRE(nodes);

BOOST_CHECK_EQUAL(nodes->size(), inputs.size());
for (std::size_t i = 0; i < inputs.size(); ++i)
{
BOOST_CHECK_EQUAL(static_cast<std::uint64_t>(nodes->Get(i)), inputs[i]);
}
}
48 changes: 48 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,49 @@ 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)
{
const OSMNodeID id_val = ids[i];
BOOST_CHECK_EQUAL(static_cast<std::uint64_t>(id_val), 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