Merge pull request ampproject#7597 from ampproject/fix/siblings-disal…

…lowed
rtCamp · Aug 3, 2023 · 1aa9a63 · 1aa9a63
2 parents d421f58 + 7fca058
commit 1aa9a63
Show file tree

Hide file tree

Showing 6 changed files with 194 additions and 2 deletions.
diff --git a/bin/amphtml-update.py b/bin/amphtml-update.py
@@ -799,6 +799,9 @@ def GetTagRules(tag_spec):
  if tag_spec.HasField('unique_warning'):
  tag_rules['unique_warning'] = tag_spec.unique_warning
 
+ if tag_spec.HasField('siblings_disallowed'):
+ tag_rules['siblings_disallowed'] = tag_spec.siblings_disallowed
+
  if tag_spec.HasField('child_tags'):
  child_tags = collections.defaultdict( lambda: [] )
  for field in tag_spec.child_tags.ListFields():

diff --git a/bin/latest-extension-versions.json b/bin/latest-extension-versions.json
@@ -115,6 +115,7 @@
  "amp-sticky-ad" : "1.0",
  "amp-story" : "1.0",
  "amp-story-360" : "0.1",
+ "amp-story-audio-sticker" : "0.1",
  "amp-story-auto-ads" : "0.1",
  "amp-story-auto-analytics" : "0.1",
  "amp-story-captions" : "0.1",

