6 ms·

> Will the DB raise an exception? Will it crash? No – It automatically truncates the data so it fits the column size. If you write software, never be "helpful"

by pilif 11y ago

> Will the DB raise an exception? Will it crash? No – It automatically truncates the data so it fits the column size. If you write software, never be "helpful" and try to fix problems with input data you can't handle. You have no idea whether the fix you have in mind is going to be appropriate in all situations and there is no guarantee that your users are going to acknowledge the warning you might (or might not) issue in that case (case in point: MySQL does fire warnings, but many libraries don't even expose them to the user, so even if they wanted to check for warnings after every statement, they couldn't) "Fixing" broken input data is nothing but actually corrupting data because after the fact there is no way to know what the original was, nor is there a way to know whether the "fixed" data was actually fixed or whether it has been that way to begin with. If you are in a position where you can't do something based on the input data, then blow up. That way you help users to avoid ever having to deal with corrupted data (or all their private bugs exposed to the public like what happened here)

deleted 11y ago
[deleted]
bkor 11y ago
Not really the full story. Bugzilla explicitly disables strict mode (due to another problem). Pretty stupid to have done that! See for details https://bugzilla.mozilla.org/show_bug.cgi?id=321645#c20 https://bugzilla.mozilla.org/show_bug.cgi?id=321645#c20 PS: I wrote the patch to disable strict mode.
- makomk 11y ago
  If I'm reading that correctly, the problem was that some MySQL versions couldn't actually restore database dumps they created using mysqldump if strict mode was enabled. That's a pretty ugly bug.
  - bkor 11y ago
    If you're on strict mode, TEXT and BLOB types in MySQL 5 don't support a default value. Bugzilla didn't set anything for those fields; relying on MySQL putting in the default value. At that time Bugzilla had loads of places where the database would insert things into the database. Fixing all of those would've been pretty daunting task and IMO MySQL not supporting those defaults anymore is a too invasive (backwards compatibility breaking) change. See http://dev.mysql.com/doc/refman/5.6/en/data-type-defaults.html http://dev.mysql.com/doc/refman/5.6/en/data-type-defaults.ht.... This was around the time that GNOME once again was upgrading its Bugzilla instance (IIRC), plus lots of people were moving to MySQL 4. Fixing this was pretty urgent and assumed that MySQL's behaviour would change back. Because of this MySQL change being backwards incompatible, it also broke restoring backups. This patch wouldn't help with that though (it just changes the setting for the database connection created by Bugzilla; it doesn't change the server setting). It took many years until the Bugzilla code was less of a mess. If the same bug would've been opened nowadays a proper fix could've been made. Back then, urgh.
    - pilif 11y ago
      > Because of this MySQL change being backwards incompatible, it also broke restoring backups. No matter how you paint it, if MySQL isn't able to restore a backup created with the backup utility of the same version, then this is MySQL's fault. It is absolutely not acceptable for a backup utility to run through but produce something that then cannot be restored. I would have expected mysqldump to blow up or even better, the newer daemon to not start up if it sees a schema that it doesn't support any more.
