Make WordPress Core

Opened 16 years ago

Closed 16 years ago

Last modified 15 years ago

#2889 closed defect (bug) (fixed)

duplicate content when submitting comments

Reported by: whooami's profile whooami Owned by:
Milestone: Priority: high
Severity: normal Version: 2.0.3
Component: General Keywords: comment, function, anchor, make_clickable, bg|has_patch
Focuses: Cc:

Description

An example of the problem is viewable on my brand new sandbox blog @ http://www.village-idiot.org/test-install/?p=1#comments

That is a brand new install with NO plugins installed.

Essentially what is happening is that link is being filtered twice and content is being added to the comment.

When you view the source for the second comment, you will notice that nofollow tags are changed - thats because I edited the make_clickable function and the wp_rel_nofollow function:

function wp_rel_nofollow( $text ) {
	$text = preg_replace('|<a (.+?)>|i', '<a $1 rel="nofollow-function">', $text);
	return $text;
}
function make_clickable($ret) {
	$ret = ' ' . $ret . ' ';
	$ret = preg_replace("#([\s>])(https?)://([^\s<>{}()]+[^\s.,<>{}()])#i", "$1<a href='$2://$3' rel='nofollow-clickable'>$2://$3</a>", $ret);
	$ret = preg_replace("#(\s)www\.([a-z0-9\-]+)\.([a-z0-9\-.\~]+)((?:/[^ <>{}()\n\r]*[^., <>{}()\n\r]?)?)#i", "$1<a href='http://www.$2.$3$4' rel='nofollow-clickable'>www.$2.$3$4</a>", $ret);
	$ret = preg_replace("#(\s)([a-z0-9\-_.]+)@([a-z0-9\-_.]+)\.([^,< \n\r]+)#i", "$1<a href=\"mailto:$2@$3.$4\">$2@$3.$4</a>", $ret);
	$ret = trim($ret);
	return $ret;
}

Now the source for the link in question:

<a href="http://www.test.com" rel="nofollow-function"><a href='http://www.test.com' rel='nofollow-clickable'>http://www.test.com</a></a>

the comment I submitted was:

<a href="http://www.test.com">http://www.test.com</a>

Obviously thats not correct.

Attachments (3)

functions-formatting.php.diff (1.0 KB) - added by ptvguy 16 years ago.
make_clickable without anchor wrapping
2889.20.diff (1.2 KB) - added by ryan 16 years ago.
2889.trunk.diff (1.1 KB) - added by ryan 16 years ago.

Download all attachments as: .zip

Change History (22)

#1 @ptvguy
16 years ago

I tried to duplicate this on the Sandbox blog, but moderation is turned on. I can't view the result.

#2 @ptvguy
16 years ago

Nonreproducable error. Possible mod interaction.

#3 @whooami
16 years ago

youre ability to reproduce it on my site is non-factor. I reproduced it 4 times? View the source of the page:

Source of Comment tagged as Markdown comment :

			<p>markdown comment:</p>

<p><a href="http://www.markdown.com" rel="nofollow-function"></a><a href="http://www.markdown.com" rel="nofollow-clickable">http://www.markdown.com</a>
</p>

		

Once again, does that look correct to you?

Ill tell you how to reproduce it:

Make sure you are allowing HTML links in your comments. Thats configurable via kses.php.

Then go comment on your own blog.

Ive reproduced this on 2 installs on 2 seperate servers.

#4 @whooami
16 years ago

and for the record, you didnt even submit the link in the proper format.

An HTML link is done like so:

<a href="http://www.link.com">http://www.link.com</a>

and that above is exactly what causes the problem.

NOT:

<a href="http://www.link.com">link here</a>

and not

<a href="http://www.link.com"></a>

which is, i might add, what you posted in your attempt to comment.

#5 @whooami
16 years ago

here it is again:

http://www.ladydelaluna.com/lara-kulpa/happy-independence-day/

Look at the source of my comment:

<p>
<a href="http://www.link.com" rel="nofollow"></a>
<a href="http://www.link.com" rel="nofollow">http://www.link.com</a></p>

Thats NOT valid XHTML and it is duplication because of the filtering.

#6 @ryan
16 years ago

This is caused by make_clickable() not properly handling anchors that have the scheme (http://) in the content.

#7 @whooami
16 years ago

well woooonhhoo, nice of you to verify that it does in fact exist, now a fix would be fabulous :)

#8 @ptvguy
16 years ago

  • Milestone set to 2.0.4

Try this (sans mod):

