WordPress.org

Make WordPress Core

Opened 8 days ago

Last modified 38 hours ago

#44141 new defect (bug)

Privacy: Don't replace comment author URL with anything

Reported by: johnbillion Owned by:
Milestone: 4.9.7 Priority: normal
Severity: normal Version: 4.9.6
Component: Comments Keywords: has-patch has-unit-tests
Focuses: privacy Cc:

Description

When anonymizing comment author information, replacing the comment author's site URL with https://site.invalid has the effect of exposing broken links to end users.

During the anonymization process there's no reason to replace the comment author URL with anything. It should be replaced with an empty string as if the comment author had not entered a URL.

Introduced in [42994].

Attachments (2)

44141.patch (878 bytes) - added by TZ Media 8 days ago.
44141.diff (5.7 KB) - added by desrosj 38 hours ago.

Download all attachments as: .zip

Change History (9)

@TZ Media
8 days ago

#1 @TZ Media
8 days ago

  • Keywords has-patch added; needs-patch removed

Quick fix that sets comment_author_url to an empty string instead of using wp_privacy_anonymize_data().

#2 @johnbillion
8 days ago

  • Keywords needs-unit-tests added

Thanks for the patch, @TZ Media . This will need some updates to the corresponding unit tests too.

#3 @iandunn
8 days ago

I think the reason to replace it was so that it's obvious that its been redacted, rather than introducing ambiguity about whether or not there originally was data there.

I don't think that's critical in this case, but it should be considered before changing it.

An alternative approach would be to modify the front end comment form to ignore anonymized URLs, although that's not as elegant as just making it an empty string.

#4 @summoner
8 days ago

What if there would be a screen with some text like "Most probably the user has let their data deleted" or something like that. So the people could find out what has happened to the content they wanted to see and even the link would not be invalid.

#5 @desrosj
4 days ago

  • Milestone changed from 4.9.7 to 4.9.8

Moving all tickets in 4.9.7 to 4.9.8.

#6 @SergeyBiryukov
3 days ago

  • Type changed from enhancement to defect (bug)

Exposing broken links to end users sounds like a bug to me, I don't think avoiding the ambiguity about whether or not the URL was originally provided is worth it.

@desrosj
38 hours ago

#7 @desrosj
38 hours ago

  • Keywords has-unit-tests added; needs-unit-tests removed
  • Milestone changed from 4.9.8 to 4.9.7

In 44141.diff:

  • Add @group privacy to the tests in Tests_Comments that test the anonymization of comment data.
  • Adds translator context to the Anonymous string that replaces the commenter's name. This could be important for some languages.
  • Continue passing the comment author's URL through wp_privacy_anonymize_data() so that plugins and themes can filter the URL if desired.
  • Adjust tests for anonymizing comment author URLs and emails.
  • Add a _wp_anonymized comment meta value of true when a comment is successfully anonymized.
  • Change wp_privacy_anonymize_data() to return an empty string by default for both emails & URLs.

The main change here is that a comment meta value is added flagging a comment as anonymized. It allows the URL to be empty but it's still obvious that there was potentially data there that was redacted. The _wp_anonymized key could also contain an array of the fields that did contain data before being erased if knowing that is important.

Note: See TracTickets for help on using tickets.