Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#804 Update Allowed Tags And Attributes #823

Merged
merged 27 commits into from
Dec 12, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
0b987a6
804 : Update allowed AMP tags in sanitizer file.
Dec 5, 2017
30064ca
804 : Prevent errors on amp_wp_build.py, update extensions dir.
Dec 5, 2017
ba459a2
Create is_mandatory_attribute_missing method
Dec 6, 2017
8cff768
804 : Modify PHPUnit tests for tag and attribute validation.
Dec 7, 2017
dd807a8
804 : Remove the requirement that 'data-multi-size' be empty.
Dec 7, 2017
a698eea
804 : PHPUnit tests for newly-allowed tags.
Dec 7, 2017
a066117
804 : Move logic for missing attributes.
Dec 7, 2017
cfcf730
804 : Remove the unused $spec_value variable.
Dec 7, 2017
8f0a925
804 : Improve variable alignment and add PHPDoc tags.
Dec 7, 2017
a400fa5
804 : Exclude the generated allowed tags file from PHPCS checks.
Dec 7, 2017
cfc0247
804 : Align 'array' keywords vertically.
Dec 7, 2017
7f9a2db
804 : Change the conditional to simply compare to true.
Dec 7, 2017
b4bfd81
804 : Array and equal sign alignment fixes.
Dec 7, 2017
6f5e39d
804 : Prevent accessing an object as an array.
Dec 7, 2017
2b1443e
Suppress DoubleError phpcs errors in test-tag-and-attribute-sanitizer…
westonruter Dec 8, 2017
c73f92b
[WIP] AMP HTML bash updater draft
ThierryA Dec 8, 2017
f67041d
[WIP] AMP HTML bash updater improvements
ThierryA Dec 8, 2017
236968b
Wrap up AMP HTML bash updater
ThierryA Dec 8, 2017
eab6207
804 : Add a contributing.md file, with steps to use build script.
Dec 9, 2017
eef4b8b
804 : Test that 'amp-playbuzz' is sanitized properly.
Dec 9, 2017
ae92a58
804 : Add allowed protocols to sanitization file.
Dec 9, 2017
4597be5
Fix WPCS warning
ThierryA Dec 11, 2017
44554c5
804 : Add to contributing.md, including unit test information.
Dec 11, 2017
ed3c93d
Merge branch 'feature/804-allowed-tags' of https://github.com/Automat…
Dec 11, 2017
c3b28c0
804 : Merge in develop, resolve conflicts.
Dec 11, 2017
4b3ec53
804 : Correct PHPUnit test for <script> validation.
Dec 12, 2017
af7b830
804 : Add spaces to correct PHPCS errors.
Dec 12, 2017
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
.DS_Store
vendor
221 changes: 221 additions & 0 deletions bin/amphtml-fix.diff
Original file line number Diff line number Diff line change
@@ -0,0 +1,221 @@
diff --git a/validator/validator_gen_md.py b/validator/validator_gen_md.py
new file mode 100644
index 000000000..80909fd4c
--- /dev/null
+++ b/validator/validator_gen_md.py
@@ -0,0 +1,215 @@
+#
+# Copyright 2015 The AMP HTML Authors. All Rights Reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS-IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the license.
+#
+"""Generates validator-generated.md"""
+
+import os
+
+
+def GenerateValidatorGeneratedMd(specfile, validator_pb2, text_format, out):
+ """Main method for the markdown generator.
+
+ This method reads the specfile and emits Markdown to sys.stdout.
+
+ Args:
+ specfile: Path to validator.protoascii.
+ validator_pb2: The proto2 Python module generated from validator.proto.
+ text_format: The text_format module from the protobuf package, e.g.
+ google.protobuf.text_format.
+ out: a list of lines to output (without the newline characters), to
+ which this function will append.
+ """
+ out.append('<!-- Generated by %s - do not edit. -->' %
+ os.path.basename(__file__))
+ out.append(
+ '<!-- WARNING: This does not fully reflect validator.protoascii yet. -->')
+ out.append('')
+ out.append('[Accelerated Mobile Pages Project](https://www.ampproject.org)')
+ out.append('')
+
+ out.append('# validator.protoascii')
+ out.append('')
+
+ rules = validator_pb2.ValidatorRules()
+ text_format.Merge(open(specfile).read(), rules)
+ if rules.HasField('spec_file_revision'):
+ out.append('* spec file revision: %d' % rules.spec_file_revision)
+ if rules.HasField('min_validator_revision_required'):
+ out.append('* minimum validator revision required: %d' %
+ rules.min_validator_revision_required)
+ out.append('')
+ out.append('Allowed Tags')
+ out.append('')
+ out.append('[TOC]')
+ out.append('')
+ for (field_desc, field_val) in rules.ListFields():
+ if field_desc.name == 'tags':
+ for tag_spec in field_val:
+ PrintTagSpec(validator_pb2, tag_spec, out)
+
+
+def GetLayout(validator_pb2, layout_index):
+ """Helper function that returns the AmpLayout.Layout name for a given index.
+
+ See amp.validator.AmpLayout.Layout in validator.proto for details.
+
+ Args:
+ validator_pb2: The proto2 Python module generated from validator.proto.
+ layout_index: integer representing a particular AmpLayout.Layout
+ Returns:
+ A string which represents the name for this supported layout.
+ """
+ amp_layout = validator_pb2.DESCRIPTOR.message_types_by_name['AmpLayout']
+ layouts = amp_layout.fields_by_name['supported_layouts'].enum_type.values
+ return layouts[layout_index].name
+
+
+def PrintAmpLayout(validator_pb2, amp_layout, out):
+ """Prints a Markdown version of the given proto message (AmpLayout).
+
+ See amp.validator.AmpLayout in validator.proto for details of proto message.
+
+ Args:
+ validator_pb2: The proto2 Python module generated from validator.proto.
+ amp_layout: The AmpLayout message.
+ out: A list of lines to output (without newline characters), to which this
+ function will append.
+ """
+ for layout in amp_layout.supported_layouts:
+ out.append('* %s' % UnicodeEscape(GetLayout(validator_pb2, layout)))
+ if amp_layout.defines_default_width:
+ out.append('* Defines Default Width')
+ if amp_layout.defines_default_height:
+ out.append('* Defines Default Height')
+
+
+def PrintAttrSpec(attr_spec, out):
+ """Prints a Markdown version of the given proto message (AttrSpec).
+
+ See amp.validator.AttrSpec in validator.proto for details of proto message.
+
+ Args:
+ attr_spec: The AttrSpec message.
+ out: A list of lines to output (without newline characters), to which this
+ function will append.
+ """
+ out.append('* %s' % UnicodeEscape(attr_spec.name))
+ if attr_spec.alternative_names:
+ out.append(' * Alternative Names: %s' %
+ RepeatedFieldToString(attr_spec.alternative_names))
+ if attr_spec.mandatory:
+ out.append(' * Mandatory')
+ if attr_spec.mandatory_oneof:
+ out.append(' * Mandatory One of: %s' % attr_spec.mandatory_oneof)
+ if attr_spec.value:
+ out.append(' * Required Value: %s' % attr_spec.value)
+
+
+def PrintTagSpec(validator_pb2, tag_spec, out):
+ """Prints a Markdown version of the given proto message (TagSpec).
+
+ See amp.validator.TagSpec in validator.proto for details of proto message.
+
+ Args:
+ validator_pb2: The proto2 Python module generated from validator.proto.
+ tag_spec: The TagSpec message.
+ out: A list of lines to output (without newline characters), to which this
+ function will append.
+ """
+ header = '## %s' % UnicodeEscape(tag_spec.tag_name)
+ if tag_spec.spec_name and (tag_spec.tag_name != tag_spec.spec_name):
+ header += ': %s' % UnicodeEscape(tag_spec.spec_name)
+ if tag_spec.deprecation:
+ header += ' (DEPRECATED)'
+ header += '{#%s_%s}' % (UnicodeEscape(tag_spec.tag_name).replace(' ', '_'),
+ UnicodeEscape(tag_spec.spec_name).replace(' ', '_'))
+ out.append('')
+ out.append(header)
+ out.append('')
+ if tag_spec.deprecation:
+ out.append('This is deprecated.')
+ out.append('Please see [%s](%s)' %
+ (UnicodeEscape(tag_spec.deprecation), tag_spec.deprecation_url))
+ out.append('')
+ if tag_spec.mandatory:
+ out.append('* Mandatory')
+ if tag_spec.mandatory_alternatives:
+ out.append('* Mandatory Alternatives: %s' %
+ UnicodeEscape(tag_spec.mandatory_alternatives))
+ if tag_spec.unique:
+ out.append('* Unique')
+ if tag_spec.mandatory_parent:
+ out.append('* Mandatory Parent: %s' %
+ UnicodeEscape(tag_spec.mandatory_parent))
+ if tag_spec.mandatory_ancestor:
+ out.append('* Mandatory Ancestor: %s' %
+ UnicodeEscape(tag_spec.mandatory_ancestor))
+ if tag_spec.mandatory_ancestor_suggested_alternative:
+ out.append('* Mandatory Ancestor Alternative: %s' %
+ UnicodeEscape(tag_spec.mandatory_ancestor_suggested_alternative))
+ if tag_spec.disallowed_ancestor:
+ out.append('* Disallowed Ancestor: %s' %
+ RepeatedFieldToString(tag_spec.disallowed_ancestor))
+ if tag_spec.attrs:
+ out.append('')
+ out.append('Allowed Attributes:')
+ out.append('')
+ for attr_spec in tag_spec.attrs:
+ PrintAttrSpec(attr_spec, out)
+ if (tag_spec.amp_layout.supported_layouts or
+ tag_spec.amp_layout.defines_default_width or
+ tag_spec.amp_layout.defines_default_height):
+ out.append('')
+ out.append('Allowed Layouts:')
+ out.append('')
+ PrintAmpLayout(validator_pb2, tag_spec.amp_layout, out)
+ if tag_spec.spec_url:
+ out.append('')
+ out.append('[Spec](%s)' % tag_spec.spec_url)
+ out.append('')
+
+
+def RepeatedFieldToString(field):
+ """Helper function which converts a list into an escaped string.
+
+ Args:
+ field: A list of strings.
+ Returns:
+ A string, segmented by commas.
+ """
+ return ', '.join([UnicodeEscape(s) for s in field])
+
+
+def UnderscoreToTitleCase(under_score):
+ """Helper function which converts under_score names to TitleCase.
+
+ Args:
+ under_score: A name, segmented by under_scores.
+ Returns:
+ A name, segmented as TitleCase.
+ """
+ segments = under_score.split('_')
+ return ' '.join([s.title() for s in segments])
+
+
+def UnicodeEscape(string):
+ """Helper function which escapes unicode characters.
+
+ Args:
+ string: A string which may contain unicode characters.
+ Returns:
+ An escaped string.
+ """
+ return ('' + string).encode('unicode-escape')
8 changes: 4 additions & 4 deletions bin/amp_wp_build.py → bin/amphtml-update.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,11 +83,11 @@ def GenValidatorProtoascii(out_dir):
assert re.match(r'^[a-zA-Z_\-0-9]+$', out_dir), 'bad out_dir: %s' % out_dir

protoascii_segments = [open('validator-main.protoascii').read()]
extensions = glob.glob('extensions/*/0.1/validator-*.protoascii')
extensions = glob.glob('extensions/*/validator-*.protoascii')
# In the Github project, the extensions are located in a sibling directory
# to the validator rather than a child directory.
if not extensions:
extensions = glob.glob('../extensions/*/0.1/validator-*.protoascii')
extensions = glob.glob('../extensions/*/validator-*.protoascii')
extensions.sort()
for extension in extensions:
protoascii_segments.append(open(extension).read())
Expand Down Expand Up @@ -282,7 +282,7 @@ def GenerateValuesPHP(out, values, indent_level = 6):
if isinstance(value, (str, bool)):
out.append('%s\'%s\' => \'%s\',' % (indent, key.lower(), value))

if isinstance(value, list):
else:
out.append('%s\'%s\' => array(' % (indent, key.lower()))
sorted_value = sorted(value)
for v in sorted_value:
Expand Down Expand Up @@ -428,7 +428,7 @@ def GetTagRules(tag_spec):

tag_rules = {}

if tag_spec.also_requires_tag:
if hasattr(tag_spec, 'also_requires_tag') and tag_spec.also_requires_tag:
also_requires_tag_list = []
for also_requires_tag in tag_spec.also_requires_tag:
also_requires_tag_list.append(UnicodeEscape(also_requires_tag))
Expand Down
44 changes: 44 additions & 0 deletions bin/amphtml-update.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
#!/bin/bash
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is a draft the bash script to update AMP HTML. This script is meant to run in Linux environment (ex. VVV). To run the script, simply cd to the plugin root and run bash bin/amphtml-update.sh.

set -e

# Go to the right location.
cd "$(dirname "$0")"

BIN_PATH="$(pwd)"
PROJECT_PATH=$(dirname $PWD)
VENDOR_PATH=$PROJECT_PATH/vendor

if ! command -v apt-get >/dev/null 2>&1; then
echo "The AMP HTML uses apt-get, make sure to run this script in a Linux environment"
exit 1
fi

# Install dependencies.
sudo apt-get install git python protobuf-compiler python-protobuf

# Create and go to vendor.
if [[ ! -e $VENDOR_PATH ]]; then
mkdir $VENDOR_PATH
fi
cd $VENDOR_PATH

# Clone amphtml repo.
if [[ ! -e $VENDOR_PATH/amphtml ]]; then
git clone https://github.com/ampproject/amphtml amphtml
else
cd $VENDOR_PATH/amphtml/validator
git pull
fi

# Copy script to location and go there.
cp $BIN_PATH/amphtml-update.py $VENDOR_PATH/amphtml/validator
cd $VENDOR_PATH/amphtml/validator

# Temporary fix until https://github.com/ampproject/amphtml/issues/12371 is addressed.
if [ ! -f $VENDOR_PATH/amphtml/validator/validator_gen_md.py ]; then
git apply $BIN_PATH/amphtml-fix.diff
fi

