Return an error for malformed JPEG input - #1096
Open
ilia-sokolov wants to merge 1 commit into
Open
Conversation
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens the JPEG decoding path in DecodeImage so malformed or truncated JPEG inputs return an OrtxStatus instead of potentially terminating the host process via libjpeg’s default fatal error handler.
Changes:
- Added a libjpeg error manager that uses
setjmp/longjmpto convert fatal libjpeg errors intoOrtxStatusfailures. - Implemented a non-suspending in-memory JPEG source manager that treats truncation and out-of-bounds marker skips as corrupt data.
- Expanded test coverage to ensure invalid JPEG inputs fail without process termination and that decoding still works after repeated failures.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
operators/vision/image_decoder.hpp |
Reworks JPEG decode to override libjpeg fatal error handling, keep mutable libjpeg state on the heap, and improve truncation/skip handling. |
test/pp_api_test/test_imgcodec.cc |
Adds regression tests asserting invalid JPEGs return errors (not process exit) and that valid JPEG decoding still succeeds after failures. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+222
to
+225
| auto* const state = | ||
| new JpegDecodeState(encoded_image_data, encoded_image_data_len); | ||
| state->cinfo.err = jpeg_std_error(&state->error.base); | ||
| state->error.base.error_exit = &JpegErrorManager::ErrorExit; |
Comment on lines
+6
to
9
| #include <csetjmp> | ||
| #include <cstdint> | ||
| #include <string> | ||
|
|
Comment on lines
+34
to
+37
| static void ErrorExit(j_common_ptr cinfo) { | ||
| auto* error = reinterpret_cast<JpegErrorManager*>(cinfo->err); | ||
| (*cinfo->err->format_message)(cinfo, error->message); | ||
| longjmp(error->jump_buffer, 1); |
| {0xFF, 0xD8, 0xFF, 0xE1, 0x7F, 0xFF, 0x00, 0x00, 0xFF, 0xD9}, | ||
| }; | ||
|
|
||
| for (auto encoded : invalid_images) { |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
OrtxStatussetjmp/longjmpdoes not leave modified automatic C++ state indeterminateRoot cause
The generic decoder installed
jpeg_std_errorwithout overridingerror_exit. Fatal libjpeg errors therefore used the library's default handler, which can terminate the host process instead of returning an inference error.Validation
A Linux build with the portable libjpeg backend passed all six image-decoder tests:
git diff --checkalso passed.Fixes #1094