dspillett 11y ago
> If you write software, never be "helpful" and try to fix problems with input data you can't handle. Yep. Accepting bad data in any way is a bug, not a feature, and it isn't always (I'd suggest it isn't commonly) intentional: for instance mySQL used to accept dates like 2015-02-30 simply because its internal date format permitted it rather than because they were explicitly allowing it - it was an accident not an attempt to be helpful. Accidental bugs that accept bad data are often indistinguishable from intentional "help", and can get passed up through a stack of calls (App A uses library B that wraps library C which calls service D that uses library E...) so even if the help is very well documented at source that knowledge is usually lost as you move up the chain and the application dev is left with a library that accepts bad data unbeknownst to him/her which could end up masking a serious bug that will bite them later. > "Fixing" broken input data is nothing but actually corrupting Exactly. The old "be strict in what you send, but generous in what you receive" is often misinterpreted - you should only be generous in what you receive if everything you receive can be properly handled all the way into your stack and all the way back out again, and never be generous enough to accept bad data. Back to the date example: yes, accept any unambiguous date format the user might throw at you, but never accept an invalid date. > If you are in a position where you can't do something based on the input data, then blow up. Definitely. If something could fail it should fail as early as possible, and not quietly in development (caveat: failures should not be outwardly verbose in production for security reasons), that way you are more likely to catch problems early, before they compound into something worse.
- josephcooney 11y ago
  I don't think it is quite that clear cut. See, for example, Postel's Law AKA the Robustness Principle. Be conservative in what you do, be liberal in what you accept from others https://en.wikipedia.org/wiki/Robustness_principle https://en.wikipedia.org/wiki/Robustness_principle
  - pilif 11y ago
    The problem with > should accept non-conformant input as long as the meaning is clear on that article is that the definition of "clear" is somewhat murky. Let's use the MySQL string truncation that spurred this thread as an example. When asked to put 300 character string in a 255 character wide field, for MySQL it was "clear" that the user only meant to actually store 255 characters. From the perspective of MySQL, that actually makes sense: The field is declared as storing a maximum of 255 characters, so obviously, the user intends to store a maximum of 255 characters in that field. Now look where this interpretation of "clear" facts has lead us to. The issues that can be caused by "fixing" up invalid data are usually much harder (and more embarrassing) to fix than an exception that happens early. Trust me: I'd rather get an exception than a CVE when given the choice. Furthermore: By "fixing" up data, your fixes become part of the protocol. Implementations of clients derived from the broken implementation might not notice the brokenness and suddenly you're stuck with having to fix issues the same way for all eternity because becoming more strict will cause backwards compatibility breaks. Worse: You also lock yourself out of the ability to later actually extend the protocol in a meaninful way because you're already accepting broken data. Let's say you have a JSON based protocol that has a flag "foo" that can be set to some value: {"foo": 12}. Now there's a broken implementation of a client around that sends {"foobar": 12}. As you're "sure" that they actually mean "foo", you add an alias "foobar" to mean "foo". Even though you've never intended it, now "foobar" is part of the official protocol and clients start sending this all over the place. If at a later point, you actually want to support "foobar", you can't because that's already a hack to mean "foo", so now you'll end up with some crap like {"real_foobar": 1234}. So not only is this behaviour irresponsible to clients (see the bugzilla issue), no, it's also a sure way to make your own life harder in the future as it makes for harder to maintain code and makes you lose flexibility in protocol design.
MichaelGG 11y ago
MySQL is basically taking Visual Basic's "On Error Resume Next" approach. Which is also what a lot of JS seems to be doing, too, based on limited experience.
- praseodym 11y ago
  Indeed, and in some cases this can have side effects like skipping certificate validation: https://github.com/github/hubot/issues/1046 https://github.com/github/hubot/issues/1046

by pilif 11y ago

deleted 11y ago

[deleted]

bkor 11y ago

Not really the full story. Bugzilla explicitly disables strict mode (due to another problem). Pretty stupid to have done that! See for details https://bugzilla.mozilla.org/show_bug.cgi?id=321645#c20 https://bugzilla.mozilla.org/show_bug.cgi?id=321645#c20 PS: I wrote the patch to disable strict mode.

makomk 11y ago
If I'm reading that correctly, the problem was that some MySQL versions couldn't actually restore database dumps they created using mysqldump if strict mode was enabled. That's a pretty ugly bug.
- bkor 11y ago
  If you're on strict mode, TEXT and BLOB types in MySQL 5 don't support a default value. Bugzilla didn't set anything for those fields; relying on MySQL putting in the default value. At that time Bugzilla had loads of places where the database would insert things into the database. Fixing all of those would've been pretty daunting task and IMO MySQL not supporting those defaults anymore is a too invasive (backwards compatibility breaking) change. See http://dev.mysql.com/doc/refman/5.6/en/data-type-defaults.html http://dev.mysql.com/doc/refman/5.6/en/data-type-defaults.ht.... This was around the time that GNOME once again was upgrading its Bugzilla instance (IIRC), plus lots of people were moving to MySQL 4. Fixing this was pretty urgent and assumed that MySQL's behaviour would change back. Because of this MySQL change being backwards incompatible, it also broke restoring backups. This patch wouldn't help with that though (it just changes the setting for the database connection created by Bugzilla; it doesn't change the server setting). It took many years until the Bugzilla code was less of a mess. If the same bug would've been opened nowadays a proper fix could've been made. Back then, urgh.
  - pilif 11y ago
    > Because of this MySQL change being backwards incompatible, it also broke restoring backups. No matter how you paint it, if MySQL isn't able to restore a backup created with the backup utility of the same version, then this is MySQL's fault. It is absolutely not acceptable for a backup utility to run through but produce something that then cannot be restored. I would have expected mysqldump to blow up or even better, the newer daemon to not start up if it sees a schema that it doesn't support any more.

dspillett 11y ago

> If you write software, never be "helpful" and try to fix problems with input data you can't handle. Yep. Accepting bad data in any way is a bug, not a feature, and it isn't always (I'd suggest it isn't commonly) intentional: for instance mySQL used to accept dates like 2015-02-30 simply because its internal date format permitted it rather than because they were explicitly allowing it - it was an accident not an attempt to be helpful. Accidental bugs that accept bad data are often indistinguishable from intentional "help", and can get passed up through a stack of calls (App A uses library B that wraps library C which calls service D that uses library E...) so even if the help is very well documented at source that knowledge is usually lost as you move up the chain and the application dev is left with a library that accepts bad data unbeknownst to him/her which could end up masking a serious bug that will bite them later. > "Fixing" broken input data is nothing but actually corrupting Exactly. The old "be strict in what you send, but generous in what you receive" is often misinterpreted - you should only be generous in what you receive if everything you receive can be properly handled all the way into your stack and all the way back out again, and never be generous enough to accept bad data. Back to the date example: yes, accept any unambiguous date format the user might throw at you, but never accept an invalid date. > If you are in a position where you can't do something based on the input data, then blow up. Definitely. If something could fail it should fail as early as possible, and not quietly in development (caveat: failures should not be outwardly verbose in production for security reasons), that way you are more likely to catch problems early, before they compound into something worse.

josephcooney 11y ago
I don't think it is quite that clear cut. See, for example, Postel's Law AKA the Robustness Principle. Be conservative in what you do, be liberal in what you accept from others https://en.wikipedia.org/wiki/Robustness_principle https://en.wikipedia.org/wiki/Robustness_principle
- pilif 11y ago
  The problem with > should accept non-conformant input as long as the meaning is clear on that article is that the definition of "clear" is somewhat murky. Let's use the MySQL string truncation that spurred this thread as an example. When asked to put 300 character string in a 255 character wide field, for MySQL it was "clear" that the user only meant to actually store 255 characters. From the perspective of MySQL, that actually makes sense: The field is declared as storing a maximum of 255 characters, so obviously, the user intends to store a maximum of 255 characters in that field. Now look where this interpretation of "clear" facts has lead us to. The issues that can be caused by "fixing" up invalid data are usually much harder (and more embarrassing) to fix than an exception that happens early. Trust me: I'd rather get an exception than a CVE when given the choice. Furthermore: By "fixing" up data, your fixes become part of the protocol. Implementations of clients derived from the broken implementation might not notice the brokenness and suddenly you're stuck with having to fix issues the same way for all eternity because becoming more strict will cause backwards compatibility breaks. Worse: You also lock yourself out of the ability to later actually extend the protocol in a meaninful way because you're already accepting broken data. Let's say you have a JSON based protocol that has a flag "foo" that can be set to some value: {"foo": 12}. Now there's a broken implementation of a client around that sends {"foobar": 12}. As you're "sure" that they actually mean "foo", you add an alias "foobar" to mean "foo". Even though you've never intended it, now "foobar" is part of the official protocol and clients start sending this all over the place. If at a later point, you actually want to support "foobar", you can't because that's already a hack to mean "foo", so now you'll end up with some crap like {"real_foobar": 1234}. So not only is this behaviour irresponsible to clients (see the bugzilla issue), no, it's also a sure way to make your own life harder in the future as it makes for harder to maintain code and makes you lose flexibility in protocol design.

MichaelGG 11y ago

MySQL is basically taking Visual Basic's "On Error Resume Next" approach. Which is also what a lot of JS seems to be doing, too, based on limited experience.

praseodym 11y ago
Indeed, and in some cases this can have side effects like skipping certificate validation: https://github.com/github/hubot/issues/1046 https://github.com/github/hubot/issues/1046