Skip to content

Define SpecCompliance enum for controlling spec strictness#2767

Merged
197g merged 2 commits intoimage-rs:mainfrom
telecos:feature/spec-compliance-api
Feb 22, 2026
Merged

Define SpecCompliance enum for controlling spec strictness#2767
197g merged 2 commits intoimage-rs:mainfrom
telecos:feature/spec-compliance-api

Conversation

@telecos
Copy link
Contributor

@telecos telecos commented Feb 12, 2026

This PR is originated after #2764 and #2684 where it was spotted the need to allow controlling strictness on the decoder for accepting files which may not be fully compliant to the spec. A solution was implemented to #2764 defaulting to lenient validation for BMP (which is also the case for JPEG and PNG for example). In #2684 it was spotted the need to allow forcing strictness of validation to allow control from the caller and a solution based on a boolean parameter was proposed. This PR is proposed a common solution for codecs allowing such control (at this point BMP and JPEG).

@telecos telecos marked this pull request as ready for review February 12, 2026 23:44
@mstoeckl
Copy link
Contributor

This PR is proposed a common solution for codecs allowing such control (at this point BMP and JPEG).

There is also HDR; it currently defaults to strict instead of lenient, but that may be worth changing:

pub fn new(reader: R) -> ImageResult<Self> {
HdrDecoder::with_strictness(reader, true)
}

@telecos
Copy link
Contributor Author

telecos commented Feb 20, 2026

This PR is proposed a common solution for codecs allowing such control (at this point BMP and JPEG).

There is also HDR; it currently defaults to strict instead of lenient, but that may be worth changing:

pub fn new(reader: R) -> ImageResult<Self> {
HdrDecoder::with_strictness(reader, true)
}

Thanks for pointing to this. I added a similar new_with_strictness ctor which receives the SpecCompliance enum as parameter. Also found that there was a "new_nostrict" and tagged it as deprecated suggesting usage of the new one.

@telecos telecos force-pushed the feature/spec-compliance-api branch from be592f0 to f9fe7e8 Compare February 21, 2026 21:44
@RunDevelopment
Copy link
Member

I think this is a good idea.

I want to mention that this feature would also be useful for plugin decoders. E.g. the (hopefully soon to be) DDS decoder in image-extras currently uses a permissive mode to parse DDS headers. So if this PR gets merged, I think #2672 should be updated to add a method like: ImageDecoder::set_spec_compliance(&mut self, compliance: SpecCompliance) -> ImageResult<()> (mirroring the setter for limits). This would allow plugins to take advantage of this setting too.

Copy link
Member

@197g 197g left a comment

Choose a reason for hiding this comment

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

Thanks for the added motivation. The main concerns holding this back in the past:

  • specialized to a small set of formats. I believe this was addressed by recent comments and potential use to image-extras. Adding it as an optional configuration to trait ImageDecoder should be doable as proposed.
  • unclear requirements. This is not resolved but for this specific case we have a clearly defined target; and for externally registered decoders it is outside our set of responsibilities.

I definitely like the convergence on one type for this instead of separate ones for different decoders. I see one potential problem with the trait interface in hdr which interprets strict in the constructor whereas a trait method would use set_strict(&mut self) so it's too late. I'll sort this out in the PR for it.

@197g 197g merged commit 4e85f6e into image-rs:main Feb 22, 2026
32 checks passed
@telecos telecos deleted the feature/spec-compliance-api branch February 23, 2026 12:03
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.

4 participants