# Run script.
python amphtml-update.py
cp amp_wp/class-amp-allowed-tags-generated.php ../../../includes/sanitizers/
34 changes: 34 additions & 0 deletions contributing.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
# AMP Contributing Guide

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps add a welcoming sentence (ex. "Thanks for taking the time to contribute!")

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @ThierryA. This commit adds your sentence to contributing.md:

Thanks for taking the time to contribute!

Thanks for taking the time to contribute!

To clone this repository
``` bash
$ git clone --recursive [email protected]:Automattic/amp-wp.git
```

### Updating Allowed Tags And Attributes

The file `class-amp-allowed-tags-generated.php` has the AMP specification's allowed tags and attributes. It's used in sanitization.
To update that file:
1. `cd` to the root of this plugin
2. run `bash bin/amphtml-update.sh`
That script is intended for a Linux environment like [VVV](https://github.com/Varying-Vagrant-Vagrants/VVV).

### PHPUnit Testing
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The way the dev lib runs PHPUnit (run_phpunit_local) needs to run in an environment which has WordPress Unit Test installed, for example VVV. I would advise to add that as a note.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this came up in #828,.along with questions about how to get pre-commit hook working from wp-dev-lib.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @ThierryA. This commit adds your point to contributing.md:

Please run these tests in an environment with WordPress unit tests installed, like [VVV](https://github.com/Varying-Vagrant-Vagrants/VVV).


Please run these tests in an environment with WordPress unit tests installed, like [VVV](https://github.com/Varying-Vagrant-Vagrants/VVV).

Run tests:

``` bash
$ phpunit
```

Run tests with an HTML coverage report:

``` bash
$ phpunit --coverage-html /tmp/report
```

When you push a commit to your PR, Travis CI will run the PHPUnit tests and sniffs against the WordPress Coding Standards.
Loading