Skip to content

Prevent loading images that would require too much memory. - #30291

Merged
nickvergessen merged 1 commit into
masterfrom
image-memory-limit
Jan 11, 2022
Merged

Prevent loading images that would require too much memory.#30291
nickvergessen merged 1 commit into
masterfrom
image-memory-limit

Conversation

@fancycode

@fancycodefancycode commented Dec 16, 2021•
edited
Loading

Copy link
Copy Markdown
Member

For most image formats, the header specifies the width/height. PHP allocates an image object from that size, even if the actual
image data is much smaller. This image object size is not limited by the limit configured in PHP.

The memory limit can be configured through config setting "preview_max_memory" and defaults to 128 MBytes which should be enough for most images without filling up all memory.

Signed-off-by: Joachim Bauch bauch@struktur.de

@fancycode
fancycodeforce-pushed the image-memory-limit branch 3 times, most recently from 9d98b3b to 87c4ba5CompareDecember 16, 2021 09:40
@solracsfsolracsf added the 3. to review Waiting for reviews label Dec 16, 2021
nickvergessen
nickvergessen reviewed Jan 3, 2022
View reviewed changes
Comment threadlib/private/legacy/OC_Image.php Outdated
@nickvergessen
nickvergessen requested review from a team, come-nc, nickvergessen and skjnldsv and removed request for a teamJanuary 3, 2022 14:10
@nickvergessennickvergessen added this to the Nextcloud 24 milestone Jan 3, 2022
blizzz
blizzz reviewed Jan 4, 2022
View reviewed changes
Comment threadlib/private/legacy/OC_Image.php Outdated
@skjnldsv

skjnldsv commented Jan 10, 2022

Copy link
Copy Markdown
Member

Hey @fancycode , could you address the comments ? :)
Happy new year! 🎉

@fancycode

fancycode commented Jan 10, 2022

Copy link
Copy Markdown
MemberAuthor

Hey @fancycode , could you address the comments ? :) Happy new year! tada

Happy new year to you, too. I just came back from vacation today and are going through my inbox. Will address the comments as soon as possible.

@fancycode

fancycode commented Jan 10, 2022

Copy link
Copy Markdown
MemberAuthor

All comments addressed. Commits should be squashed and the text of the first commit should be adjusted (limit is now read from config.php instead of the app config) - I can do this before merging.

@fancycode
Prevent loading images that would require too much memory.
d3d65e5
For most image formats, the header specifies the width/height.
PHP allocates an image object from that size, even if the actual
image data is much smaller. This image object size is not limited
by the limit configured in PHP.
The memory limit can be configured through "config.php" setting
"preview_max_memory" and defaults to 128 MBytes which should be
enough for most images without filling up all memory.
Signed-off-by: Joachim Bauch <bauch@struktur.de>
@fancycode

fancycode commented Jan 11, 2022

Copy link
Copy Markdown
MemberAuthor

Commits are squashed and the message is updated, so this is ready to merge from my side.

@blizzzblizzz added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Jan 11, 2022
@nickvergessen
nickvergessen merged commit c47406a into masterJan 11, 2022
@nickvergessen
nickvergessen deleted the image-memory-limit branch January 11, 2022 12:35
@fancycode

fancycode commented Jan 11, 2022

Copy link
Copy Markdown
MemberAuthor

Thanks for merging. Probably would be good to also backport this change to all versions that still receive security updates.

@nickvergessen

nickvergessen commented Jan 11, 2022

Copy link
Copy Markdown
Member

/backport to stable23

@nickvergessen

nickvergessen commented Jan 11, 2022

Copy link
Copy Markdown
Member

/backport to stable22

@nickvergessen

nickvergessen commented Jan 11, 2022

Copy link
Copy Markdown
Member

/backport to stable21

@backportbot-nextcloudbackportbot-nextcloudBot mentioned this pull request Jan 11, 2022
Merged
This was referenced Jan 11, 2022
Merged
Merged
@solracsfsolracsf mentioned this pull request Jan 23, 2023
Closed
8 tasks
@joshtrichardsjoshtrichards mentioned this pull request Jan 1, 2025
Closed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nickvergessennickvergessennickvergessen approved these changes
@ChristophWurstChristophWurstChristophWurst approved these changes
@blizzzblizzzblizzz approved these changes
@LukasReschkeLukasReschkeAwaiting requested review from LukasReschke
@skjnldsvskjnldsvAwaiting requested review from skjnldsv
@come-nccome-ncAwaiting requested review from come-nc

Assignees

No one assigned

Labels

4. to releaseReady to be released and/or waiting for tests to finishbugfeature: previews and thumbnails

Projects

None yet

Milestone

Nextcloud 24

Development

Successfully merging this pull request may close these issues.

6 participants

@fancycode@skjnldsv@nickvergessen@ChristophWurst@blizzz@solracsf