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

Remove PHP memory_limit from max upload size calculation #16741

Merged
merged 1 commit into from Jun 17, 2017

Conversation

Projects
None yet
7 participants
@SniperSister
Contributor

SniperSister commented Jun 17, 2017

Summary of Changes

With this PR I removed the current value for memory_limit from the max_upload_size calculation in JUtility::getMaxUploadSize. The memory_limit does NOT affect the maximum upload size of files as files are handled as streams which have a very low memory impact, therefore the value should not be taken into account.

Testing Instructions

We stumbled upon this issue while running the Joomla unit test suite in cli environment with memory_limit = -1 which means "unlimited memory".
Because of the way how the memory calculation in JUtility works, -1 is returned as the "smallest" and therefore limiting value, causing the unit test to fail.

So, in order to test this PR, you should run the JFormFieldFileTest (which uses JUtility in it's rendering) in a CLI environment with memory_limit = -1 - you'll get an error like this:

There was 1 error:

  1. JFormFieldFileTest::testGetInput
    Undefined offset: -9223372036854775808

Afterwards, apply the patch and re-run the test, it will work now.

Expected result

The test suite should work.

Actual result

Error message, see above

Documentation Changes Required

none

@yvesh

yvesh approved these changes Jun 17, 2017

@yvesh

This comment has been minimized.

Member

yvesh commented Jun 17, 2017

I have tested this item successfully on 96c30bd


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/16741.

1 similar comment
@rdeutz

This comment has been minimized.

Contributor

rdeutz commented Jun 17, 2017

I have tested this item successfully on 96c30bd


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/16741.

@zero-24 zero-24 added this to the Joomla 3.7.3 milestone Jun 17, 2017

@zero-24 zero-24 merged commit 2cc0efa into joomla:staging Jun 17, 2017

5 checks passed

JTracker/HumanTestResults Human Test Results: 2 Successful 0 Failed.
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/drone/pr the build was successful
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
hound No violations found. Woof!
@zero-24

This comment has been minimized.

Contributor

zero-24 commented Jun 17, 2017

Merged Thanks!

@bobby375

This comment has been minimized.

bobby375 commented Jul 6, 2017

I get the error "Maximum PHP file upload size is too small" and cannot upload the template files.

@franz-wohlkoenig

This comment has been minimized.

franz-wohlkoenig commented Jul 6, 2017

Please ask help on the forums. This repository concerns Joomla coding.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment