fix: validate comment verb length#40965
Conversation
bcd7468 to
0725814
Compare
|
Kudos, SonarCloud Quality Gate passed! |
| throw new \InvalidArgumentException('Non-empty String expected.'); | ||
| } | ||
| $verb = \trim($verb); | ||
| if (\mb_strlen($verb, 'UTF-8') > IComment::MAX_VERB_LENGTH) { |
There was a problem hiding this comment.
If this is correct, it will need a comment.
I see a "verb" column in the oc_comments table, which is where this data will be stored I guess. The DB length is in bytes, and I think the mb_strlen will count the chars. If we consider non-latin languages, 1 char can take 3-4 bytes, which means that the verb can take up to 64 chars or 256 bytes, so this won't fit in the DB.
In addition, I assume that the MAX_VERB_LENGTH is counted in bytes. If it's counted in multibyte chars, it needs to be documented.
There was a problem hiding this comment.
THX. You are absolutly right about this. I blindly copied the logic from message.
This whole db-length-byte madness only applies to MySQL only ... right?
There was a problem hiding this comment.
https://www.postgresql.org/docs/current/datatype-character.html
SQL defines two primary character types: character varying(n) and character(n), where n is a positive integer. Both of these types can store strings up to n characters (not bytes) in length
https://docs.oracle.com/cd/A57673_01/DOC/server/doc/SCN73/ch6.htm#varchar2
The VARCHAR2 datatype stores variable-length character strings. When you create a table with a VARCHAR2 column, you specify a maximum column length (in bytes, not characters)
https://dev.mysql.com/doc/refman/8.0/en/char.html
The CHAR and VARCHAR types are declared with a length that indicates the maximum number of characters you want to store. For example, CHAR(30) can hold up to 30 characters.
It seems oracle is the only one counting bytes, not chars...
0725814 to
0935954
Compare
0935954 to
9870862
Compare
|
Kudos, SonarCloud Quality Gate passed! |








Description
The comment verb is user input and needs validation in length.
Related Issue
Motivation and Context
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Checklist: