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.

Sign up for GitHub

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

Jump to bottom

Fix setting GIF limits, implement limits for GIF animations #2090

Merged
merged 18 commits into from
Jan 12, 2024
Merged

Fix setting GIF limits, implement limits for GIF animations #2090

merged 18 commits into from
Jan 12, 2024

Conversation

Shnatsel
Copy link
Contributor

@Shnatsel Shnatsel commented Jan 9, 2024

  1. Fix .set_limits() on the GIF decoder to actually set limits
  2. Implement limit tracking in the GIF animation decoder, so APIs such as .into_frames() now also obey both image size and allocation limits, including for intermediate buffers
  3. Add tests to verify limits in animation work correctly, which found (1)

At long last, fixes #1052

Shnatsel added 16 commits January 9, 2024 02:33
@Shnatsel
Add GIF test files to be used in resource limit tests
49a13c4
@Shnatsel
gif: wire up limit checks to non_disposed_frame allocation
d552f9a
@Shnatsel
Factor out limits check into a function, add a limit check for the fr…
7779a00 
…ame buffer
@Shnatsel
Add limit checks to the third and final allocation in GIF decoding
aea2f82
@Shnatsel
Drop redundant comment
fa80842
@Shnatsel
Delete an animation test file, turns out it's not needed
748b114
@Shnatsel
Change test image dimensions to 1000*1000 for simplicity
e36e924
@Shnatsel
Fix memory limit reservations being too low
e5745bb
@Shnatsel
Add a test image
691affe
@Shnatsel
Add WIP test for limits in animated images
32862d0
@Shnatsel
Implement set_limits for GifDecoder; previously it didn't actually se…
01f484d 
…t the limits field at all
@Shnatsel
Comment
f6a70ca
@Shnatsel
Add another GIF decoding limit test
fae515d
@Shnatsel
Follow Clippy
83bce03
@Shnatsel
Correctly feature-gate animation limit tests
228f307
@Shnatsel
Do not set any memory limit in GifDecoder::new() to avoid breaking ex…
9edafbf 
…isting users
kornelski
kornelski reviewed Jan 9, 2024
View reviewed changes
src/codecs/gif.rs Outdated Show resolved Hide resolved
fintelia
fintelia reviewed Jan 9, 2024
View reviewed changes
src/codecs/gif.rs Outdated Show resolved Hide resolved
@fintelia fintelia merged commit d55be71 into image-rs:master Jan 12, 2024
35 checks passed
@Shnatsel Shnatsel mentioned this pull request Jan 16, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fintelia fintelia fintelia left review comments

@kornelski kornelski kornelski approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Large CPU and memory consumption on decoding a crafted GIF file
3 participants
@Shnatsel @kornelski @fintelia