Make WordPress Core

Opened 16 months ago

Closed 15 months ago

Last modified 15 months ago

#58007 closed defect (bug) (fixed)

HTML API: Support comments created by invalid tag name in tag closers

Reported by: dmsnell's profile dmsnell Owned by: bernhard-reiter's profile Bernhard Reiter
Milestone: 6.2.1 Priority: normal
Severity: normal Version: 6.2
Component: HTML API Keywords: has-patch has-unit-tests
Focuses: Cc:

Description (last modified by sabernhardt)

Adds support to the Tag Processor for a form of HTML comment derived from tag closers with invalid tag names. While it's the case for tag openers that an invalid tag name signals the parser to treat the syntax as normal text, if the token encountered is a tag closer then it starts an HTML comment until the first >

For example, <3 cannot start an HTML element because no tag name can start with a digit. However, </3 abides by a different rule. This is structural content and starts a comment. The comment should span until the first > in the text.

GitHub PR #4256

Side note: can we get a new "Component" in Trac for the "HTML API"?

Change History (21)

This ticket was mentioned in PR #4256 on WordPress/wordpress-develop by @dmsnell.


16 months ago
#1

  • Keywords has-patch has-unit-tests added

## Status

  • [x] Satisfy linter requirements
  • x Create Trac ticket #58007-trac
  • [ ] Expand comments, docblocks, add version and Trac ticket refereces
  • [ ] Create backport in Gutenberg

## What

Adds support for a few invalid HTML comment forms.

## Why

We want to continue to find and handle edge cases in HTML so that the Tag Processor remains reliable in the face of unexpected inputs.

## How

Captures the patterns described in the HTML specification for parse errors relating to invalid comment construction. Since these are comments it's appropriate to proceed consuming input until the end, and there's no need to look for other tags or attributes in the process.

cc: @adamziel @gziolo

This ticket was mentioned in PR #4256 on WordPress/wordpress-develop by @dmsnell.


16 months ago
#2

## Status

  • [x] Satisfy linter requirements
  • x Create Trac ticket #58007-trac
  • [ ] Expand comments, docblocks, add version and Trac ticket refereces
  • [ ] Create backport in Gutenberg

## What

Adds support for a few invalid HTML comment forms.

## Why

We want to continue to find and handle edge cases in HTML so that the Tag Processor remains reliable in the face of unexpected inputs.

## How

Captures the patterns described in the HTML specification for parse errors relating to invalid comment construction. Since these are comments it's appropriate to proceed consuming input until the end, and there's no need to look for other tags or attributes in the process.

cc: @adamziel @gziolo

#3 @sabernhardt
16 months ago

  • Component changed from General to Editor
  • Description modified (diff)

#4 @SergeyBiryukov
16 months ago

  • Component changed from Editor to HTML API

#5 follow-up: @dmsnell
16 months ago

Thank you @SergeyBiryukov for creating that category, if you did that!

#6 in reply to: ↑ 5 @SergeyBiryukov
16 months ago

@dmsnell Thank you for suggesting it :)

#7 @costdev
16 months ago

  • Milestone changed from Awaiting Review to 6.2.1

Moving to 6.2.1 for visibility.

Last edited 16 months ago by costdev (previous) (diff)

#8 @dmsnell
16 months ago

Does anyone have any further thoughts on this? If not, would it be fine to merge in?

#9 @costdev
16 months ago

@dmsnell I just left a few docblock-related suggestions on the PR 🙂

@dmsnell commented on PR #4256:


16 months ago
#10

@costdev I think I've addressed all your comments now, thanks. must say I tried to get the docblock ordering and formatting right, but for some reason that's consistently hard to do

#11 @dmsnell
16 months ago

Thank you @costdev - I believe I have addressed your comments

@dmsnell commented on PR #4256:


16 months ago
#12

So there were @costdev - sorry for missing that. I'll double-check once the tests finish again to make sure no remain.

#13 @dmsnell
15 months ago

Looks like this has been sitting without any further comment or issue for around a week. Is it okay to merge or are there any outstanding concerns?

#14 @dmsnell
15 months ago

@ockham I've updated the description in the GitHub PR, added comments about the extra syntax forms for which this patch adds support. a note here: it's not possible to visit these new forms, because they do not represent tags, but it's important that the processor doesn't get off-track when it encounters them.

#15 @Bernhard Reiter
15 months ago

  • Owner set to Bernhard Reiter
  • Resolution set to fixed
  • Status changed from new to closed

In 55667:

HTML API: Add support for a few invalid HTML comment forms.

  • Comments created by means of a tag closer with an invalid tag name, e.g. </3>.
  • Comments closed with the invalid --!> closer. (Comments should be closed by --> but if the ! appears it will also close it, in error.)
  • Empty tag name elements, which are technically skipped over and aren't comments, e.g. </>.

Props dmsnell, costdev.
Fixes #58007.

#16 @dmsnell
15 months ago

Thanks @bernhard-reiter!

@Bernhard Reiter commented on PR #4256:


15 months ago
#17

Committed to Core trunk in `r55667`.

#18 @Bernhard Reiter
15 months ago

  • Keywords fixed-major added
  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening for 6.2.1 consideration.

#19 @Bernhard Reiter
15 months ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 55668:

HTML API: Add support for a few invalid HTML comment forms.

  • Comments created by means of a tag closer with an invalid tag name, e.g. </3>.
  • Comments closed with the invalid --!> closer. (Comments should be closed by --> but if the ! appears it will also close it, in error.)
  • Empty tag name elements, which are technically skipped over and aren't comments, e.g. </>.

Props dmsnell, costdev.
Merges [55667] to the 6.2 branch.
Fixes #58007.

@Bernhard Reiter commented on PR #4256:


15 months ago
#20

Backported to Core's 6.2 branch in `r55668`.

#21 @Bernhard Reiter
15 months ago

  • Keywords fixed-major removed
Note: See TracTickets for help on using tickets.