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.

Sign up for GitHub

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

Jump to bottom

Fix dispatch_key handling for NAME_VALUE_DISPATCH; add cdata validation #926

Merged
merged 5 commits into from
Feb 4, 2018
Merged

Fix dispatch_key handling for NAME_VALUE_DISPATCH; add cdata validation #926

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6030e2d
Include consideration of dispatch_key when validating to fix amp-ad[d…
westonruter Feb 3, 2018
a8fb194
Add tests to try validating NAME_VALUE_PARENT_DISPATCH and NAME_DISPATCH
westonruter Feb 3, 2018
e216aee
Include cdata in AMP_Allowed_Tags_Generated
westonruter Feb 3, 2018
f986804
Add blacklisted_cdata_regex validation
westonruter Feb 3, 2018
306f53d
Add validation of amp-keyframes styles
westonruter Feb 3, 2018
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
37 changes: 29 additions & 8 deletions bin/amphtml-update.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -248,18 +248,22 @@ def GeneratePropertiesPHP(out, properties, indent_level = 5):
sorted_properties = sorted(properties.items())
for (prop, values) in collections.OrderedDict(sorted_properties).iteritems():
logging.info('generating php for property: %s...' % prop.lower())
if isinstance(values, (str, bool)):
if isinstance(values, str):
if isinstance(values, (unicode,str, bool)):
if isinstance(values, (unicode,str)):
values = values.lower()
out.append('%s\'%s\' => \'%s\',' % (indent, prop.lower(), values))
elif isinstance(values, (int)):
out.append('%s\'%s\' => %d,' % (indent, prop.lower(), values))
else:
out.append('%s\'%s\' => array(' % (indent, prop.lower()))
sorted_values = sorted(values.items())
for(value_type, value) in collections.OrderedDict(sorted_values).iteritems():
if isinstance(value, (str, bool)):
for(key, value) in collections.OrderedDict(sorted_values).iteritems():
if isinstance(value, (unicode, str, bool)):
if isinstance(value, str):
value = value.lower()
out.append('%s\t\'%s\' => \'%s\',' % (indent, value_type, value))
out.append('%s\t\'%s\' => \'%s\',' % (indent, key, value))
elif isinstance(value, (int)):
out.append('%s\t\'%s\' => %d,' % (indent, key, value))
else:
GenerateValuesPHP(out, value)
out.append('%s),' % indent)
Expand All @@ -284,6 +288,9 @@ def GenerateValuesPHP(out, values, indent_level = 6):
if isinstance(value, (str, bool)):
out.append('%s\'%s\' => \'%s\',' % (indent, key.lower(), value))

elif isinstance(value, (int)):
out.append('%s\'%s\' => %d,' % (indent, key.lower(), value))

else:
out.append('%s\'%s\' => array(' % (indent, key.lower()))
sorted_value = sorted(value)
Expand Down Expand Up @@ -435,7 +442,6 @@ def GetTagSpec(tag_spec, attr_lists):

tag_dict = GetTagRules(tag_spec)
attr_dict = GetAttrs(tag_spec.attrs)
# TODO: add CDATA section if validation of non-body elements is required.

# Now add attributes from any attribute lists to this tag.
for (tag_field_desc, tag_field_val) in tag_spec.ListFields():
Expand All @@ -444,7 +450,22 @@ def GetTagSpec(tag_spec, attr_lists):
attr_dict.update(attr_lists[UnicodeEscape(attr_list)])

logging.info('... done')
return {'tag_spec':tag_dict, 'attr_spec_list':attr_dict}
tag_spec_dict = {'tag_spec':tag_dict, 'attr_spec_list':attr_dict}
if tag_spec.HasField('cdata'):
cdata_dict = {}
for (field_descriptor, field_value) in tag_spec.cdata.ListFields():
if isinstance(field_value, (unicode, str, bool, int)):
cdata_dict[ field_descriptor.name ] = field_value
else:
if hasattr( field_value, '_values' ):
cdata_dict[ field_descriptor.name ] = {}
for _value in field_value._values:
for (key,val) in _value.ListFields():
cdata_dict[ field_descriptor.name ][ key.name ] = val
if len( cdata_dict ) > 0:
tag_spec_dict['cdata'] = cdata_dict

return tag_spec_dict


def GetTagRules(tag_spec):
Expand Down Expand Up @@ -552,7 +573,7 @@ def GetValues(attr_spec):
if attr_spec.HasField('blacklisted_value_regex'):
value_dict['blacklisted_value_regex'] = UnicodeEscape(attr_spec.blacklisted_value_regex)

# dispatch_key is a boolean
# dispatch_key is an int
if attr_spec.HasField('dispatch_key'):
value_dict['dispatch_key'] = attr_spec.dispatch_key

Expand Down
70 changes: 70 additions & 0 deletions includes/sanitizers/class-amp-allowed-tags-generated.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,9 @@ class AMP_Allowed_Tags_Generated {
'attr_spec_list' => array(
'alt' => array(),
'data-multi-size' => array(
'dispatch_key' => 2,
'mandatory' => true,
'value' => '',
),
'json' => array(),
'media' => array(),
Expand Down Expand Up @@ -746,6 +748,7 @@ class AMP_Allowed_Tags_Generated {
'attr_spec_list' => array(
'alt' => array(),
'data-multi-size' => array(
'dispatch_key' => 2,
'mandatory' => true,
'value' => '',
),
Expand Down Expand Up @@ -2948,6 +2951,7 @@ class AMP_Allowed_Tags_Generated {
'attr_spec_list' => array(
'align' => array(),
'submitting' => array(
'dispatch_key' => 1,
'mandatory' => true,
),
),
Expand All @@ -2961,6 +2965,7 @@ class AMP_Allowed_Tags_Generated {
'attr_spec_list' => array(
'align' => array(),
'submit-success' => array(
'dispatch_key' => 1,
'mandatory' => true,
),
),
Expand All @@ -2974,6 +2979,7 @@ class AMP_Allowed_Tags_Generated {
'attr_spec_list' => array(
'align' => array(),
'submit-error' => array(
'dispatch_key' => 1,
'mandatory' => true,
),
),
Expand Down Expand Up @@ -3965,6 +3971,7 @@ class AMP_Allowed_Tags_Generated {
),
'enctype' => array(),
'method' => array(
'dispatch_key' => 2,
'mandatory' => true,
'value_casei' => 'post',
),
Expand Down Expand Up @@ -4942,6 +4949,7 @@ class AMP_Allowed_Tags_Generated {
),
'hreflang' => array(),
'itemprop' => array(
'dispatch_key' => 2,
'mandatory' => true,
'value_casei' => 'sameas',
),
Expand Down Expand Up @@ -5239,6 +5247,7 @@ class AMP_Allowed_Tags_Generated {
),
),
'http-equiv' => array(
'dispatch_key' => 2,
'mandatory' => true,
'value_casei' => 'x-ua-compatible',
),
Expand Down Expand Up @@ -5279,6 +5288,7 @@ class AMP_Allowed_Tags_Generated {
'value_casei' => 'text/html; charset=utf-8',
),
'http-equiv' => array(
'dispatch_key' => 2,
'mandatory' => true,
'value_casei' => 'content-type',
),
Expand All @@ -5300,6 +5310,7 @@ class AMP_Allowed_Tags_Generated {
'mandatory' => true,
),
'http-equiv' => array(
'dispatch_key' => 2,
'mandatory' => true,
'value_casei' => 'content-language',
),
Expand All @@ -5321,6 +5332,7 @@ class AMP_Allowed_Tags_Generated {
'mandatory' => true,
),
'http-equiv' => array(
'dispatch_key' => 2,
'mandatory' => true,
'value_casei' => 'pics-label',
),
Expand All @@ -5342,6 +5354,7 @@ class AMP_Allowed_Tags_Generated {
'mandatory' => true,
),
'http-equiv' => array(
'dispatch_key' => 2,
'mandatory' => true,
'value_casei' => 'imagetoolbar',
),
Expand All @@ -5364,6 +5377,7 @@ class AMP_Allowed_Tags_Generated {
'value_casei' => 'text/css',
),
'http-equiv' => array(
'dispatch_key' => 2,
'mandatory' => true,
'value_casei' => 'content-style-type',
),
Expand All @@ -5386,6 +5400,7 @@ class AMP_Allowed_Tags_Generated {
'value_casei' => 'text/javascript',
),
'http-equiv' => array(
'dispatch_key' => 2,
'mandatory' => true,
'value_casei' => 'content-script-type',
),
Expand All @@ -5407,6 +5422,7 @@ class AMP_Allowed_Tags_Generated {
'mandatory' => true,
),
'http-equiv' => array(
'dispatch_key' => 2,
'mandatory' => true,
'value_casei' => 'origin-trial',
),
Expand All @@ -5428,6 +5444,7 @@ class AMP_Allowed_Tags_Generated {
'mandatory' => true,
),
'http-equiv' => array(
'dispatch_key' => 2,
'mandatory' => true,
'value_casei' => 'resource-type',
),
Expand Down Expand Up @@ -5500,6 +5517,7 @@ class AMP_Allowed_Tags_Generated {
array(
'attr_spec_list' => array(
'toolbar' => array(
'dispatch_key' => 1,
'mandatory' => true,
),
'toolbar-target' => array(
Expand Down Expand Up @@ -6307,10 +6325,17 @@ class AMP_Allowed_Tags_Generated {
'attr_spec_list' => array(
'nonce' => array(),
'type' => array(
'dispatch_key' => 2,
'mandatory' => true,
'value_casei' => 'application/ld+json',
),
),
'cdata' => array(
'blacklisted_cdata_regex' => array(
'error_message' => 'html comments',
'regex' => '<!--',
),
),
'tag_spec' => array(
'spec_name' => 'script type=application/ld+json',
),
Expand All @@ -6319,10 +6344,17 @@ class AMP_Allowed_Tags_Generated {
array(
'attr_spec_list' => array(
'type' => array(
'dispatch_key' => 3,
'mandatory' => true,
'value_casei' => 'application/json',
),
),
'cdata' => array(
'blacklisted_cdata_regex' => array(
'error_message' => 'html comments',
'regex' => '<!--',
),
),
'tag_spec' => array(
'html_format' => array(
'amp',
Expand All @@ -6336,6 +6368,7 @@ class AMP_Allowed_Tags_Generated {
'attr_spec_list' => array(
'nonce' => array(),
'type' => array(
'dispatch_key' => 3,
'mandatory' => true,
'value' => 'application/json',
),
Expand All @@ -6357,10 +6390,17 @@ class AMP_Allowed_Tags_Generated {
'attr_spec_list' => array(
'nonce' => array(),
'type' => array(
'dispatch_key' => 3,
'mandatory' => true,
'value_casei' => 'application/json',
),
),
'cdata' => array(
'blacklisted_cdata_regex' => array(
'error_message' => 'html comments',
'regex' => '<!--',
),
),
'tag_spec' => array(
'html_format' => array(
'amp',
Expand All @@ -6379,10 +6419,17 @@ class AMP_Allowed_Tags_Generated {
'attr_spec_list' => array(
'nonce' => array(),
'type' => array(
'dispatch_key' => 3,
'mandatory' => true,
'value_casei' => 'application/json',
),
),
'cdata' => array(
'blacklisted_cdata_regex' => array(
'error_message' => 'html comments',
'regex' => '<!--',
),
),
'tag_spec' => array(
'mandatory_parent' => 'amp-animation',
'requires_extension' => array(
Expand All @@ -6396,10 +6443,17 @@ class AMP_Allowed_Tags_Generated {
'attr_spec_list' => array(
'nonce' => array(),
'type' => array(
'dispatch_key' => 3,
'mandatory' => true,
'value_casei' => 'application/json',
),
),
'cdata' => array(
'blacklisted_cdata_regex' => array(
'error_message' => 'html comments',
'regex' => '<!--',
),
),
'tag_spec' => array(
'html_format' => array(
'amp',
Expand All @@ -6417,10 +6471,17 @@ class AMP_Allowed_Tags_Generated {
'attr_spec_list' => array(
'nonce' => array(),
'type' => array(
'dispatch_key' => 3,
'mandatory' => true,
'value_casei' => 'application/json',
),
),
'cdata' => array(
'blacklisted_cdata_regex' => array(
'error_message' => 'html comments',
'regex' => '<!--',
),
),
'tag_spec' => array(
'html_format' => array(
'amp',
Expand Down Expand Up @@ -6795,11 +6856,15 @@ class AMP_Allowed_Tags_Generated {
array(
'attr_spec_list' => array(
'amp-boilerplate' => array(
'dispatch_key' => 3,
'mandatory' => true,
'value' => '',
),
'nonce' => array(),
),
'cdata' => array(
'cdata_regex' => '\s*body{-webkit-animation:none;-moz-animation:none;-ms-animation:none;animation:none}\s*',
),
'tag_spec' => array(
'html_format' => array(
'amp',
Expand All @@ -6816,10 +6881,15 @@ class AMP_Allowed_Tags_Generated {
array(
'attr_spec_list' => array(
'amp-keyframes' => array(
'dispatch_key' => 1,
'mandatory' => true,
'value' => '',
),
),
'cdata' => array(
'max_bytes' => 500000,
'max_bytes_spec_url' => 'https://www.ampproject.org/docs/reference/spec#keyframes-stylesheet',
),
'tag_spec' => array(
'html_format' => array(
'amp',
Expand Down
18 changes: 18 additions & 0 deletions includes/sanitizers/class-amp-rule-spec.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ abstract class AMP_Rule_Spec {
*/
const ATTR_SPEC_LIST = 'attr_spec_list';
const TAG_SPEC = 'tag_spec';
const CDATA = 'cdata';

/**
* AMP attr_spec value check results
Expand Down Expand Up @@ -47,6 +48,23 @@ abstract class AMP_Rule_Spec {
const VALUE_REGEX = 'value_regex';
const VALUE_REGEX_CASEI = 'value_regex_casei';

/*
* DispatchKeyTypes:
* https://github.com/ampproject/amphtml/blob/eda1daa8c40f830207edc8d8088332b32a15c1a4/validator/validator.proto#L111-L120
*/

// Indicates that the attribute does not form a dispatch key.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would be good to add valid PHPDoc with @SInCE tags for these constances.

const NONE_DISPATCH = 0;

// Indicates that the name of the attribute alone forms a dispatch key.
const NAME_DISPATCH = 1;

// Indicates that the name + value of the attribute forms a dispatch key.
const NAME_VALUE_DISPATCH = 2;

// Indicates that the name + value + mandatory parent forms a dispatch key.
const NAME_VALUE_PARENT_DISPATCH = 3;

/**
* If a node type listed here is invalid, it and it's subtree will be
* removed if it is invalid. This is mainly because any children will be
Expand Down
Loading