Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Policy for uncommon formats implemented via third-party crates #2363

Open
Shnatsel opened this issue Oct 25, 2024 · 9 comments
Open

Policy for uncommon formats implemented via third-party crates #2363

Shnatsel opened this issue Oct 25, 2024 · 9 comments

Comments

@Shnatsel
Copy link
Contributor

The problem

There is a number of formats that are implemented as third-party crates, but are not supported by image:

JPEG XL: https://crates.io/crates/jxl-oxide
RAW: https://crates.io/crates/rawloader or https://crates.io/crates/quickraw
PSD subset: https://crates.io/crates/zune-psd or https://crates.io/crates/psd
ICNS: https://crates.io/crates/icns
JPEG 2000: https://crates.io/crates/openjp2 (not sure if the port is complete enough just yet)

None of these formats have specifications available publicly and for free. As per #1979, right now they are not supported by image at all.

This leads to each API users that needs them reimplementing the integrations into image. oculante has already done this, and I'm going to have to do this in wondermagick too.

Possible solutions

Adding image integrations to those crates (like it's done for libwebp) is a step in the right direction, but doesn't really solve this because the API is still quite awkward: you cannot just use reader.with_guessed_format().decode()?, the API user has to explicitly list every format and also deal with magic numbers for them.
Also, when image ships a new semver-incompatible version, every integration would have to bump their semver as well, and it could take a long while until all the format integrations are upgraded.

A solution that seems promising to me is a tiered format support, where we specifically advertise third-party crates listed above as Tier 2. We clearly state that we do not accept bug reports about them and that any issues should go to the third-party crate's issue tracker. These formats would also be disabled by default, with explicit opt-in required to enable them.
This avoids the issues with the other option, but still adds some (hopefully small) additional maintenance burden for the in-tree integration of the third-party crate.

Thoughts? Any other good options I've missed?

@aschampion
Copy link
Contributor

Not sure whether to comment on this or #2367 or #2368, but I'm glad this discussion is happening. I think any policy about new formats should depend on all three issues, specifically the idea of tiering. For example:

  • Tier 1: popular/canonical formats only, image- crate controlled by the image org, some reasonable degree of upstream/supply chain trust, strong Rust backend preference, included in default feature set (default-formats), fuzzed to a reasonable effort. When you include image by default you're only getting this tier and have a reasonable assurance of usefulness and security.
  • Tier 2: preferably a "real" format by some fuzzy definition (not hobby, though with legacy exceptions), image- crate controlled by the image org, more permissive of new maintainers and unmatured implementations, not included in the default feature flags, tested but not necessarily fuzzed.
  • Tier 3: useful 3rd party crates like those listed above with an in-tree integration, not included in the default feature flags, tested but not necessarily fuzzed.
  • Tier 4: the bazaar of anything in the wild that does not meet the above definitions, can only be included by users as a static hook or dynamic dispatch as discussed in other issues, may be included in image CI testing as a fail-allowed smoke test if it's popular enough, using a new API we want to mature, or on its way to tier 3.

Such a tiering could be adopted over two major releases, i.e., first release assigns tiers to existing formats with warnings and the second fully adopts the policy. So for example if farbfeld is determined to be Tier 2, on the first release users get a deprecation warning if they're using the type/variant without the explicit farbfeld format feature flag, on the second release it breaks.

I think this is similar to your proposal, but with an intermediate tier 2 that allows for first-party but less-polished or -assured formats, with the third-party formats in tier 3 and an explicit notion of truly external formats in tier 4.

This also preserves the usefulness of some things like farbfeld (which keeps coming up in discussion of inconsistent inclusion criteria), as being simple, illustrative example of how to implement a format in the image ecosystem, without that causing thousands of dependent crates to have this random format by default.

I'm not actively contributing to image at the moment, but am concerned with external format crates, and in general having a cohesive policy for formats that respects the quality and security goals of the core image crate while having the ergonomics to support a 3rd-party and external format ecosystem.

@fintelia
Copy link
Contributor

Agreed that having tiers is a helpful frame for different levels of format support. I'd probably break up formats into more categories than just real vs. hobby though.

  • Major formats -- Formats like PNG, JPEG, GIF, TIFF, etc.
  • Specialized and obscure formats -- Less common than the major formats but used because they outperform the major formats on some dimension or because they're the chosen standard in some field.
  • Emerging formats -- Recently created formats likely to land in one of the prior categories. Frequently accompanied by a large marketing push to pressure software makers to include the format so they'll gain traction.
  • Obsolete formats -- Before PNG was widely used, dozens of different software packages created their own lossless image formats. These are "real" formats in that they were used by real shipping software, but these days are broadly considered obsolete. They tended to either be uncompressed or only use some bespoke RLE compression scheme. IMO, in almost all cases, anyone who still has images in one of these formats should probably just convert them to PNG and move on. The description of PFM includes the line that the format came from "a program he found somewhere" (without any indication of what program or where it was found!), which to me really sums up how the bar was for new formats.
  • Hobby formats -- Something Farbfeld or QOI that is relatively new but not backed by a major company or coalition.
  • Experimental formats -- Stuff like FLIF or WebP 2 that aren't intended for broader use. Probably the clearest set of formats not worth supporting.

There's also enormous differences in implementation complexity between formats. Most of the obsolete and hobby formats are extremely simple.

@fintelia
Copy link
Contributor

Another question is in-tree vs. out-of-tree.

I'm starting to think that we might want to make an image-extras crate to hold Tier 3 formats rather than accepting a large number of additional bindings and optional dependencies into image. Disabled-by-default formats theoretically shouldn't cause issues, but we've already had one kind of serious bug in our Cargo.toml from a seemingly unrelated edit. A separate crate would sharply reduce the risk and if we get external hooks support right, the location of format bindings shouldn't make a big difference.

I should also add that the exact definitions of different tiers would have to be tuned. Our jpeg decoder is currently backed by zune-jpeg, but I don't think that should degrade it to tier 3 or even tier 2. And even image-png uses simd-adler32 which is an external crate. Similarly hoping for all our enabled-by-default codecs to be fuzzed is good in principle, but at the moment there's no one interested in doing the bug triage and fixing work associated with that.

@HeroicKatora
Copy link
Member

HeroicKatora commented Oct 28, 2024

Does anyone feel knowledgeable enough about ffmpeg and/or imagemagick, to describe how their format support discovery mechanism works? If there is a form of dynamic loading then this is out-of-scope for now (we need much better language agnostic interfaces in practice, that do not require code-loading and/or pipes to a forked process imo); but the remainder of this mechanism and their interfaces for policy would be worthwhile looking at.

@fintelia
Copy link
Contributor

I'm not sure about those two libraries, but I looked into the file tool a while back. IIRC it has the different formats controlled via a config file that has essentially bytecode snippits for each format. Many just amount to "match the first N bytes against this constant", but it also can do fancier things like "read the next two bytes as a little endian integer and jump forward that many bytes"

@awxkee
Copy link
Contributor

awxkee commented Nov 4, 2024

Does anyone feel knowledgeable enough about ffmpeg and/or imagemagick, to describe how their format support discovery mechanism works? If there is a form of dynamic loading then this is out-of-scope for now (we need much better language agnostic interfaces in practice, that do not require code-loading and/or pipes to a forked process imo); but the remainder of this mechanism and their interfaces for policy would be worthwhile looking at.

Magick have a fixed list of methods that invoked at start.
If core compiled against module plugin adds info about itself, otherwise skipping it.
Then it enumerates Modules that registered info about themselves, and it is trying to load shared libs.
Not sure if it is helpful in Rust.

@kornelski
Copy link
Contributor

Each format registers a magic callback function, and each format checks for itself. I've checked a few, and they don't look sophisticated, e.g. https://github.com/ImageMagick/ImageMagick/blob/main/coders/svg.c#L232

@sorairolake
Copy link
Contributor

Is there a possibility to split out the low-level handling of image file formats (e.g., Farbfeld) which are implemented directly in this crate into separate crates (like image-webp)?

@Shnatsel
Copy link
Contributor Author

Shnatsel commented Nov 6, 2024

Yes. Although most formats that are currently bundled are so simple that splitting them out didn't really make sense.

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

No branches or pull requests

7 participants