diff --git a/includes/sanitizers/class-amp-allowed-tags-generated.php b/includes/sanitizers/class-amp-allowed-tags-generated.php
diff --git a/includes/sanitizers/class-amp-rule-spec.php b/includes/sanitizers/class-amp-rule-spec.php
@@ -37,6 +37,7 @@ abstract class AMP_Rule_Spec {
  const MANDATORY_PARENT = 'mandatory_parent';
  const DESCENDANT_TAG_LIST = 'descendant_tag_list';
  const CHILD_TAGS = 'child_tags';
+ const SIBLINGS_DISALLOWED = 'siblings_disallowed';
 
  /*
  * HTML Element Attribute rule names

diff --git a/includes/sanitizers/class-amp-tag-and-attribute-sanitizer.php b/includes/sanitizers/class-amp-tag-and-attribute-sanitizer.php
@@ -45,6 +45,7 @@ class AMP_Tag_And_Attribute_Sanitizer extends AMP_Base_Sanitizer {
  const DISALLOWED_CHILD_TAG = 'DISALLOWED_CHILD_TAG';
  const DISALLOWED_DESCENDANT_TAG = 'DISALLOWED_DESCENDANT_TAG';
  const DISALLOWED_FIRST_CHILD_TAG = 'DISALLOWED_FIRST_CHILD_TAG';
+ const DISALLOWED_SIBLING_TAG = 'DISALLOWED_SIBLING_TAG';
  const DISALLOWED_PROCESSING_INSTRUCTION = 'DISALLOWED_PROCESSING_INSTRUCTION';
  const DISALLOWED_PROPERTY_IN_ATTR_VALUE = 'DISALLOWED_PROPERTY_IN_ATTR_VALUE';
  const DISALLOWED_RELATIVE_URL = 'DISALLOWED_RELATIVE_URL';
@@ -749,6 +750,11 @@ static function ( $validation_error ) {
  }
  }
 
+ // If the node disallows any siblings, remove them.
+ if ( ! empty( $tag_spec[ AMP_Rule_Spec::SIBLINGS_DISALLOWED ] ) ) {
+ $this->remove_disallowed_siblings( $node, $this->get_spec_name( $node, $tag_spec ) );
+ }
+
  // After attributes have been sanitized (and potentially removed), if mandatory attribute(s) are missing, remove the element.
  $missing_mandatory_attributes = $this->get_missing_mandatory_attributes( $attr_spec_list, $node );
  if ( ! empty( $missing_mandatory_attributes ) ) {
@@ -2453,6 +2459,46 @@ private function remove_disallowed_descendants( DOMElement $node, $allowed_desce
  }
  }
 
+ /**
+ * Loop through node's siblings and remove them.
+ *
+ * @param DOMElement $node Node.
+ * @param string $spec_name Spec name.
+ */
+ private function remove_disallowed_siblings( DOMElement $node, $spec_name ) {
+ $prev_sibling = $node->previousSibling;
+ while ( null !== $prev_sibling ) {
+ $prev_prev_sibling = $prev_sibling->previousSibling;
+ if ( $prev_sibling instanceof Element ) {
+ $this->remove_invalid_child(
+ $prev_sibling,
+ [
+ 'code' => self::DISALLOWED_SIBLING_TAG,
+ 'sibling' => $node->nodeName,
+ 'spec_name' => $spec_name,
+ ]
+ );
+ }
+ $prev_sibling = $prev_prev_sibling;
+ }
+
+ $next_sibling = $node->nextSibling;
+ while ( null !== $next_sibling ) {
+ $next_next_sibling = $next_sibling->nextSibling;
+ if ( $next_sibling instanceof Element ) {
+ $this->remove_invalid_child(
+ $next_sibling,
+ [
+ 'code' => self::DISALLOWED_SIBLING_TAG,
+ 'sibling' => $node->nodeName,
+ 'spec_name' => $spec_name,
+ ]
+ );
+ }
+ $next_sibling = $next_next_sibling;
+ }
+ }
+
  /**
  * Check whether the node validates the constraints for children.
  *

diff --git a/tests/php/test-tag-and-attribute-sanitizer.php b/tests/php/test-tag-and-attribute-sanitizer.php
@@ -1397,6 +1397,48 @@ static function() {
  [ 'amp-sidebar', 'amp-story' ],
  ],
 
+ 'amp_story_with_disallowed_sibling' => [
+ str_replace(
+ [ "\n", "\t" ],
+ '',
+ '
+ <b id="disallowed-before1"></b>
+ <i id="disallowed-before2"></i>
+ <amp-story standalone="standalone" title="Stories in AMP - Hello World" publisher="AMP Project" publisher-logo-src="https://ampbyexample.com/favicons/coast-228x228.png" poster-portrait-src="https://ampbyexample.com/img/story_dog2_portrait.jpg">
+ <amp-story-page id="cover">
+ <amp-story-grid-layer template="fill">
+ <h1>Hello World</h1>
+ <p>This is the cover page of this story.</p>
+ </amp-story-grid-layer>
+ </amp-story-page>
+ </amp-story>
+ <span id="disallowed-after1"></span>
+ <div id="disallowed-after2"></div>
+ '
+ ),
+ str_replace(
+ [ "\n", "\t" ],
+ '',
+ '
+ <amp-story standalone="standalone" title="Stories in AMP - Hello World" publisher="AMP Project" publisher-logo-src="https://ampbyexample.com/favicons/coast-228x228.png" poster-portrait-src="https://ampbyexample.com/img/story_dog2_portrait.jpg">
+ <amp-story-page id="cover">
+ <amp-story-grid-layer template="fill">
+ <h1>Hello World</h1>
+ <p>This is the cover page of this story.</p>
+ </amp-story-grid-layer>
+ </amp-story-page>
+ </amp-story>
+ '
+ ),
+ [ 'amp-story' ],
+ [
+ AMP_Tag_And_Attribute_Sanitizer::DISALLOWED_SIBLING_TAG,
+ AMP_Tag_And_Attribute_Sanitizer::DISALLOWED_SIBLING_TAG,
+ AMP_Tag_And_Attribute_Sanitizer::DISALLOWED_SIBLING_TAG,
+ AMP_Tag_And_Attribute_Sanitizer::DISALLOWED_SIBLING_TAG,
+ ],
+ ],
+
  'amp_sidebar_with_autoscroll' => [
  str_replace(
  [ "\n", "\t" ],