Modify ↓

Opened 12 months ago

Closed 11 months ago

Last modified 11 months ago

#23550 closed defect (fixed)

[PATCH] Error when deserializing PBF Blob

Reported by: giora.kosoi@… Owned by: team
Priority: normal Milestone: 24.03
Component: Core Version:
Keywords: PBF Cc:

Description

When deserializing a Blob from OSM PBF file the current version is relying on the serialization order. This crashes when deserializing OSM PBF files generated with ​Rust Prost library

The attached fix removes the dependency on the serialization order and initializes explicitly the inflator stream.

Attachments (2)

pbf-deserialization-fix.diff​ (3.4 KB ) - added by giora.kosoi@… 12 months ago.
Fix for deserialization error
niue-230109.osm.pbf​ (398.1 KB ) - added by giora.kosoi@… 12 months ago.
a sample osm.pbf of niue island that exibits the problem. osmosis --read-pbf ./niue-230109.osm.pbf --write-pbf ./osmosis-niue.osm.pbf works fine

Download all attachments as: .zip

Change History (11)

by giora.kosoi@…, 12 months ago

Attachment: pbf-deserialization-fix.diff​ added

Fix for deserialization error

comment:1 by taylor.smock, 12 months ago

@giora.kosoi: Can you attach a small sample PBF that exhibits the problem? Either that, or write a test that fails without the patch applied and passes with the patch applied.

by giora.kosoi@…, 12 months ago

Attachment: niue-230109.osm.pbf​ added

a sample osm.pbf of niue island that exibits the problem. osmosis --read-pbf ./niue-230109.osm.pbf --write-pbf ./osmosis-niue.osm.pbf works fine

comment:2 by giora.kosoi@…, 12 months ago

The attached niue-230109.osm.pbf​ reproduces the problem. It was generated by the Rust ​osm-io library that uses the ​prost library for PBF serialization.

Osmosis and osmium tools consume this file (niue-230109.osm.pbf) without errors. Tests on data content after consumption also indicate correct processing.

The current implementation of PBF deserialization in JOSM relies on the data field in Blog message to be packed after the raw_size field, while the ​PBF spec recommends not to assume any specific order.

The attached patch fixes the issue.

comment:3 by GerdP, 12 months ago

Just to mention: The tool osmconvert (version 0.8.10) also fails to read that file:

C:\Users\Gerd\Downloads>osmconvert niue-230109.osm.pbf > xxx.osm
osmconvert Error: block raw size expected at: 0x1A.

comment:4 by anonymous, 11 months ago

At a glance looks that osmconvert has a similar issue. The read loop expects fields to be in a predetermined order, contrary to PBF protocol recommendation.

Please see osmconvert.c version 0.8.11 line 4355 at ​http://m.m.i24.cc/osmconvert.c

comment:5 by *Martin*, 11 months ago

Could this be please merged?

comment:6 by taylor.smock, 11 months ago

It can be. I just need to find the time to write a non-regression test first. Which I'll be using attachment:niue-230109.osm.pbf​ for.

comment:7 by taylor.smock, 11 months ago

Resolution: → fixed
Status: new → closed

In 19024/josm:

Fix #23550: Error when deserializing PBF Blog (patch by giora.kosoi, modified)

Modifications are as follows:

  • Added non-regression test
  • Removed unnecessary changes in Blob

comment:8 by taylor.smock, 11 months ago

Milestone: → 24.03

comment:9 by anonymous, 11 months ago

Summary: [PATCH] Error when deserializing PBF Blog → [PATCH] Error when deserializing PBF Blob

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain team.
as The resolution will be set.
The resolution will be deleted. Next status will be 'reopened'.

Add Comment
E-mail address and name can be saved in the Preferences .
Attachments ↑ Description ↑
 
Note: See TracTickets for help on using tickets.