Skip to content

fix(api)!: prevent OSM node ID truncation in match/route annotations#7552

Merged
DennisOSRM merged 9 commits into
masterfrom
fix/osm-node-id-overflow-7069
May 15, 2026
Merged

fix(api)!: prevent OSM node ID truncation in match/route annotations#7552
DennisOSRM merged 9 commits into
masterfrom
fix/osm-node-id-overflow-7069

Conversation

@DennisOSRM
Copy link
Copy Markdown
Collaborator

  • fix(api)!: prevent OSM node ID truncation in match/route annotations
  • Fix formatting: align inline comments in packed_vector test
  • Mark incompatible fingerprint and update HTTP docs for 64-bit OSM node ids
  • Add FlatBuffers Annotation.nodes roundtrip unit test
  • Fix compile: avoid chained conversions in packed_vector test
  • Fix compile: use OSMNodeID when extracting from PackedOSMIDs

Contributor and others added 7 commits May 1, 2026 11:52
OSM node IDs above 2^34 were silently truncated when written into the
PackedOSMIDs storage (PackedVector<OSMNodeID, 34, ...>) used for the
.osrm.nbg_nodes file. The truncated values then flowed unchanged through
GetOSMNodeIDOfNode into the annotations.nodes field of /match and /route
responses, as well as vector tiles.

Additionally, the FlatBuffers schema for Annotation.nodes used [uint],
truncating any ID above 2^32 a second time on the FB wire path; the JSON
path was already 64-bit clean once storage stops truncating.

Changes:
- Widen PackedOSMIDs from 34 to 40 bits (max ~1.1e12, decades of headroom).
- Add checkPackedOSMNodeIdFits() guard at every osm_node_ids.push_back
  site so future overflow throws util::exception during osrm-extract
  rather than silently masking values.
- Change Annotation.nodes FlatBuffers field from [uint] to [ulong] and
  fix the corresponding std::vector<uint32_t> in route_api.hpp to
  std::vector<uint64_t>; regenerate fbresult_generated.js accordingly.
- Add unit tests for round-trip of IDs above 2^34 and for the overflow
  guard.
- Document the 64-bit semantics of annotations.nodes in docs/http.md.

This is a breaking change for two reasons:
1. The on-disk packed-vector layout for osm_node_ids changes; existing
   .osrm datasets must be re-extracted with the new osrm-extract.
2. The FlatBuffers Annotation.nodes wire type changes from uint32 to
   uint64; FB clients reading annotations.nodes must update accordingly.

Fixes #7069
Align array entry comments for readability.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…e ids

Bump the dataset fingerprint (magic number) to force incompatibility for the updated
packed storage layout (packed_osm_ids). Update docs/http.md to clarify that
Annotation.nodes are 64-bit (flatbuffers: [ulong]) and that JS bindings expose
these values as BigInt.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes OSM node ID truncation in the route/match annotations API: OSM node IDs above 2^34 were being silently masked by the PackedOSMIDs storage (34-bit) and again truncated by the FlatBuffers Annotation.nodes field (32-bit). Widens packed storage to 40 bits with an explicit overflow check, switches the FlatBuffers field to ulong (64-bit), bumps the data fingerprint, updates docs, and adds regression tests.

Changes:

  • Widen PackedOSMIDs to 40 bits, add MAX_PACKED_OSM_NODE_ID and checkPackedOSMNodeIdFits guard called at every push site in extractor.cpp / files.hpp.
  • Change FlatBuffers Annotation.nodes from [uint] to [ulong], update C++ route API serializer and JS bindings (BigUint64Array/BigInt), and bump fingerprint magic to invalidate caches built with the old 34-bit layout.
  • Add regression unit tests (packed_vector.cpp round-trip + overflow throw, new library/fb_annotation_nodes.cpp for the 64-bit FB wire format) and document the breaking change in docs/http.md.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.

Show a summary per file
File Description
include/extractor/packed_osm_ids.hpp Switches packed width to 40 bits; adds MAX_PACKED_OSM_NODE_ID and checkPackedOSMNodeIdFits guard.
src/extractor/extractor.cpp Calls the new overflow guard before pushing node_id into packed storage.
include/extractor/files.hpp Adds the same guard in the raw-NB-graph reader and pulls in packed_osm_ids.hpp.
include/engine/api/flatbuffers/route.fbs Promotes Annotation.nodes to [ulong].
include/engine/api/route_api.hpp FlatBuffers nodes vector typed as uint64_t.
features/support/fbresult_generated.js Regenerated JS bindings: BigInt/BigUint64Array/addInt64/8-byte vector stride.
src/util/fingerprint.cpp Bumps magic number from OSRN to OSRO to invalidate old caches.
docs/http.md Documents 64-bit node IDs and BigInt behavior for the FlatBuffers wire format.
unit_tests/util/packed_vector.cpp Adds round-trip and overflow-guard regression tests for PackedOSMIDs.
unit_tests/library/fb_annotation_nodes.cpp New test that round-trips 64-bit node IDs through the FlatBuffers Annotation.nodes field.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@DennisOSRM DennisOSRM merged commit f975ffa into master May 15, 2026
26 checks passed
@DennisOSRM DennisOSRM deleted the fix/osm-node-id-overflow-7069 branch May 15, 2026 18:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants