Add WP_XML_Tag_Processor and WP_XML_Processor #6713
Conversation
Trac Ticket MissingThis pull request is missing a link to a Trac ticket. For a contribution to be considered, there must be a corresponding ticket in Trac. To attach a pull request to a Trac ticket, please include the ticket's full URL in your pull request description. More information about contributing to WordPress on GitHub can be found in the Core Handbook. |
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
| * | ----------------|---------------------------------------------------------------------| | ||
| * | *Prolog* | The parser is parsing the prolog. | | ||
| * | *Element* | The parser is parsing the root element. | | ||
| * | *Misc* | The parser is parsing miscellaneous content. | |
There was a problem hiding this comment.
It might be helpful to start without an XML Tag Processor and go straight to the XML Processor, following the design of the HTML Processor with its step() and step_in_body() following the different contexts of the HTML API spec.
Prolog, for example, sounds an awful lot like the IN PROLOG context, where the rules can be contained and indicate a state transition rather than embedding the rules for staging in the lower-level tag processing.
There was a problem hiding this comment.
Shipping only an XML Processor was quite tedious
out of curiosity, what was tedious about it? I have a hard time understanding the need for an XML tag processor, since the nesting rules are straightforward when compared with HTML, where the complicated semantic rules forced a split
There was a problem hiding this comment.
I found it difficult to:
- Think about two different levels of parsing (tokenization, context) in a single class
- Avoid intertwining them
- Come up with names for all the methods
That being said, there's nothing inherently wrong with the approach and I can see how it could work.
|
There's only one actual test failure on PHP 7: Edit: That was an invalid test – XML has no attributes in tag closers. The rest of it is the PHPUnit setup complaining about |
I ended up removing $wxr = file_get_contents(__DIR__ . '/test.wxr');
$xml_stream = stream_next_xml_token(chunk_text($wxr));
foreach($xml_stream as $processor) {
if (
// We could also match on #text nodes
$processor->get_token_type() === '#cdata-section' &&
$processor->matches_breadcrumbs(array('content:encoded'))
) {
echo "\n " . dump_token($processor);
}
} |
| * We limit this scan to 30 characters, which allows twenty zeros at the front. | ||
| */ | ||
| 30 | ||
| ); |
There was a problem hiding this comment.
in this case I don't think we want a limit on these. for one, XML requires the trailing semicolon, and for two, we don't want to add rules to the spec and accidentally let something slip in, for example, by prepending a long reference in the front.
I can clean this up here, but I think it's important we not mush together all of the characters. something like this…
if ( '#' === $text[ $next_character_reference_at + 1 ] ) {
$is_hex = 'x' === $text[ $next_character_reference_at + 2 ] || 'X' === $text[ … ];
$zeros_start_at = $next_character_reference_at + 3 + ( $is_hex ? 1 : 0 );
$zeros_length = strspn( $text, '0', $zeros_start_at );
$digits_start_at = $zeros_start_at + $zeros_length;
$digit_chars = $is_hex ? '0123456789abcdefABCDEF' : '0123456789';
$digits_length = strspn( $text, $digit_chars, $digits_start_at );
$semicolon_at = $digits_start_at + $digits_length;
// Must be followed by a semicolon.
if ( ';' !== $text[ $semicolon_at ] ) {
return false;
}
// Null bytes cannot be encoded in XML.
if ( 0 === $digits_length ) {
return false;
}
/*
* Must encode a valid Unicode code point.
* (Avoid parsing more than is necessary).
*/
$max_digits = $is_hex ? 6 : 7;
if ( $digits_length > $max_digits ) {
return false;
}
$base = $is_hex ? 16 : 10;
$code_point = intval( substr( $text, $digits_start_at, $digits_length ), $base );
if ( if_allowable_code_point( $code_point ) ) {
$decoded .= WP_HTML_Decoder::code_point_to_utf8_bytes( $code_point );
continue;
}
return false;
}
// Must be a named character reference.
$name_starts_at = $next_character_reference_at + 1;
$standard_entities = array(
'amp;' => '&',
'apos;' => "'",
'gt;' => '>',
'lt;' => '<',
'quot;' => '"',
);
foreach ( $standard_entities as $name => $replacement ) {
if ( substr_compare( $text, $name, $name_starts_at, strlen( $name ) ) ) {
$decoded .= $replacement;
continue;
}
}granted, we want to perform all length checks to avoid crashing, but I think we have to scan the entire thing to avoid mis-parsing and security issues
There was a problem hiding this comment.
Oh, the decoder won't just skip over the 30 bytes when it stumbles upon ģ. If there's no valid reference within those 30 bytes, that's a parse failure and we halt. Still, I think you're right and it wouldn't be that bad. If someone loaded 1GB of data into memory, they must be expecting the parser will scan through it eventually.
|
Have you seen the Extensible Markup Language (XML) Conformance Test Suites? It may be a helpful resource to find a lot of test cases, although after a quick scan many cases seem to violate "No DTD, DOCTYPE, ATTLIST, ENTITY, or conditional sections" |
… the MISC context where whitespace text nodes are always complete and non-whitespace ones are syntax errors
I haven't seen that one, thank you for sharing! With everything that's going on in Playground I may not be able to iron out all the remaining nuances here – would you like to take over at one point once XML parsing becomes a priority? |
| * | ||
| * @var bool | ||
| */ | ||
| protected $is_incomplete_text_node = false; |
There was a problem hiding this comment.
this name confused me. I think something like $inside_document_trailing_whitespace could be clearer, but I could still be confused
|
Remember too that text appears in XML outside of In effect, Probably need to examine the comment parser because XML cannot contain |
|
@dmsnell Text is supported, or at least should be – is there a specific scenario that this PR doesn't handle today? |
|
Oh that replacement is just overly selective, it can target text nodes, too. By the way, there's no easy way of setting text in a tag without any text nodes. I'm noodling on emitting an empty text node, but haven't yet found a good way of doing it. Another idea would be to couple the tag tag token with the text that follows it, but that sounds unintuitive for the consumer of this API. |
You're way too fast for me to keep up on this, but it shouldn't be that hard, because we will know where the start and end of a tag is, or if it's an empty tag, we know the full token. We'll just need to create a function like |
|
@adamziel I pushed some changes to the decoder to avoid mixing the HTML and XML parsing rules. Sadly, while I thought that PHP's In mucking around I became aware of how much more the role of errors is going to have to play in an XML API. I don't have any idea what's best. Character encoding failures I would assume are going to be fairly benign as long as we treat those failures as plaintext instead of actually decoding them, but that's a point to ponder. This is going to get interesting with documents mixing HTML and XML, such as WXR. We're going to need to ensure that the tag and text parsing rules are properly separated. I'm still not sure what that means for us when we find something like a WXR without proper escaping of the content inside. |
Streaming makes it a bit more difficult, e.g. we may not have the closer yet, or we may not have the opener anymore. Perhaps pausing on incomplete input before yielding the tag opened would be useful here. |
Oh dang it! Too bad.
Yes, that struck me too. At minimum, we'll need to communicate on which byte offset the error has occurred. Ideally, we'd show the context of the document, highlight the relevant part, and give a highly informative error message.
I'm not sure I follow. By escaping of the content do you mean, say, missing |
My plan with the HTML API is to allow a "soft limit" on memory use. If we need to we can add an additional "hard limit" where it will fail. Should content come in and we're still inside a token, we just keep ballooning past the soft limit until we run out of memory, hit the hard limit, or parse the token. So in this way I don't see streaming as a problem. The goal is to enable low-latency and low-overhead processing, but if we have to balloon in order to handle more extreme documents we can break the rules as long as it's not too wild. I prototyped this with the 🔔 Notifications on WordPress.com though in a slightly different way. The Notifications Processor has a runtime duration limit and a length limit, counting code points in the text nodes of the processed document. If it hits the runtime duration limit, it stops processing formatting and instead rapidly joins the remaining text nodes as unformatted plaintext. If it hits the length limit it stops processing altogether. I believe that this Processor framework opens up new avenues for constraints and graceful degradation beyond those limits.
This could be an interesting operating mode: when bailing, produce an Elm/Rust-quality error message. The character reference errors make me think we also have some notion of recoverable and non-recoverable errors. The character reference error does not cause syntactic issues, so we can zoom past them if we want, collecting them along the way into a list of errors. Errors for things like invalid tag names, attribute names, etc… are similar. Missing or unexpected tags though I could see as being more severe since they have ambiguous impact on the rest of the document.
Some WXRs I've seen will have something like I believe that I've seen WXRs that are broken by not encoding the HTML either way, and these are the ones that scare me: Because the embedded document isn't encoded there's no boundary to detect it. I think this implies that for WordPress, our XML parser could have reason to be itself a blend of XML and HTML. For example:
This exploration is quite helpful because I think it's starting to highlight how the shape of WordPress' XML parsing needs differ from those of HTML. |
|
💡This will generally not be in the most critical performance hot path. We can probably relax some of the excessive optimizations, like relying on more functions to separate concepts like Update: You already had this idea 😆 |
Brings together a few explorations to stream-rewrite site URLs in a WXR file coming from a remote server. All of that with no curl, DOMDocument, or other PHP dependencies. It's just a few small libraries built with WordPress core in mind: * [AsyncHttp\Client](WordPress/blueprints#52) * [WP_XML_Processor](WordPress/wordpress-develop#6713) * [WP_Block_Markup_Url_Processor](https://github.com/adamziel/site-transfer-protocol) * [WP_HTML_Tag_Processor](https://developer.wordpress.org/reference/classes/wp_html_tag_processor/) Here's what the rewriter looks like: ```php $wxr_url = "https://github.com/raw/WordPress/blueprints/normalize-wxr-assets/blueprints/stylish-press-clone/woo-products.wxr"; $xml_processor = new WP_XML_Processor('', [], WP_XML_Processor::IN_PROLOG_CONTEXT); foreach( stream_remote_file( $wxr_url ) as $chunk ) { $xml_processor->stream_append_xml($chunk); foreach ( xml_next_content_node_for_rewriting( $xml_processor ) as $text ) { $string_new_site_url = 'https://mynew.site/'; $parsed_new_site_url = WP_URL::parse( $string_new_site_url ); $current_site_url = 'https://github.com/raw/wordpress/blueprints/normalize-wxr-assets/blueprints/stylish-press-clone/wxr-assets/'; $parsed_current_site_url = WP_URL::parse( $current_site_url ); $base_url = 'https://playground.internal'; $url_processor = new WP_Block_Markup_Url_Processor( $text, $base_url ); foreach ( html_next_url( $url_processor, $current_site_url ) as $parsed_matched_url ) { $updated_raw_url = rewrite_url( $url_processor->get_raw_url(), $parsed_matched_url, $parsed_current_site_url, $parsed_new_site_url ); $url_processor->set_raw_url( $updated_raw_url ); } $updated_text = $url_processor->get_updated_html(); if ($updated_text !== $text) { $xml_processor->set_modifiable_text($updated_text); } } echo $xml_processor->get_processed_xml(); } echo $xml_processor->get_unprocessed_xml(); ```
Trac Ticket: Core-61365
What
Proposes an XML Tag Processor and XML Processor to complement the HTML Tag processor and the HTML Processor.
The XML API implements a subset of the XML 1.0 specification and supports documents with the following characteristics:
The API and ideas closely follow the HTML API implementation. The parser is streaming in nature, has a minimal memory footprint, and leaves unmodified markup as it was originally found.
It also supports streaming large XML documents, see adamziel#43 for proof of concept.
Design decisions
Ampersand handling in text and attribute values
XML attributes cannot contain the characters
<or&.Enforcing
<is fast and easy. Enforcing&is slow and complex because ampersands are actually allowed when used as the start. This means we'd have to decode all entities as we scan through the document – it doesn't seem to be worth it.Right now, the WP_XML_Tag_Processor will only bale out when attempting to explicitly access an attribute value or text containing an invalid entity reference.
Accepting all byte sequences up to
<as text dataXML spec defines text data as follows:
Currently WP_XML_Tag_Processor does not attempt to reject bytes outside of these ranges. It will treat everything between valid elements as text data. On the upside, we avoid using an expensive
preg_match()call for processing text. On the downside, we won't bale out early when processing a malformed, truncated, or a non-UTF-8 document.I think it's a good trade-off to take, but I'm also happy to discuss.
Entity decoding
This API only decodes the following entities:
&,<,>,",'{orOther entities trigger a parse error. This API could use the full HTML decoder, but it would be wrong and slow here. If lack of support for other entities ever becomes a problem,
html_entity_decode()would be a good replacement for the custom implementation. It's worth noting that inXML1mode it doesn't decode many HTML entities:Follow-up work
WP_XML_Processorclass as well as theWP_HTML_Processorclass is documented today.Out of scope for the foreseeable future
<!DOCTYPE, see https://www.w3.org/TR/xml/#sec-prolog-dtd<!ATTLIST, see https://www.w3.org/TR/xml/#attdecls<!ENTITY, see https://www.w3.org/TR/xml/#sec-entity-decl<!NOTATION, see https://www.w3.org/TR/xml/#sec-entity-declcc @dmsnell @sirreal