#10377 closed defect (bug) (fixed)
Comment fields should have max lengths
Reported by: | muriloazevedo | Owned by: | rachelbaker |
---|---|---|---|
Milestone: | 4.5 | Priority: | normal |
Severity: | normal | Version: | 2.8 |
Component: | Comments | Keywords: | has-patch has-unit-tests |
Focuses: | Cc: |
Description
Hello, I don't know if exactly it's a bug or use javascript validation, but i think so there's no
treatment for this, or was fix in the new version.
In the section of comments(Wordpress 2.8), we can insert how many characheters we wants, generating an SQL Exception and breaking the all system.
The solution is simple, use the property maxlenght in the tag
<input /> e limit the characters if will be send to database.
Attachments (8)
Change History (51)
#2
@
15 years ago
also note, That MySQL should actually truncate the data if its longer, not throw an error..
#3
in reply to:
↑ 1
@
15 years ago
Replying to dd32:
... so there's a limit of 65,535 characters in a comment at present.. Perhaps the comment handler should throw a error upon a longer comment.
There's some basic back-end validation already (duplicate comments, non-empty name and email, etc.). Adding max length for all comment fields should be trivial.
Actually this may be good for most POST requests in the admin including AJAX.
#4
@
14 years ago
- Summary changed from Problem in comments! to Comment fields should have max lengths
@
11 years ago
Add maxlength to comment textarea and size check in wp-comments-post.php (extends patch from #10377)
#6
@
11 years ago
- Cc weston@… added
- Keywords has-patch added; 'sql exception' comments removed
As noted in the attachment comment, in addition to the $comment_content
size check in wp-comments-post.php
, I've also patched the comment form to add a maxlength
to the textarea
itself (this extends a patch from #4332). Patch attached, and branch also pushed to GitHub fork: https://github.com/x-team/WordPress/compare/10377-comment-fields-should-have-max-lengths
#8
@
9 years ago
- Keywords needs-refresh removed
- Milestone changed from Future Release to 4.4
- Owner set to rachelbaker
- Status changed from new to accepted
#11
@
9 years ago
Yep, that's fine. comment_content
is a text
field, which stores 65535 bytes. strlen()
also counts the number of bytes, so we're being consistent.
#12
follow-up:
↓ 13
@
9 years ago
Instead of a hardcoded value, seems like we should use $wpdb->get_col_length()
to get the actual value, in case someone modified the DB schema for a particular install.
See upgrade_430_fix_comments() for reference.
#13
in reply to:
↑ 12
;
follow-up:
↓ 14
@
9 years ago
Replying to SergeyBiryukov:
Instead of a hardcoded value, seems like we should use
$wpdb->get_col_length()
to get the actual value, in case someone modified the DB schema for a particular install.
See upgrade_430_fix_comments() for reference.
I don't know if that's worth the extra db call. Someone could just as easily use the filter to modify it if that's the case
#14
in reply to:
↑ 13
@
9 years ago
Replying to chriscct7:
I don't know if that's worth the extra db call. Someone could just as easily use the filter to modify it if that's the case
It's not an extra call - the same call will be done later when the data is actually being written to the DB.
I agree with @SergeyBiryukov, $wpdb->get_col_length()
is a better way of making sure it's the right length.
#15
@
9 years ago
- Keywords needs-refresh added
Thank you all for the feedback and direction. Updated patch forthcoming.
#17
@
9 years ago
- Keywords dev-feedback added; needs-refresh removed
I made an attempt at using $wpdb->get_col_length()
in the new function wp_get_comment_content_max_length()
(isn't that a mouthful).
#18
follow-up:
↓ 19
@
9 years ago
The maxlength
attribute is probably redundant (and would cause an extra query on front-end), I think just checking the length in wp-comments-post.php
would be enough.
#19
in reply to:
↑ 18
@
9 years ago
Replying to SergeyBiryukov:
The
maxlength
attribute is probably redundant (and would cause an extra query on front-end), I think just checking the length inwp-comments-post.php
would be enough.
The issue I see there is that if a commenter was to leave a 65525+ comment they wouldn't know it was too long until they press submit. Including "max-length" in the comment form template would at least give the commenter feedback that they have reached some sort of limit when they try to type the 65526 character.
#20
@
9 years ago
Don't think there's a problem to have hard-coded maxlength. 65k characters is a lot of text, about 11,000 words or 44 printed pages :)
We already have a filter 'comment_form_default_fields'
. Can add an array with hard-coded lengths for the (very rare) cases when somebody wants comments longer than that. Then run the same filter on checking the POST lengths. Or maybe have a function similar to wp_get_comment_content_max_length() that will filter and return the hard-coded values.
Also: what about maxlength and strlen() for the other comment fields? If we are adding that for the textarea would be good to add everywhere.
This ticket was mentioned in Slack in #core by dd32. View the logs.
9 years ago
#23
@
9 years ago
- Milestone changed from 4.4 to Future Release
- Owner rachelbaker deleted
- Status changed from accepted to assigned
Punting out of 4.4
#25
follow-up:
↓ 26
@
9 years ago
- Keywords has-unit-tests dev-feedback added; needs-refresh removed
In 10377.4.diff:
- hardcoded
maxlength
attribute on thecomment_field
, the markup here can be modified via thecomment_form_defaults
filter (1) - added
wp_get_comment_column_max_length()
which returns the max column length for a given column name, and is filterable (2) - added logic in
wp_handle_comment_submission()
to return a WP_Error when thecomment_author
,comment_author_url
, orcomment_content
values exceed the max length of their columns - added unit tests for the error conditions in
wp_handle_comment_submission()
- We could add hardcoded attributes like this to the two other comment form inputs. Open to second opinions here.
- Is there a good way to convert characters to bytes?
#26
in reply to:
↑ 25
@
9 years ago
Replying to rachelbaker:
10377.4.diff is starting to look good.
- We could add hardcoded attributes like this to the two other comment form inputs.
Thinking this is a must. Having maxlength
on the other three ( Name, Email, Website ) text fields will improve UX if these limits are reached.
- Is there a good way to convert characters to bytes?
Nothing that is 100% compatible/reliable afaik. If you want to check the byte length of a string, mb_strlen( $string, '8bit' )
should work well. That's the proper way to test it when the "real" mb_strlen() exists. If it doesn't exist, the compat function will return strlen( $string )
.
Also this looks like a typo:
} elseif ( is_array( $col_length ) && isset( $col_length['length'] ) && (int) $col_length > 0 ) {
should probably be:
} elseif ( is_array( $col_length ) && isset( $col_length['length'] ) && intval( $col_length['length'] ) > 0 ) {
@
9 years ago
Refresh of 10377.4 with maxlength for all comment template fields and mb_strlen checks where needed
#27
follow-up:
↓ 28
@
9 years ago
@azaozz Can you take a look at 10377.5.diff when you have time? I corrected my conditional, added maxlength checks for all comments-template fields, and compare the byte length on tinytext and text columns.
#28
in reply to:
↑ 27
@
9 years ago
Replying to rachelbaker:
10377.5.diff looks great :)
#29
@
9 years ago
- Owner set to rachelbaker
- Resolution set to fixed
- Status changed from assigned to closed
In 36272:
#30
@
9 years ago
- Keywords needs-patch needs-unit-tests added; has-patch has-unit-tests dev-feedback removed
- Resolution fixed deleted
- Status changed from closed to reopened
[36272] broke comments on w.org because $wpdb->get_col_length()
is returning false and falls back to 255. Looking at upgrade_430_fix_comments()
the fallback should be 65535.
Not sure yet why it returns false, but the method returns also false in case $wpdb->is_mysql
is false.
Edit: $wpdb->is_mysql
is false via HyperDB.
#31
@
9 years ago
@ocean90 In 10377.6.diff I changed the default fallback to be 65525, 65535 - 10. This matches the logic in upgrade_430_fix_comments()
. Unit tests for each text-based column type for the comments table included in test_wp_get_comment_column_max_length()
.
This ticket was mentioned in Slack in #core by ocean90. View the logs.
9 years ago
#35
@
9 years ago
broke comments on w.org because $wpdb->get_col_length() is returning false and falls back to 255. Looking at upgrade_430_fix_comments() the fallback should be 65535.
...the method returns also false in case $wpdb->is_mysql is false.
Not sure if the default lengths for email, name, URL should be 65525. We should be using "sane" values :)
How about we go back to the idea of having an array with the default lengths and override it with the actual lengths from the DB (if we can get them)? Something like 10377.7.diff (needs to fix the unit tests).
#36
@
9 years ago
- Resolution fixed deleted
- Status changed from closed to reopened
@azaozz 10377.7.diff makes more sense. Reopened this.
That doesnt stop people from directly posting the data anyway..
The comment_content field is a TEXT field:
so theres a limit of 65,535 characters in a comment at present.. Perhaps the comment handler should throw a error upon a longer comment.