am e5d18314: fixed issue 23 : ANDing two policies was confused by allowWithoutAttributes overrides of elements like <img>,<a>,<span> that are by-default dropped without elements

* commit 'e5d1831401c6302339a6902f790d7c133f8a4b55':
  fixed issue 23 : ANDing two policies was confused by allowWithoutAttributes overrides of elements like <img>,<a>,<span> that are by-default dropped without elements

2 files changed, 39 insertions, 1 deletions

diff --git a/src/main/org/owasp/html/ElementAndAttributePolicies.java b/src/main/org/owasp/html/ElementAndAttributePolicies.java
index 0d372c2..3cc841e 100644
--- a/src/main/org/owasp/html/ElementAndAttributePolicies.java
+++ b/src/main/org/owasp/html/ElementAndAttributePolicies.java
@@ -78,10 +78,24 @@ final class ElementAndAttributePolicies {
         joinedAttrPolicies.put(attrName, e.getValue());
       }
     }
+
+    // HACK: this is attempting to recognize when skipIfEmpty has been
+    // explicitly set in HtmlPolicyBuilder and can only make a best effort at
+    // that and is also too tightly coupled with HtmlPolicyBuilder.
+    // Maybe go tri-state.
+    boolean combinedSkipIfEmpty;
+    if (HtmlPolicyBuilder.DEFAULT_SKIP_IF_EMPTY.contains(elementName)) {
+      // Either policy explicitly opted out of skip if empty.
+      combinedSkipIfEmpty = skipIfEmpty && p.skipIfEmpty;
+    } else {
+      // Either policy explicitly specified skip if empty.
+      combinedSkipIfEmpty = skipIfEmpty || p.skipIfEmpty;
+    }
+
     return new ElementAndAttributePolicies(
         elementName,
         ElementPolicy.Util.join(elPolicy, p.elPolicy),
         joinedAttrPolicies.build(),
-        skipIfEmpty || p.skipIfEmpty);
+        combinedSkipIfEmpty);
   }
 }
diff --git a/src/tests/org/owasp/html/SanitizersTest.java b/src/tests/org/owasp/html/SanitizersTest.java
index 052ab02..68f621c 100644
--- a/src/tests/org/owasp/html/SanitizersTest.java
+++ b/src/tests/org/owasp/html/SanitizersTest.java
@@ -248,4 +248,28 @@ public class SanitizersTest extends TestCase {
         + "</table>";
     assertEquals(sanitized, s.sanitize(input));
   }
+
+  @Test
+  public static final void testSkipIfEmptyUnionsProperly() {
+    // Issue 23
+    PolicyFactory extras = new HtmlPolicyBuilder()
+        .allowWithoutAttributes("span", "div")
+        .allowElements("span", "div", "textarea")
+        // This is not the proper way to require the attribute disabled on
+        // textareas.  This is a test.  This is only a test.
+        .allowAttributes("disabled").onElements("textarea")
+        .disallowWithoutAttributes("textarea")
+        .toFactory();
+    PolicyFactory policy = Sanitizers.FORMATTING
+        .and(Sanitizers.BLOCKS)
+        .and(Sanitizers.IMAGES)
+        .and(Sanitizers.STYLES)
+        .and(extras);
+    String input =
+        "<textarea>text</textarea><textarea disabled></textarea>"
+        + "<div onclick='redirect()'><span>Styled by span</span></div>";
+    String want = "text<textarea disabled=\"disabled\"></textarea>"
+        + "<div><span>Styled by span</span></div>";
+    assertEquals(want, policy.sanitize(input));
+  }
 }