Project

General

Profile

Actions

Bug #15220

closed

Filesize of maximum allowed files in mailforms

Added by Lars Houmark over 18 years ago. Updated almost 14 years ago.

Status:
Closed
Priority:
Should have
Category:
Frontend
Target version:
-
Start date:
2005-11-18
Due date:
% Done:

0%

Estimated time:
TYPO3 Version:
3.8.1
PHP Version:
4
Tags:
Complexity:
Is Regression:
Sprint Focus:

Description

In around line 142 in the file /t3lib/class.t3lib_formmail.php the size of the maximum allowed file size for uploades files in a mailform is hardcoded.

This should be configurable, either in the Install tool or in som TS config.

Simply put a mailform on a site, trying to upload a file of size more than 250 kb and there's trouble.

May I at the same time suggest that the default value is either somewhat higher and maybe taken from another upload variable, if not changed in Install tool or TS.
(issue imported from #M1900)


Files

bug_1900.patch (3.49 KB) bug_1900.patch Administrator Admin, 2005-11-19 10:50

Related issues 3 (0 open3 closed)

Has duplicate TYPO3 Core - Bug #15349: Maximum file size for file uploads in mailforms still hardcodedClosedSebastian Kurfuerst2006-01-03

Actions
Has duplicate TYPO3 Core - Feature #15733: mailform and hardcoded attachment-filesizeClosedSebastian Kurfuerst2006-02-27

Actions
Is duplicate of TYPO3 Core - Bug #17036: Still 250000 bytes maximum size is hardcoded for mailform attachmentsClosedErnesto Baschny2007-02-24

Actions
Actions #1

Updated by Sebastian Kurfuerst over 18 years ago

Hi,
I think an install tool configuration would be a good idea.
Greets, Sebastian

Actions #2

Updated by Sebastian Kurfuerst over 18 years ago

Hi,
I didn't test this patch, it would be a big help if you could check if it works as expected.

Greets, Sebastian

Actions #3

Updated by Lars Houmark over 18 years ago

Sebastian,

Thanks for your rapid response.

I will find time to test your patch within the next few days.

I am just a bit concerned for the "if not set" statement, that would unlimit the upload size.

Also I would love if the standard size would be larger than 250kb, as this is not very much these days. If this has to stay as is, I would have to either config EVERY current site or make an extension to enlarge the value and install that on every site.

Thanks,

Lars

Actions #4

Updated by Sebastian Kurfuerst over 18 years ago

"I am just a bit concerned for the "if not set" statement, that would unlimit the upload size."
I planned it like this, but maybe it is better when there will be no unlimited upload size, dunno. Can this be a DoS or security rist? Any comments?
Greets, Sebastian

Actions #5

Updated by Lars Houmark over 18 years ago

Maybe just a "0" value in the field could make it unlimit, which would require the user to put in a zero. Emptying it just seems very easy to me.

Security, hmmmm. Well, it's certaintly not very nice to be emailed a file sized ie. 100 MB.
Also, if someone finds it possible to upload very large files, and is having a very strong connection, he could open many requests and by that upload many files of big sizes, where this seems very unlikely to make any sence one can only upload 1-2 MB files.
It is though, a very little security issue, which I think we should not be concerned about.

Actions #6

Updated by Lars Houmark over 18 years ago

Hello Sebastian,

I have now tested your patch. It's working just fine.

We were discussing how the check should be. I'll leave it up to you, as I am sure that you find the best method and the core team will vote for the correct way ;)

I am not sure if there should be a notification if a file is larger that the allowed upload size. Maybe some line should be added in the bottom of the mail or the addAttachment function should be modified to be able to put out a text message instead of including the file, if the file is to large.
The reciever won't know that the sender was trying to attach a file, so this makes great sence IMHO.

While looking at this, I discovered that one formular seems to have a max. amount of attachments of 10. I don't believe this is documented and checked for in the backend. Maybe the for loop should be modified to run until all files have been attached.

What do you think?

Actions #7

Updated by Lars Houmark over 18 years ago

And while we're at it. Maybe the administrator should be able to control which file endings should be allowed for uploads. This is a new feature though.

Actions #8

Updated by Sebastian Kurfuerst over 18 years ago

Nice idea. Is it possible for you to write a patch? That would be of great help.

Greets, Sebastian

Actions #9

Updated by Lars Houmark over 18 years ago

Good idea for which of them? ;)

I've looked more into it.

1. It's rather difficult to count the amount of attachments, because of the namespace used: attachmentx, so I would just suggest letting the loop run for 100 instead of 10. This shouldn't be that slow, if they are all empty anyways.

2. The error message should be localized, but there is no such in the mailer, which would make the error message english only.

Actions #10

Updated by Sebastian Kurfuerst over 18 years ago

Hi Houmark,
nice to see you are so active here!

"And while we're at it. Maybe the administrator should be able to control which file endings should be allowed for uploads. This is a new feature though."
if you could write a patch for this, that'd be great.

"I am not sure if there should be a notification if a file is larger that the allowed upload size. Maybe some line should be added in the bottom of the mail or the addAttachment function should be modified to be able to put out a text message instead of including the file, if the file is to large.
The reciever won't know that the sender was trying to attach a file, so this makes great sence IMHO."
Yes, this is a good addition. Can you add the string to locallang as well to make it configurable?

"While looking at this, I discovered that one formular seems to have a max. amount of attachments of 10. I don't believe this is documented and checked for in the backend. Maybe the for loop should be modified to run until all files have been attached."
Yes, makes sense. Don't change this to 100, it would be more clean to check if the correct key exists - and if not, abort the loop.
If you could write a patch for this as well, that would help a lot.

Greets, Sebastian

Actions #11

Updated by Sebastian Kurfuerst over 18 years ago

Needs to be changed to TS property, and disable "0".
Greets, Sebastian

Actions #12

Updated by Marc Bastian Heinrichs over 18 years ago

Hi Sebastian,

is still hardcoded in beta2...

Greets,
Marc Bastian

Actions #13

Updated by Sebastian Kurfuerst about 18 years ago

Hi,
I discussed it with other core developers and they think the right way would be a TypoScript setting for the mailform. I really agree with that, this is the place where it should be configurable.
However, implementing such a TS property is quite hard due to the mailform handling in TYPO3. A patch is welcome, I currently don't have time to look into this.

Sorry and greets, Sebastian

Actions #14

Updated by Felipe Knaesel over 17 years ago

Almost 1 year ever since and even not a simple localconf.php var.

Could you guys PLEASE take a look at it?

It would be great if it could be configured trough TypoScript, but while you don't change that, you should change to simple conf var.

It is REALLY annoying having to change a hard coded configuration value like that every time I update the source.

Not to mention when I forget it, that I certainly will.

Actions #16

Updated by Michael Stucki over 17 years ago

Sebastian, if you cannot do it, please assign it to me.

Actions #17

Updated by Martin Kutschker almost 17 years ago

We have a config.formMailCharset setting, so we could have a config.formMailFilesize as well.

We could have of course have config.formMailExtensions and perhaps even a config.formMailTypes to check for file extensions and mime types. Though the latter may be a problem. Who knows what the clients send us.

Actions #18

Updated by Robert Markula over 16 years ago

I agree with Felipe; in the meantime please set this limit to a reasonable amount, which should be sufficient for most use cases for now.

Actions

Also available in: Atom PDF