function make_clickable($ret) {
	$ret = ' ' . $ret . ' ';
	$ret = preg_replace("#([\s>])(https?)://([^\s<>{}()]+[^\s.,<>{}()])#i", "$1$2://$3", $ret);
	$ret = preg_replace("#(\s)www\.([a-z0-9\-]+)\.([a-z0-9\-.\~]+)((?:/[^ <>{}()\n\r]*[^., <>{}()\n\r]?)?)#i", "$1<a href='http://www.$2.$3$4' rel='nofollow'>www.$2.$3$4</a>", $ret);
	$ret = preg_replace("#(\s)([a-z0-9\-_.]+)@([a-z0-9\-_.]+)\.([^,< \n\r]+)#i", "$1<a href=\"mailto:$2@$3.$4\">$2@$3.$4</a>", $ret);
	$ret = trim($ret);
	return $ret;
}

@ptvguy
16 years ago

make_clickable without anchor wrapping

#9 @ptvguy
16 years ago

  • Keywords comment function anchor make_clickable bg|has_patch added

#10 @Libertus
16 years ago

ptvguy,

The anchor wrapping is necessary to make bare URLs clickable. As we discussed on IRC, the function will have to make a conditional check to ensure the text being made clickable is not already a child node of a link tag.

#11 @whooami
16 years ago

This has the same effect:

$ret = preg_replace("#(^|[\s>])(https?)://([^<>{}\s]+[^.,<>{}\s])#i", "$1<a href='$2://$3' rel='nofollow'>$2://$3</a>", $ret); 
$ret = preg_replace("#(^|[\n ])([\w]+?://[\w\#$%&~/.\-;:=,?@\[\]+]*)#is", "\\1<a href=\"\\2\" rel=\"nofollow\">\\2</a>", $ret);
$ret = preg_replace("#(^|[\n ])((www|ftp)\.[\w\#$%&~/.\-;:=,?@\[\]+]*)#is", "\\1<a href=\"http://\\2\" rel=\"nofollow\">\\2</a>", $ret);

The effect being that yes, they both fix the http:// problem but they introduce another problem: regular urls such as :

http://www.village-idiot.org

are no longer made clickable when not enclosed in tags.

I spose thats the tradeoff? or did you update that code in the time I took to type this?

#12 @whooami
16 years ago

What libertus said is exactly what I am describing as the introduced problem.

#13 @whooami
16 years ago

  • Resolution set to fixed
  • Status changed from new to closed

Ive fixed it the way it SHOULD be fixed:
bare urls work.

here is the complete function:

function make_clickable($ret) {
	$ret = ' ' . $ret;
	$ret = preg_replace("#(^|[\n ])([\w]+?://[\w\#$%&~/.\-;:=,?@\[\]+]*)#is", "$1<a href='$2' rel='nofollow'>$2</a>", $ret); //fix
	$ret = preg_replace("#(^|[\n ])((www|ftp)\.[\w\#$%&~/.\-;:=,?@\[\]+]*)#is", "$1<a href='http://$2' rel='nofollow'>$2</a>", $ret); //fix
	$ret = preg_replace("#(\s)([a-z0-9\-_.]+)@([^,< \n\r]+)#i", "$1<a href=\"mailto:$2@$3\">$2@$3</a>", $ret);
	$ret = substr($ret, 1);
	$ret = trim($ret);
	return $ret;
}

You're all welcome.

#14 @Nazgul
16 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Tickets should be closed after they're committed, not before.

#15 @ptvguy
16 years ago

Much better fix by whooami. I've already incorporated it into my own blog.

#16 @spencerp
16 years ago

Going along with what ptvguy has said, I've also incorporated it into my own blog, running 2.0.4-alpha. But, I've incorporated what you have done here whoo:

http://www.village-idiot.org/archives/2006/07/05/wordpress-bug-2889-fixed/

I have *not* tested this since adding the "fix", but if someone else wants to try it out, it's fine with me.

http://www.vindictivebastard.net/permalink-structure-should-i-i-dunno-yet.htm#respond

I'll just remove the comments when ever. =P And if all else fails, I'll just revert back to the original functions-formatting.php file from the SVN, and go from there. =P

@ryan
16 years ago

@ryan
16 years ago

#17 @ryan
16 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

(In [4011]) Make clickable fix from whooami. fixes #2889

#18 @ryan
16 years ago

(In [4012]) Make clickable fix from whooami. fixes #2889

#19 @(none)
15 years ago

  • Milestone 2.0.4 deleted

Milestone 2.0.4 deleted

Note: See TracTickets for help on using tickets.