Drop `$max_vacation_length` setting

[rimas-kudelis]

Since #188, vacation message length is not limited by the database. Thus we should drop $max_vacation_length setting from variables.php.example and remove any references to it from the PHP files.

[Udera]

Isn't no limit a bit dangerous? Someone could just dump something big in our database?

[rimas-kudelis]

In theory yes, but it's quite unlikely, don't you think?

[Udera]

Actually field-type text seems to have a limit: https://stackoverflow.com/questions/13932750/tinytext-text-mediumtext-and-longtext-maximum-storage-sizes, https://dev.mysql.com/doc/refman/5.7/en/storage-requirements.html

It's probably better if we restrict the length via php as well. The limit can be large, but there should be a limit. In general such notifications should be rather short.

[rimas-kudelis]

Every text field has a practical limit, I just don't think it's necessary for us to act upon it when it's absolutely unlikely that someone would hit that limit in practice. 2¹⁶ characters is dozens of A4 pages. Nobody in their right mind will type a vacation message that long. And even if someone tries to hack the system by supplying a message that is too long, I expect PDO to just throw an exception or even defer to the database driver, which in turn could either ignore this and cut off the part of the text that won't fit, or return an error, in which case again PDO will probably throw an exception.

[Udera]

I see, nobody is motivated to really test it ;-) I will see if I find some time on the weekend...

On 2016-08-30 16:13, Rimas Kudelis wrote:

Every text field has a practical limit, I just don't think it's necessary for us to act upon it when it's absolutely unlikely that someone would hit that limit in practice. 216 characters is dozens of A4 pages. Nobody in their right mind will type a vacation message that long. And even if someone tries to hack the system by supplying a message that is too long, I expect PDO to just throw an exception or even defer to the database driver, which in turn could either ignore this and cut off the part of the text that won't fit, or return an error, in which case again PDO will probably throw an exception.

You are receiving this because you commented. Reply to this email directly, view it on GitHub [1], or mute the thread [2].

*

Links:

[1] https://github.com/vexim/vexim2/issues/198#issuecomment-243452352 [2] https://github.com/notifications/unsubscribe-auth/AGCFFA1Ft9xAx9SWjwWjpSHJVApwEK0pks5qlDpwgaJpZM4Jmb6V

[rimas-kudelis]

I'm honestly surprised that you are motivated to test it. :)

[Udera]

Is it possible that this check is completely wrong implemented. It doesn't appear at all in adminuserchangesubmit.php. And in userchangesubmit.php it is checked after the values were successfully inserted in the database: https://github.com/vexim/vexim2/blob/master/vexim/userchangesubmit.php#L110-L113

[rimas-kudelis]

I guess you're right.

[Udera]

I tested with a too long text. You won't get it in because mysql throws an error which is not shown by default. A 10 kB textfile just goes into it. So it's probably not necessary to catch this exception at all (it's very unlikely that someone tries to set a vacation message > 10 kB of pure text.

[rimas-kudelis]

Fixed via #215.