Opened 13 years ago
Last modified 8 years ago
#17771 reviewing defect (bug)
URL-encoded comment_author_url gets broken by MySQL varchar 200 length limit
Reported by: | tenpura | Owned by: | SergeyBiryukov |
---|---|---|---|
Milestone: | Future Release | Priority: | normal |
Severity: | normal | Version: | 3.2 |
Component: | Comments | Keywords: | has-patch dev-feedback 2nd-opinion |
Focuses: | Cc: |
Description
WordPress sometimes pings back with long permalinks that exceed comment_author_url column length limit of 200, which results in generating unusable broken links to the post.
It easily reaches to the limit, especially if the permalink contains url-encoded multibyte title as postname. (e.g. 23 characters of UTF-8 Japanese become a 207 characters long url-encoded string. Incomplete url-encoded string may trigger 400 Bad Request too.)
Solution:
In pingback(), use shortlink instead of regular permalink if the URL is longer than 200 characters.
It seems to work ok with wp.me shortlinks.
Attachments (2)
Change History (22)
#2
@
13 years ago
- Component changed from Comments to I18N
- Summary changed from Long comment_author_url of the pingback gets broken by MySQL varchar 200 length limit to URL-encoded comment_author_url gets broken by MySQL varchar 200 length limit
- Version changed from 3.2 to 3.4
#3
@
13 years ago
- Version changed from 3.4 to 3.2
Version number indicates when the bug was initially introduced/reported.
Related: #10483
#8
@
8 years ago
- Milestone changed from Awaiting Review to Future Release
We have wp_get_comment_fields_max_lengths()
which is used to check input from the comment form against the maximum characters of the comments table columns. It might be helpful here. See #10377
This ticket was mentioned in Slack in #core by helen. View the logs.
8 years ago
#11
@
8 years ago
- Milestone changed from Future Release to 4.7
Might as well not have any breakage for now until different storage is more fully considered.
This ticket was mentioned in Slack in #core by dshanske. View the logs.
8 years ago
#13
@
8 years ago
What we need is something to store in comment_author_url to indicate that it is too large and we stored it elsewhere.
The suggestion was a short URL. However, for an outside site, we might not get one of those. Since there are functions for retrieving the URL, we could have those check and replace if too large. However, many people access the comment array on retrieval directly.
comment_author_url is overloaded in its use to be the pingback url storage location to begin with. Doesn't make it easy to determine what the right course of action is. Comment meta didn't exist when this started.
#14
follow-up:
↓ 15
@
8 years ago
I'm wondering if the best solution isn't to start storing the pingback URL in comment meta by default for new pingbacks and only check comment_author_url if the meta key doesn't exist. I'm worried about the backcompat implications.
#15
in reply to:
↑ 14
@
8 years ago
Replying to dshanske:
I'm wondering if the best solution isn't to start storing the pingback URL in comment meta by default for new pingbacks and only check comment_author_url if the meta key doesn't exist. I'm worried about the backcompat implications.
This seems like a good solution to me that would be backwards compatible. My one question that I'm not sure about is if we are currently loading meta for all comments. If not, this could add a DB query.
This ticket was mentioned in Slack in #core by jeffpaul. View the logs.
8 years ago
This ticket was mentioned in Slack in #core by stevenkword. View the logs.
8 years ago
#18
@
8 years ago
- Milestone changed from 4.7 to Future Release
Moving to future release so we can properly consider storage.
#20
@
8 years ago
- Component changed from Pings/Trackbacks to Comments
- Keywords dev-feedback 2nd-opinion added; 4.7-early removed
While the issue may be caused by pingbacks with long URLS, since the issue is that of comment fields, I'm reclassifying this as a comment component issue.
The decision that has to be made is how to address the limitation. Speaking from the Pings and Trackbacks point of view, which I think we need to address concurrently, using comment_author_url as the source for the pingback/trackback means you can't have an actual author URL separate from the source...which is an issue for my continued proposal of improving the display to be useful. So I'd like that to be kept in mind, but not override the actual issue of length.
The comments seem to indicate that there is no objection to moving the source of the pingback to the comment_meta, and having the code check the comment_author_url if no meta key exists. But that would require another database check, as mentioned. The Comment object doesn't pull in meta keys, so that also has to be addressed.
We might be able to pair this issue with other tickets that talk about enhancing comments in general and their retrieval to figure out a solution. This includes things like last modified for comments, comment avatars not coming from gravatar, etc. and look at the biger picture there.
This is something that would be better off stored in the comment meta data table IMO. Of course, that adds an extra query for displaying comments, but as metadata becomes used more, it'll be a query which is made anyway.
The useragent currently overflows the MySQL field sometimes too.. There's a ticket for that somewhere I believe (can't find it right now)