Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.We define "quantity" as a decimal(10, 2) column, which means the biggest value it can accept is 99 999 999.99 (8 digits on the left, 2 on the right).
However, commerce_line_item_handler_field_edit_quantity currently does this:
'#maxlength' => max(4, strlen($quantity)),
instead of this:
'#maxlength' => max(4, 8),
so maxlength gets set to 9 (probably because it's not rounded before the length measurement, didn't check), allowing you to input a value that crashes the update (overflow on the quantity field).
However, even with setting maxlength to a smaller value, there's still the possibility of overflowing the amount column of the price field.
That is something we can't really fix, since we don't know how big the prices can be.
Still, it's something to have in mind.
So, I should submit the maxlength change as a patch, and you guys should tell me if you have any additional ideas, so that the user can't crash the site through basic input.
| Comment | File | Size | Author |
|---|---|---|---|
| #5 | commerce-1379084-quantity_max_length-5.patch | 891 bytes | czigor |
| |||
Comments
Comment #1
amateescu CreditAttribution: amateescu commentedThe easiest solution would be to change the total amount field to a bigint. There's a small catch though, because it is a price field and that would mean changing every price field to a bigint. Maybe we need a new field type for totals?
That would allow us to store numbers like:
99 999 999.99 [decimal(10, 2)] x 2 147 483 647 [signed int] =
214 748 364 678 525 163.53, which is smaller than
9 223 372 036 854 775 807 [signed bigint].
@bojanz,
max(4, 8)? Seriously? :PComment #2
amateescu CreditAttribution: amateescu commentedHrm, scratch that, I should've compared
So, we must also change the quantity field to decimal(9,2).
Comment #3
rszrama CreditAttribution: rszrama commentedTagging for http://contribkanban.com/#/board/commerce/7.x-1.x.
Comment #4
rszrama CreditAttribution: rszrama at Centarro commentedI think we can just set the max length lower, document the change, and then leave it to people to form alter it back up if they've made other changes in their database to support large prices. We can't change the schema at this point, so this is the best we can do.
Comment #5
czigor CreditAttribution: czigor at Centarro commentedComment #6
sjmobley CreditAttribution: sjmobley at Centarro commentedComment #7
rszrama CreditAttribution: rszrama at Centarro commentedHah, so on testing, I realized that a quantity of 99999999 on even a modestly priced product would result in a fatal error by breaking the maximum allowed value for the line item total price. We can endlessly optimize this, but it's never really come up ... so I've just adjusted the #maxlength down to 6 and added additional comments to explain the decision. Best we can do without rewriting our approach toward validating data prior to input, and that isn't likely in 1.x.
Comment #8
rszrama CreditAttribution: rszrama at Centarro commentedCommitted.