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

[4.0] php memory limit #35854

Closed
wants to merge 2 commits into from
Closed

[4.0] php memory limit #35854

wants to merge 2 commits into from

Conversation

brianteeman
Copy link
Contributor

It is possible to set the memory limit in php to -1 which means unlimited memory.

Unfortunately this was not accounted for in the maths to check if a file could be uploaded.

To test you will need to change the value in your php.ini to memory_limit = -1 and restart the web server.

Then try to upload any file in the media manager and you will see an error message that the file is too large.

Apply this PR and the upload will be successful.

PR for #35853

It is possible to set the memory limit in php to -1 which means unlimited memory.

Unfortunately this was not accounted for in the maths to check if a file could be uploaded.

To test you will need to change the value in your php.ini to `memory_limit = -1` and restart the web server.

Then try to upload any file in the media manager and you will see an error message that the file is too large.

Apply this PR and the upload will be successful.

PR for joomla#35853
@rjharishabh
Copy link
Contributor

I have tested this item ✅ successfully on 43ccd32


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

@richard67
Copy link
Member

I have tested this item ✅ successfully on 6f34162


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

@richard67
Copy link
Member

@rjharishabh Could you test again? Thanks in advance.

@rjharishabh
Copy link
Contributor

I have tested this item ✅ successfully on 6f34162


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

@richard67
Copy link
Member

RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Nov 1, 2021
@@ -158,13 +158,13 @@ public function getItems()
'description' => Text::_('COM_INSTALLER_MSG_WARNINGS_UPLOADBIGGERTHANPOSTDESC'));
}

if ($post_max_size < $minMemory)
if ($post_max_size < $minMemory && $memory_limit != -1)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this check correct? Does unlimited memory_limit make PHP to ignore post_max_size & upload_max_filesize?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it is the same as other checks in the same file - it was just missing here

Copy link
Contributor

Choose a reason for hiding this comment

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

If I see it correct, they are different checks, the other which exist looking for the memory_limit, the two you changed are checking the `` post_max_sizeandupload_max_size```

Copy link
Contributor Author

Choose a reason for hiding this comment

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

All I can say is that I replicated the reported problem and this change resolved the reported problem

Copy link
Member

Choose a reason for hiding this comment

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

Sure it was the changes in this file here which solved it, i.e. the change in the other file below was not enough?

Copy link
Member

@HLeithner HLeithner left a comment

Choose a reason for hiding this comment

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

Check upload, post and memory limit if set to unlimited

@HLeithner
Copy link
Member

Check upload, post and memory limit if set to unlimited

I tried visual code on github and completely failed... so I write it down here.

Benjamin is right your changes to $post_max_size and $upload_max_filesize is wrong, you have to check this 2 variables to be bigger then 0 (not -1).

also you have to add this check to the ApiController, because it's possible to set the upload size and post size to -1 or 0 (both unlimit)

@brianteeman brianteeman deleted the rtl branch November 13, 2021 21:27
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Nov 13, 2021
@richard67
Copy link
Member

I’ve re-opened the issue.

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.

None yet

7 participants