From 14d934c7ca8495bad71192e8a8feb9a8e7dd9046 Mon Sep 17 00:00:00 2001 From: "Edward Z. Yang" Date: Mon, 26 May 2008 04:05:48 +0000 Subject: [PATCH] [3.1.1] Land vs's HTMLPurifier_Generator patch, and a number of other bugfixes for that change - Convert a number of calls to use new constructor signature for Generator - Make generator require configuration; this exposes a number of latent bugs - Removed generator hack - Convert Printers to use new optimized ConfigSchema format - Hack with Printer configuration; pass an array(generator config, render config) to distinguish between output and target. - HTML/CSS Printers need to be primed, otherwise fatal errors - Convert a few test-cases to use member properties git-svn-id: http://htmlpurifier.org/svnroot/htmlpurifier/trunk@1770 48356398-32a2-884e-a903-53898d9a118a --- NEWS | 10 +++ configdoc/usage.xml | 16 ++-- library/HTMLPurifier/ChildDef/Required.php | 9 +- library/HTMLPurifier/Generator.php | 3 +- library/HTMLPurifier/Printer.php | 5 +- library/HTMLPurifier/Printer/ConfigForm.php | 89 ++++++++++++++----- .../HTMLPurifier/Strategy/MakeWellFormed.php | 6 +- .../Strategy/RemoveForeignElements.php | 4 +- smoketests/configForm.php | 5 +- smoketests/printDefinition.php | 3 + tests/HTMLPurifier/ErrorCollectorTest.php | 51 +++++------ tests/HTMLPurifier/LanguageTest.php | 2 +- 12 files changed, 124 insertions(+), 79 deletions(-) diff --git a/NEWS b/NEWS index 27d1326d..d0e5b1d7 100644 --- a/NEWS +++ b/NEWS @@ -23,6 +23,8 @@ NEWS ( CHANGELOG and HISTORY ) HTMLPurifier - Account for browser confusion between Yen character and backslash in Shift_JIS encoding. This fix generalizes to any other encoding which is not a strict superset of printable ASCII. +- Fix missing configuration parameter in Generator calls. Thanks vs for the + partial patch. . Added HTMLPurifier_UnitConverter and HTMLPurifier_Length for convenient handling of CSS-style lengths. HTMLPurifier_AttrDef_CSS_Length now uses this class. @@ -42,6 +44,14 @@ NEWS ( CHANGELOG and HISTORY ) HTMLPurifier consult changes to HTMLPurifier_Config for details. . Variable parsing types now are magic integers instead of strings . Added benchmark for ConfigSchema +. HTMLPurifier_Generator requires $config and $context parameters. If you + don't know what they should be, use HTMLPurifier_Config::createDefault() + and new HTMLPurifier_Context(). +. Printers now properly distinguish between output configuration, and + target configuration. This is not applicable to scripts using + the Printers for HTML Purifier related tasks. +. HTML/CSS Printers must be primed with prepareGenerator($gen_config), otherwise + fatal errors will ensue. 3.1.0, released 2008-05-18 # Unnecessary references to objects (vestiges of PHP4) removed from method diff --git a/configdoc/usage.xml b/configdoc/usage.xml index 1383594c..3b68cd7a 100644 --- a/configdoc/usage.xml +++ b/configdoc/usage.xml @@ -68,19 +68,19 @@ - 281 - 305 + 288 + 315 - 283 - 310 + 293 + 323 - 307 + 319 @@ -96,17 +96,17 @@ - 41 + 40 - 70 + 69 - 84 + 83 diff --git a/library/HTMLPurifier/ChildDef/Required.php b/library/HTMLPurifier/ChildDef/Required.php index bd7bdf71..2009fc88 100644 --- a/library/HTMLPurifier/ChildDef/Required.php +++ b/library/HTMLPurifier/ChildDef/Required.php @@ -55,10 +55,7 @@ class HTMLPurifier_ChildDef_Required extends HTMLPurifier_ChildDef $escape_invalid_children = $config->get('Core', 'EscapeInvalidChildren'); // generator - static $gen = null; - if ($gen === null) { - $gen = new HTMLPurifier_Generator(); - } + $gen = new HTMLPurifier_Generator($config, $context); foreach ($tokens_of_children as $token) { if (!empty($token->is_whitespace)) { @@ -83,7 +80,7 @@ class HTMLPurifier_ChildDef_Required extends HTMLPurifier_ChildDef $result[] = $token; } elseif ($pcdata_allowed && $escape_invalid_children) { $result[] = new HTMLPurifier_Token_Text( - $gen->generateFromToken($token, $config) + $gen->generateFromToken($token) ); } continue; @@ -94,7 +91,7 @@ class HTMLPurifier_ChildDef_Required extends HTMLPurifier_ChildDef } elseif ($pcdata_allowed && $escape_invalid_children) { $result[] = new HTMLPurifier_Token_Text( - $gen->generateFromToken( $token, $config ) + $gen->generateFromToken($token) ); } else { // drop silently diff --git a/library/HTMLPurifier/Generator.php b/library/HTMLPurifier/Generator.php index e35ce8cf..42ea17fe 100644 --- a/library/HTMLPurifier/Generator.php +++ b/library/HTMLPurifier/Generator.php @@ -35,8 +35,7 @@ class HTMLPurifier_Generator * @param $config Instance of HTMLPurifier_Config * @param $context Instance of HTMLPurifier_Context */ - public function __construct($config = null, $context = null) { - if (!$config) $config = HTMLPurifier_Config::createDefault(); + public function __construct($config, $context) { $this->config = $config; $this->_scriptFix = $config->get('Output', 'CommentScriptContents'); $this->_def = $config->getHTMLDefinition(); diff --git a/library/HTMLPurifier/Printer.php b/library/HTMLPurifier/Printer.php index 680ffa1d..238cf951 100644 --- a/library/HTMLPurifier/Printer.php +++ b/library/HTMLPurifier/Printer.php @@ -20,18 +20,15 @@ class HTMLPurifier_Printer * Initialize $generator. */ public function __construct() { - $this->generator = new HTMLPurifier_Generator(); } /** * Give generator necessary configuration if possible */ public function prepareGenerator($config) { - // hack for smoketests/configForm.php $all = $config->getAll(); - if (empty($all['HTML'])) return; $context = new HTMLPurifier_Context(); - $this->generator->generateFromTokens(array(), $config, $context); + $this->generator = new HTMLPurifier_Generator($config, $context); } /** diff --git a/library/HTMLPurifier/Printer/ConfigForm.php b/library/HTMLPurifier/Printer/ConfigForm.php index 172d7bb4..9caf7e39 100644 --- a/library/HTMLPurifier/Printer/ConfigForm.php +++ b/library/HTMLPurifier/Printer/ConfigForm.php @@ -1,5 +1,8 @@ name = $name; $this->compress = $compress; // initialize sub-printers - $this->fields['default'] = new HTMLPurifier_Printer_ConfigForm_default(); - $this->fields['bool'] = new HTMLPurifier_Printer_ConfigForm_bool(); + $this->fields[0] = new HTMLPurifier_Printer_ConfigForm_default(); + $this->fields[HTMLPurifier_VarParser::BOOL] = new HTMLPurifier_Printer_ConfigForm_bool(); } /** @@ -68,14 +71,23 @@ class HTMLPurifier_Printer_ConfigForm extends HTMLPurifier_Printer /** * Returns HTML output for a configuration form - * @param $config Configuration object of current form state + * @param $config Configuration object of current form state, or an array + * where [0] has an HTML namespace and [1] is being rendered. * @param $allowed Optional namespace(s) and directives to restrict form to. */ public function render($config, $allowed = true, $render_controls = true) { - $this->config = $config; - $this->prepareGenerator($config); + if (is_array($config) && isset($config[0])) { + $gen_config = $config[0]; + $config = $config[1]; + } else { + $gen_config = $config; + } - $allowed = HTMLPurifier_Config::getAllowedDirectivesForForm($allowed); + $this->config = $config; + $this->genConfig = $gen_config; + $this->prepareGenerator($gen_config); + + $allowed = HTMLPurifier_Config::getAllowedDirectivesForForm($allowed, $config->def); $all = array(); foreach ($allowed as $key) { list($ns, $directive) = $key; @@ -148,13 +160,19 @@ class HTMLPurifier_Printer_ConfigForm extends HTMLPurifier_Printer $ret .= $this->start('td'); $def = $this->config->def->info[$ns][$directive]; - $type = $def->type; - if (!isset($this->fields[$type])) $type = 'default'; + if (is_int($def)) { + $allow_null = $def < 0; + $type = abs($def); + } else { + $type = $def->type; + $allow_null = isset($def->allow_null); + } + if (!isset($this->fields[$type])) $type = 0; // default $type_obj = $this->fields[$type]; - if ($def->allow_null) { + if ($allow_null) { $type_obj = new HTMLPurifier_Printer_ConfigForm_NullDecorator($type_obj); } - $ret .= $type_obj->render($ns, $directive, $value, $this->name, $this->config); + $ret .= $type_obj->render($ns, $directive, $value, $this->name, array($this->genConfig, $this->config)); $ret .= $this->end('td'); $ret .= $this->end('tr'); } @@ -180,7 +198,14 @@ class HTMLPurifier_Printer_ConfigForm_NullDecorator extends HTMLPurifier_Printer $this->obj = $obj; } public function render($ns, $directive, $value, $name, $config) { - $this->prepareGenerator($config); + if (is_array($config) && isset($config[0])) { + $gen_config = $config[0]; + $config = $config[1]; + } else { + $gen_config = $config; + } + $this->prepareGenerator($gen_config); + $ret = ''; $ret .= $this->start('label', array('for' => "$name:Null_$ns.$directive")); $ret .= $this->element('span', "$ns.$directive:", array('class' => 'verbose')); @@ -202,7 +227,7 @@ class HTMLPurifier_Printer_ConfigForm_NullDecorator extends HTMLPurifier_Printer $ret .= $this->elementEmpty('input', $attr); $ret .= $this->text(' or '); $ret .= $this->elementEmpty('br'); - $ret .= $this->obj->render($ns, $directive, $value, $name, $config); + $ret .= $this->obj->render($ns, $directive, $value, $name, array($gen_config, $config)); return $ret; } } @@ -214,22 +239,33 @@ class HTMLPurifier_Printer_ConfigForm_default extends HTMLPurifier_Printer { public $cols = 18; public $rows = 5; public function render($ns, $directive, $value, $name, $config) { - $this->prepareGenerator($config); + if (is_array($config) && isset($config[0])) { + $gen_config = $config[0]; + $config = $config[1]; + } else { + $gen_config = $config; + } + $this->prepareGenerator($gen_config); // this should probably be split up a little $ret = ''; $def = $config->def->info[$ns][$directive]; + if (is_int($def)) { + $type = abs($def); + } else { + $type = $def->type; + } if (is_array($value)) { - switch ($def->type) { - case 'lookup': + switch ($type) { + case HTMLPurifier_VarParser::LOOKUP: $array = $value; $value = array(); foreach ($array as $val => $b) { $value[] = $val; } - case 'list': + case HTMLPurifier_VarParser::ALIST: $value = implode(PHP_EOL, $value); break; - case 'hash': + case HTMLPurifier_VarParser::HASH: $nvalue = ''; foreach ($value as $i => $v) { $nvalue .= "$i:$v" . PHP_EOL; @@ -240,7 +276,7 @@ class HTMLPurifier_Printer_ConfigForm_default extends HTMLPurifier_Printer { $value = ''; } } - if ($def->type === 'mixed') { + if ($type === HTMLPurifier_VarParser::MIXED) { return 'Not supported'; $value = serialize($value); } @@ -249,7 +285,7 @@ class HTMLPurifier_Printer_ConfigForm_default extends HTMLPurifier_Printer { 'id' => "$name:$ns.$directive" ); if ($value === null) $attr['disabled'] = 'disabled'; - if (is_array($def->allowed)) { + if (isset($def->allowed)) { $ret .= $this->start('select', $attr); foreach ($def->allowed as $val => $b) { $attr = array(); @@ -258,8 +294,11 @@ class HTMLPurifier_Printer_ConfigForm_default extends HTMLPurifier_Printer { } $ret .= $this->end('select'); } elseif ( - $def->type == 'text' || $def->type == 'itext' || - $def->type == 'list' || $def->type == 'hash' || $def->type == 'lookup' + $type === HTMLPurifier_VarParser::TEXT || + $type === HTMLPurifier_VarParser::ITEXT || + $type === HTMLPurifier_VarParser::ALIST || + $type === HTMLPurifier_VarParser::HASH || + $type === HTMLPurifier_VarParser::LOOKUP ) { $attr['cols'] = $this->cols; $attr['rows'] = $this->rows; @@ -280,7 +319,13 @@ class HTMLPurifier_Printer_ConfigForm_default extends HTMLPurifier_Printer { */ class HTMLPurifier_Printer_ConfigForm_bool extends HTMLPurifier_Printer { public function render($ns, $directive, $value, $name, $config) { - $this->prepareGenerator($config); + if (is_array($config) && isset($config[0])) { + $gen_config = $config[0]; + $config = $config[1]; + } else { + $gen_config = $config; + } + $this->prepareGenerator($gen_config); $ret = ''; $ret .= $this->start('div', array('id' => "$name:$ns.$directive")); diff --git a/library/HTMLPurifier/Strategy/MakeWellFormed.php b/library/HTMLPurifier/Strategy/MakeWellFormed.php index 2ccc3d4e..fcbb75c0 100644 --- a/library/HTMLPurifier/Strategy/MakeWellFormed.php +++ b/library/HTMLPurifier/Strategy/MakeWellFormed.php @@ -18,7 +18,7 @@ class HTMLPurifier_Strategy_MakeWellFormed extends HTMLPurifier_Strategy // local variables $result = array(); - $generator = new HTMLPurifier_Generator(); + $generator = new HTMLPurifier_Generator($config, $context); $escape_invalid_tags = $config->get('Core', 'EscapeInvalidTags'); $e = $context->get('ErrorCollector', true); @@ -169,7 +169,7 @@ class HTMLPurifier_Strategy_MakeWellFormed extends HTMLPurifier_Strategy if ($escape_invalid_tags) { if ($e) $e->send(E_WARNING, 'Strategy_MakeWellFormed: Unnecessary end tag to text'); $result[] = new HTMLPurifier_Token_Text( - $generator->generateFromToken($token, $config, $context) + $generator->generateFromToken($token) ); } elseif ($e) { $e->send(E_WARNING, 'Strategy_MakeWellFormed: Unnecessary end tag removed'); @@ -209,7 +209,7 @@ class HTMLPurifier_Strategy_MakeWellFormed extends HTMLPurifier_Strategy if ($skipped_tags === false) { if ($escape_invalid_tags) { $result[] = new HTMLPurifier_Token_Text( - $generator->generateFromToken($token, $config, $context) + $generator->generateFromToken($token) ); if ($e) $e->send(E_WARNING, 'Strategy_MakeWellFormed: Stray end tag to text'); } elseif ($e) { diff --git a/library/HTMLPurifier/Strategy/RemoveForeignElements.php b/library/HTMLPurifier/Strategy/RemoveForeignElements.php index 3e9057f1..165308dd 100644 --- a/library/HTMLPurifier/Strategy/RemoveForeignElements.php +++ b/library/HTMLPurifier/Strategy/RemoveForeignElements.php @@ -13,7 +13,7 @@ class HTMLPurifier_Strategy_RemoveForeignElements extends HTMLPurifier_Strategy public function execute($tokens, $config, $context) { $definition = $config->getHTMLDefinition(); - $generator = new HTMLPurifier_Generator(); + $generator = new HTMLPurifier_Generator($config, $context); $result = array(); $escape_invalid_tags = $config->get('Core', 'EscapeInvalidTags'); @@ -101,7 +101,7 @@ class HTMLPurifier_Strategy_RemoveForeignElements extends HTMLPurifier_Strategy // invalid tag, generate HTML representation and insert in if ($e) $e->send(E_WARNING, 'Strategy_RemoveForeignElements: Foreign element to text'); $token = new HTMLPurifier_Token_Text( - $generator->generateFromToken($token, $config, $context) + $generator->generateFromToken($token) ); } else { // check if we need to destroy all of the tag's children diff --git a/smoketests/configForm.php b/smoketests/configForm.php index 051472d9..36c0213d 100644 --- a/smoketests/configForm.php +++ b/smoketests/configForm.php @@ -58,11 +58,10 @@ style="float:right;"> $schema_builder = new HTMLPurifier_ConfigSchema_Builder_ConfigSchema(); $schema = $schema_builder->build($interchange); -HTMLPurifier_ConfigSchema::instance($schema); -$config = HTMLPurifier_Config::loadArrayFromForm($_GET, 'config'); +$config = HTMLPurifier_Config::loadArrayFromForm($_GET, 'config', true, true, $schema); $printer = new HTMLPurifier_Printer_ConfigForm('config', '?doc#%s'); -echo $printer->render($config); +echo $printer->render(array(HTMLPurifier_Config::createDefault(), $config)); ?> diff --git a/smoketests/printDefinition.php b/smoketests/printDefinition.php index 86fd9494..58e281b6 100644 --- a/smoketests/printDefinition.php +++ b/smoketests/printDefinition.php @@ -13,8 +13,11 @@ if (file_exists('printDefinition.settings.php')) { include 'printDefinition.settings.php'; } +$gen_config = HTMLPurifier_Config::createDefault(); $printer_html_definition = new HTMLPurifier_Printer_HTMLDefinition(); +$printer_html_definition->prepareGenerator($gen_config); $printer_css_definition = new HTMLPurifier_Printer_CSSDefinition(); +$printer_css_definition->prepareGenerator($gen_config); $printer_config_form = new HTMLPurifier_Printer_ConfigForm( 'config', diff --git a/tests/HTMLPurifier/ErrorCollectorTest.php b/tests/HTMLPurifier/ErrorCollectorTest.php index d1f2166a..79ab65e2 100644 --- a/tests/HTMLPurifier/ErrorCollectorTest.php +++ b/tests/HTMLPurifier/ErrorCollectorTest.php @@ -6,6 +6,7 @@ class HTMLPurifier_ErrorCollectorTest extends HTMLPurifier_Harness public function setup() { generate_mock_once('HTMLPurifier_Language'); generate_mock_once('HTMLPurifier_Generator'); + parent::setup(); } function test() { @@ -20,14 +21,13 @@ class HTMLPurifier_ErrorCollectorTest extends HTMLPurifier_Harness $line = false; - $context = new HTMLPurifier_Context(); - $context->register('Locale', $language); - $context->register('CurrentLine', $line); + $this->context->register('Locale', $language); + $this->context->register('CurrentLine', $line); - $generator = new HTMLPurifier_Generator(); - $context->register('Generator', $generator); + $generator = new HTMLPurifier_Generator($this->config, $this->context); + $this->context->register('Generator', $generator); - $collector = new HTMLPurifier_ErrorCollector($context); + $collector = new HTMLPurifier_ErrorCollector($this->context); $line = 23; $collector->send(E_ERROR, 'message-1'); @@ -48,23 +48,21 @@ class HTMLPurifier_ErrorCollectorTest extends HTMLPurifier_Harness $config = HTMLPurifier_Config::create(array('Core.MaintainLineNumbers' => true)); - $this->assertIdentical($collector->getHTMLFormatted($config), $formatted_result); + $this->assertIdentical($collector->getHTMLFormatted($this->config), $formatted_result); } function testNoErrors() { $language = new HTMLPurifier_LanguageMock(); $language->setReturnValue('getMessage', 'No errors', array('ErrorCollector: No errors')); - $context = new HTMLPurifier_Context(); - $context->register('Locale', $language); + $this->context->register('Locale', $language); - $generator = new HTMLPurifier_Generator(); - $context->register('Generator', $generator); + $generator = new HTMLPurifier_Generator($this->config, $this->context); + $this->context->register('Generator', $generator); - $collector = new HTMLPurifier_ErrorCollector($context); + $collector = new HTMLPurifier_ErrorCollector($this->context); $formatted_result = '

No errors

'; - $config = HTMLPurifier_Config::createDefault(); - $this->assertIdentical($collector->getHTMLFormatted($config), $formatted_result); + $this->assertIdentical($collector->getHTMLFormatted($this->config), $formatted_result); } function testNoLineNumbers() { @@ -72,13 +70,12 @@ class HTMLPurifier_ErrorCollectorTest extends HTMLPurifier_Harness $language->setReturnValue('getMessage', 'Message 1', array('message-1')); $language->setReturnValue('getMessage', 'Message 2', array('message-2')); $language->setReturnValue('getErrorName', 'Error', array(E_ERROR)); - $context = new HTMLPurifier_Context(); - $context->register('Locale', $language); + $this->context->register('Locale', $language); - $generator = new HTMLPurifier_Generator(); - $context->register('Generator', $generator); + $generator = new HTMLPurifier_Generator($this->config, $this->context); + $this->context->register('Generator', $generator); - $collector = new HTMLPurifier_ErrorCollector($context); + $collector = new HTMLPurifier_ErrorCollector($this->context); $collector->send(E_ERROR, 'message-1'); $collector->send(E_ERROR, 'message-2'); @@ -91,23 +88,21 @@ class HTMLPurifier_ErrorCollectorTest extends HTMLPurifier_Harness $formatted_result = ''; - $config = HTMLPurifier_Config::createDefault(); - $this->assertIdentical($collector->getHTMLFormatted($config), $formatted_result); + $this->assertIdentical($collector->getHTMLFormatted($this->config), $formatted_result); } function testContextSubstitutions() { $language = new HTMLPurifier_LanguageMock(); - $context = new HTMLPurifier_Context(); - $context->register('Locale', $language); + $this->context->register('Locale', $language); - $generator = new HTMLPurifier_Generator(); - $context->register('Generator', $generator); + $generator = new HTMLPurifier_Generator($this->config, $this->context); + $this->context->register('Generator', $generator); $current_token = false; - $context->register('CurrentToken', $current_token); + $this->context->register('CurrentToken', $current_token); - $collector = new HTMLPurifier_ErrorCollector($context); + $collector = new HTMLPurifier_ErrorCollector($this->context); // 0 $current_token = new HTMLPurifier_Token_Start('a', array('href' => 'http://example.com'), 32); @@ -123,7 +118,7 @@ class HTMLPurifier_ErrorCollectorTest extends HTMLPurifier_Harness $collector->send(E_NOTICE, 'message-attr'); // test when context isn't available // 2 - $context->register('CurrentAttr', $current_attr); + $this->context->register('CurrentAttr', $current_attr); $collector->send(E_NOTICE, 'message-attr'); $result = array( diff --git a/tests/HTMLPurifier/LanguageTest.php b/tests/HTMLPurifier/LanguageTest.php index 013c86ed..de0e981a 100644 --- a/tests/HTMLPurifier/LanguageTest.php +++ b/tests/HTMLPurifier/LanguageTest.php @@ -37,7 +37,7 @@ class HTMLPurifier_LanguageTest extends HTMLPurifier_Harness function test_formatMessage_tokenParameter() { $config = HTMLPurifier_Config::createDefault(); $context = new HTMLPurifier_Context(); - $generator = new HTMLPurifier_Generator(); // replace with mock if this gets icky + $generator = new HTMLPurifier_Generator($config, $context); // replace with mock if this gets icky $context->register('Generator', $generator); $lang = new HTMLPurifier_Language($config, $context); $lang->_loaded = true;