Announcement

Symphony's issue tracker has been moved to Github.

Issues are displayed here for reference only and cannot be created or edited.

Browse

Closed#647: Upload field limit/error logic is wrong

Originally posted by michael-e on github:

Symphony has the following config (default) value for file uploads:

###### ADMIN ######
'admin' => array(
    'max_upload_size' => '5242880',
),
########

This value does not limit the size of uploads from the frontend. (I actually never knew this.) Instead it is used:

  1. as a default value for event example code (in content.blueprintsevents.php)

    (That might as well be hardcoded. It is only an example value.)

  2. as a hidden field in backend forms to limit upload size in the backend (content.publish.php)

    (That might be useful.)

  3. as one of two values in class.frontendpage.php to build the upload-limit page parameter:

    // Get max upload size from php and symphony config then choose the smallest
    $upload_size_php = ini_size_to_bytes(ini_get('upload_max_filesize'));
    $upload_size_sym = Symphony::Configuration()->get('max_upload_size','admin');
    

    (This is pure nonsense, because it will not work like this.)

  4. as error message value in upload fields, e.g. the core upload field:

    case UPLOAD_ERR_FORM_SIZE:
        $message = __('File chosen in "%1$s" exceeds the maximum allowed upload size of %2$s, specified by Symphony.', array($this->get('label'), General::formatFilesize(Symphony::Configuration()->get('max_upload_size', 'admin'))));
        break;
    

    (Again this is nonsense, because the actual upload limit will be set in the form as a hidden field.)

This needs a cleanup. It is nearly impossible for a normal user to undestand what happens. (And it took me while, too.)

I suggest to:

  • keep the config value as it is, but make it have an effect in the backend ("admin" context) only. This has been described as number 2.
  • number 1: hardcode the example value. Don't use the config value.
  • number 3: remove this param. It does not make any sense, because actually the upload limit will be set by either the php.ini value or hidden form fields.
  • number 4: Remove it from the error message, try to use the MAX_FILE_SIZE value instead (otherwise drop the number from the error message). Then send pull requests to upload field extensions which use a similar same error message logic!

Should I try and care for this? Or do you think that we might go for a different solution?

(Hey, if somebody finds the time, I would be really glad if he could care for this. My current project is really exhausting.)

  1. I don't see there is anything to gain by just setting this to 1234 when we can use the configuration parameter as a base example (that if a user has bothered to change they are probably expecting what you expected)
  2. Yep that's correct.
  3. Not entirely true. If you are creating MAX_FILE_SIZE fields in your forms, you should be using this parameter as it will return the effective limit. If you look a little further down in the class, you'll notice it min's the two values, so the correct limit is exposed. Perhaps this needs to happen for the example event markup too (1.). If you are setting the MAXFILESIZE to something greater than the server limit (ie. ignoring this parameter), that will fail regardless of what Symphony does.
  4. Related to 3 I guess.

I think the main thing to come from your findings is that it needs to be documented/exposed so that we all know what is going on! $upload-limit is useful because it's exposing the maximum file size that the server will accept. If the Symphony setting is less than the server's, fine that will be enforced, otherwise it'll abide to the server's setting.

1) probably needs the most attention, it should implement the same check as 3)

Fixed the inconsistency between 1) and 3) in this commit. I guess we now know why max_upload_size sits inside ADMIN in the manifest and not SYMPHONY ;)

I will test it and report back.

At the moment I still think that whisking several values is against the "don't be clever" philosophy, because people won't understand it.

People don't have to understand it, they just can trust it. If the param was removed, how would a dev know what the max file size is? Its a non issue imo

I tested this again.

In the backend you have two possible errors:

  • File chosen in "Unique File Upload" exceeds the maximum allowed upload size of 2.5M specified by your host.
  • File chosen in "Unique File Upload" exceeds the maximum allowed upload size of 5.00 MB, specified by Symphony.

I was never aware of the fact that I should/could use the upload-limit page parameter in my frontend forms in order to have the same (dynamic) limit when POSTing from the frontend! So instead of using

<input name="MAX_FILE_SIZE" type="hidden" value="5242880" />

I could use

<input name="MAX_FILE_SIZE" type="hidden" value="{//params/upload-limit}" />

Right?

(One might add this possibility to the event documentation. Maybe this would be a nice task for the documentation/UX working group?)

Without any hidden form field the only file size error in the frontend is:

  • File chosen in "Unique File Upload" exceeds the maximum allowed upload size of 2.5M specified by your host.

So the configuration value has no effect on the event itself. That is fine.

I think we can close this issue.

I was never aware of the fact that I should/could use the upload-limit page parameter in my frontend forms in order to have the same (dynamic) limit when POSTing from the frontend!

Neither was I until I investigated this issue, I have to say we don't often use the MAXFILESIZE field on our sites.

You can also use $upload-limit (although you probably already guessed that)

(One might add this possibility to the event documentation. Maybe this would be a nice task for the documentation/UX working group?)

Agreed, perhaps we need the idea of 'quick tips' instead of full blown articles?

This issue is closed.

Symphony • Open Source XSLT CMS

Server Requirements

  • PHP 5.3-5.6 or 7.0-7.3
  • PHP's LibXML module, with the XSLT extension enabled (--with-xsl)
  • MySQL 5.5 or above
  • An Apache or Litespeed webserver
  • Apache's mod_rewrite module or equivalent

Compatible Hosts

Sign in

Login details