Skip to content

catch stoul exceptions in CanonMakerNote print0x000a/print0x000c#9320

Merged
kevinbackhouse merged 3 commits into
Exiv2:mainfrom
netliomax25-code:canon-print-stoul-catch
Jul 1, 2026
Merged

catch stoul exceptions in CanonMakerNote print0x000a/print0x000c#9320
kevinbackhouse merged 3 commits into
Exiv2:mainfrom
netliomax25-code:canon-print-stoul-catch

Conversation

@netliomax25-code

Copy link
Copy Markdown
Contributor

print0x000a and print0x000c call std::stoul(value.toString()), but the stored type of Canon tags 0x000a and 0x000c is not enforced during parsing, so a crafted makernote can hand them a non-numeric ASCII value. stoul then throws std::invalid_argument, which Exifdatum::write does not catch (it only guards std::out_of_range), so it escapes during -pa printing. Catch both and fall back to the raw value, like the existing path in print0x000c.

@kevinbackhouse

Copy link
Copy Markdown
Collaborator

@mergify backport 0.28.x 0.29.x

@mergify

mergify Bot commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

backport 0.28.x 0.29.x

✅ Backports have been created

Details

Cherry-pick of f62ab78 has failed:

On branch mergify/bp/0.28.x/pr-9320
Your branch is up to date with 'origin/0.28.x'.

You are currently cherry-picking commit f62ab784.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   src/canonmn_int.cpp

no changes added to commit (use "git add" and/or "git commit -a")

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

Comment thread src/canonmn_int.cpp Outdated
Comment on lines +2720 to +2723
} catch (const std::invalid_argument&) {
return os << value;
} catch (const std::out_of_range&) {
return os << value;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
} catch (const std::invalid_argument&) {
return os << value;
} catch (const std::out_of_range&) {
return os << value;
} catch (const std::logic_error&) {
return os << value;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good catch, both derive from std::logic_error so one catch covers them. Switched both functions over and pushed.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

You used std::logic_error in #9321, so why not here? It makes me think that this is AI-generated code.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That crossed with my push. I'd already collapsed both functions here to std::logic_error a bit earlier, so it now matches #9321. The split was just me writing the two PRs at separate times before going back to tidy this one. I do lean on an LLM now and then for sanity-checking, but the reasoning here is plain: stoul throws either invalid_argument or out_of_range, both under logic_error, and Exifdatum::write only catches out_of_range, so the other one was escaping.

Comment thread src/canonmn_int.cpp Outdated
Comment on lines +2739 to +2742
} catch (const std::invalid_argument&) {
return os << value;
} catch (const std::out_of_range&) {
return os << value;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
} catch (const std::invalid_argument&) {
return os << value;
} catch (const std::out_of_range&) {
return os << value;
} catch (const std::logic_error&) {
return os << value;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Same here, collapsed to std::logic_error in print0x000c too.

@mergify

mergify Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

  • Queue this pull request

@kevinbackhouse kevinbackhouse merged commit 4ff7a97 into Exiv2:main Jul 1, 2026
103 of 106 checks passed
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