WordPress.org

Make WordPress Core

Opened 7 years ago

Last modified 15 months 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)

17771.diff (623 bytes) - added by tenpura 6 years ago.
updated for 3.4
17771.patch (913 bytes) - added by chriscct7 3 years ago.

Download all attachments as: .zip

Change History (22)

#1 @dd32
7 years ago

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)

@tenpura
6 years ago

updated for 3.4

#2 @tenpura
6 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 @SergeyBiryukov
6 years ago

  • Version changed from 3.4 to 3.2

Version number indicates when the bug was initially introduced/reported.

Related: #10483

#4 @dd32
5 years ago

#16519 was marked as a duplicate.

#5 @ocean90
4 years ago

  • Component changed from I18N to Pings/Trackbacks

@chriscct7
3 years ago

#6 @chriscct7
3 years ago

Refreshed patch

#7 @SergeyBiryukov
23 months ago

  • Keywords 4.7-early added

#8 @rachelbaker
23 months 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.


22 months ago

#10 @SergeyBiryukov
22 months ago

  • Owner set to SergeyBiryukov
  • Status changed from new to reviewing

#11 @helen
22 months 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.


22 months ago

#13 @dshanske
22 months 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: @dshanske
22 months 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 @jorbin
21 months 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.


20 months ago

This ticket was mentioned in Slack in #core by stevenkword. View the logs.


20 months ago

#18 @stevenkword
20 months ago

  • Milestone changed from 4.7 to Future Release

Moving to future release so we can properly consider storage.

#19 @dshanske
20 months ago

This is going to require some discussion.

#20 @dshanske
15 months 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.

Note: See TracTickets for help on using tickets.