Form with <optgroup> incorrectly rewritten

[baptistegrimaud]

When using <optgroup> within a <select> element, the sanitized HTML is invalid and contains extra <select> tags.
The following HTML:

<form><select><optgroup label="mygroup"><option>My option</option></optgroup></select></form>

would become:

<form><select><optgroup label="mygroup"><select><option>My option</option></select></optgroup></select></form>

Note that without the <optgroup> tag, the sanitized HTML is correct.

Unit test to reproduce the issue (with version 20240325.1):

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

import static org.junit.Assert.assertEquals;

public class OptgroupTest {

    @Test
    public void not_working() {
        HtmlPolicyBuilder policyBuilder = new HtmlPolicyBuilder();
        PolicyFactory factory = policyBuilder.allowElements("form", "select", "optgroup", "option").allowAttributes("label").globally().toFactory();
        String html = "<form><select><optgroup label=\"mygroup\"><option>My option</option></optgroup></select></form>";
        String result = factory.sanitize(html);
        assertEquals(html, result); // this fails!
//        Expected :<form><select><optgroup label="mygroup"><option>My option</option></optgroup></select></form>
//        Actual   :<form><select><optgroup label="mygroup"><select><option>My option</option></select></optgroup></select></form>
    }

    @Test
    public void working() {
        HtmlPolicyBuilder policyBuilder = new HtmlPolicyBuilder();
        PolicyFactory factory = policyBuilder.allowElements("form", "select", "option").toFactory();
        String html = "<form><select><option>My option</option></select></form>";
        String result = factory.sanitize(html);
        assertEquals(html, result);
    }
}

Is there anything missing when creating the PolicyFactory to properly support <optgroup>?

NB: Same problem seems to happen with <datalist> tags.

[Shangeeth29]

Hi @baptistegrimaud
I’ve opened a PR with the fix here: #375

[melloware]

@jmanico this issue can be closed now that its merged