Html sanitizer adds rel="nofollow" even if it pre-exists

[rahul-shedam]

Hi,

Consider the input as
String input = html <a href='www.google.com' rel='nofollow'></a>;

In my policy factory i have requireRelNofollowOnLinks() ON.

Expected output : html <a href='www.google.com' rel='nofollow'></a> ;

Actual output : html <a href="www.google.com" rel="nofollow" rel="nofollow"></a>

Sometimes it adds multiple rel="nofllow" which will result in more than 2 rel="follow" in single tag.

Thanks

[mikesamuel]

I can't repeat this bug. The following test works for me

  @Test
  public static final void testLinksWithNofollowAlreadyPresent() {
    assertEquals(
        "html <a href=\"/\" rel=\"nofollow\">link</a>",
        apply(
            new HtmlPolicyBuilder()
              .allowElements("a")
              .allowAttributes("href").onElements("a")
              .requireRelNofollowOnLinks(),
            "html <a href='/' rel='nofollow'>link</a>"));
  }

Do you have a test that repeats the problem you're seeing?

[fvschie]

The problem seems to be that rel handling in the html sanitizer sets the rel value, instead of appending to it. Which is an odd syntax.

I have a policy with
.allowAttributes("rel", [...]).onElements("a")

Which causes the duplication of rel content. Because some links do have rels from our editor and some don't, depending on customer needs/wants (and some randomness).

Given the above policy, and an enabled .requireRelNofollowOnLinks():

• If I have rel="noreferer" I would expect the end product to be rel="noreferer nofollow"
• If I have rel="nofollow" I would expect the end product to be rel="nofollow"

And similar for any rel value, obviously.

[mikesamuel]

I think I understand better now, but a testcase that you'd like to see pass would help.

[dhorlick]

Here is a failing test case:

package org.riverporpoise;

import static org.junit.Assert.assertEquals;

import org.junit.Test;
import org.owasp.html.HtmlPolicyBuilder;
import org.owasp.html.PolicyFactory;

public class AppTest 
{
    @Test
    public void issue145()
    {
		final PolicyFactory policy = new HtmlPolicyBuilder().allowElements("a")
				.allowAttributes("href", "target", "rel").onElements("a")
				.allowStandardUrlProtocols()
				.requireRelNofollowOnLinks()
				.toFactory();
		final String htmlContent = "<a href=\"http://google.com/\" rel=\"nofollow noopener noreferrer\" target=\"_blank\">hello</a>";
		final String sanitized = policy.sanitize(htmlContent);
		assertEquals(htmlContent, sanitized);
    }
}

This outputs:

[INFO] Running org.riverporpoise.AppTest
[ERROR] Tests run: 1, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 0.153 s <<< FAILURE! - in org.riverporpoise.AppTest
[ERROR] issue145(org.riverporpoise.AppTest)  Time elapsed: 0.107 s  <<< FAILURE!
org.junit.ComparisonFailure: expected:<... noopener noreferrer[]" target="_blank">he...> but was:<... noopener noreferrer[ nofollow noopener noreferrer]" target="_blank">he...>
	at org.riverporpoise.AppTest.issue145(AppTest.java:21)

All three values are duplicated.

Note that I have prescribed specific sequencing of rel values in my test, for brevity's sake.

[anudhuri23]

Is this issue resolved? Because I am still getting the same issue

[anudhuri23]

Here is a failing test case:

package org.riverporpoise;

import static org.junit.Assert.assertEquals;

import org.junit.Test;
import org.owasp.html.HtmlPolicyBuilder;
import org.owasp.html.PolicyFactory;

public class AppTest 
{
    @Test
    public void issue145()
    {
		final PolicyFactory policy = new HtmlPolicyBuilder().allowElements("a")
				.allowAttributes("href", "target", "rel").onElements("a")
				.allowStandardUrlProtocols()
				.requireRelNofollowOnLinks()
				.toFactory();
		final String htmlContent = "<a href=\"http://google.com/\" rel=\"nofollow noopener noreferrer\" target=\"_blank\">hello</a>";
		final String sanitized = policy.sanitize(htmlContent);
		assertEquals(htmlContent, sanitized);
    }
}

This outputs:

[INFO] Running org.riverporpoise.AppTest
[ERROR] Tests run: 1, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 0.153 s <<< FAILURE! - in org.riverporpoise.AppTest
[ERROR] issue145(org.riverporpoise.AppTest)  Time elapsed: 0.107 s  <<< FAILURE!
org.junit.ComparisonFailure: expected:<... noopener noreferrer[]" target="_blank">he...> but was:<... noopener noreferrer[ nofollow noopener noreferrer]" target="_blank">he...>
	at org.riverporpoise.AppTest.issue145(AppTest.java:21)

All three values are duplicated.

Note that I have prescribed specific sequencing of rel values in my test, for brevity's sake.

@dhorlick dhorlick Is your issue resolved? Please help me on getting it resolved

[mikesamuel]

Fixed in e8aa0f1