Skip to content

Fix: Replace invalid UncheckedIOException usage in BinaryParser - #90

Closed
Manas-Dikshit wants to merge 2 commits into
openstreetmap:masterfrom
Manas-Dikshit:master
Closed

Fix: Replace invalid UncheckedIOException usage in BinaryParser#90
Manas-Dikshit wants to merge 2 commits into
openstreetmap:masterfrom
Manas-Dikshit:master

Conversation

@Manas-Dikshit

Copy link
Copy Markdown

This PR fixes a compile-time error in BinaryParser.java caused by improper wrapping of a non-IOException inside UncheckedIOException.

Details
Removed: throw new UncheckedIOException(new FileFormatException(e));
Added: throw new FileFormatException(e);
FileFormatException already extends RuntimeException, making it suitable to throw directly.
Also added a small safety check in getStringById() to prevent potential ArrayIndexOutOfBoundsException for malformed PBF data.

Impact
Fixes compilation errors in the Java build (mvn package or ant).
Improves runtime safety and error clarity when parsing malformed OSM PBF files.

@joto

joto commented Dec 3, 2025

Copy link
Copy Markdown
Collaborator

Please don't mix unrelated changes in a single PR. In this case the code changes with unrelated whitespace or wording changes. This makes is very hard to review the change set.

@simon04

simon04 commented Jul 22, 2026

Copy link
Copy Markdown
Member

The PR touches one file with +99 / −63, but only two lines of that are functional changes:

  1. The "fix" (the PR's headline change):
- throw new UncheckedIOException(new FileFormatException(e));
+ throw new FileFormatException(e);

So throw new FileFormatException(e) inside handleBlock is an unreported checked exception — it would break compilation. The original throw new UncheckedIOException(new FileFormatException(e)) exists precisely to wrap the checked exception so it can propagate. The PR would introduce the compile error it claims to fix.

  1. A bounds check in getStringById():
+ if (id < 0 || id >= strings.length) return null;

Close this PR?

@joto joto closed this Jul 24, 2026
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.

3 participants