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

Remove restrictive limitation on inline comments #713

Merged
merged 1 commit into from
Sep 8, 2022
Merged

Remove restrictive limitation on inline comments #713

merged 1 commit into from
Sep 8, 2022

Conversation

wooorm
Copy link
Contributor

@wooorm wooorm commented Jun 14, 2022

  • This limitation is not imposed by block comments
  • This limitation is not imposed by HTML
  • This limitation is not expected to be depended on by authors

Closes GH-712.

@wooorm
Remove restrictive limitation on inline comments
434f50a 
*   This limitation is not imposed by block comments
*   This limitation is not imposed by HTML
*   This limitation is not expected to be depended on by authors

Closes commonmarkGH-712.
@wooorm wooorm mentioned this pull request Jun 19, 2022
Closed
@jgm jgm merged commit d5ddfae into commonmark:master Sep 8, 2022
@wooorm wooorm deleted the overly-restrictive-comments branch September 8, 2022 16:45
jgm added a commit to commonmark/cmark that referenced this pull request Sep 8, 2022
@jgm
Update HTML comment scanner...
3dfe48d 
to accord with commonmark/commonmark-spec#713.
jgm added a commit that referenced this pull request Sep 8, 2022
@jgm
Update spec examples for HTML comments...
ea177af 
to conform to the change in #713.
jgm added a commit to commonmark/commonmark.js that referenced this pull request Sep 8, 2022
@jgm
Fix HTML comment regex to conform to commonmark/commonmark-spec#713.
28c82d8
@jgm
Copy link
Member

jgm commented Sep 8, 2022

Question: If this is really what the HTML5 spec says, why do validators reject <!--> and <!---> as comments?
I've tried https://en.rakko.tools/tools/58/ and html-tidy and html5validator and https://validator.w3.org/nu/about.html

@wooorm
Copy link
Contributor Author

wooorm commented Sep 8, 2022

Hmm, if linters see it that way, it seems my statement that this wasn’t a parse error (#712 (comment)) is incorrect.
Still, the HTML spec defines how to handle these cases (as it does any “parse error”), in this case as a comment.

@jgm
Copy link
Member

jgm commented Sep 8, 2022

I see. So, it will be treated as a comment by browsers, even though it isn't a comment.
Question is whether our spec should allow it? Seems better to conform to official spec for comments?
(Also, it certainly makes parsing more complex to allow for <!--> and <!--->, at least in cmark with re2c.)

@wooorm
Copy link
Contributor Author

wooorm commented Sep 8, 2022

even though it isn't a comment.

This part I disagree with. The spec clearly says it is a comment, but that “parse errors” mean that parsers can warn about it.
Same as processing instructions here (<?php?>), they’re comment according to HTML, and a parse error.

In my case, it makes parsing simpler, particularly the -- case.

@wooorm wooorm mentioned this pull request Sep 8, 2022
Closed
@jgm
Copy link
Member

jgm commented Sep 8, 2022

OK. We can stick with this, then.

@wooorm
Copy link
Contributor Author

wooorm commented Sep 8, 2022

I can’t imagine this being a problem for someone, and I really prefer the consistency with both types of inline/block HTML. But I believe we agree that if this is causing problems, it can be reverted!

netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Jan 24, 2023
cmark: updated to 0.30.3
63245d6 
0.30.3

* Fix quadratic complexity bug with repeated `![[]()`.
  Resolves CVE-2023-22486. Add new pathological test.

* Allow declarations with no space, as per spec.

* Set `enumi*` counter correctly in LaTeX output.

* Allow `<!DOCTYPE` to be case-insensitive. (This conforms to the
  existing spec.)

* Fixed HTML comment scanning. Need to handle this case: `<!--> and -->`.
  Since the scanner finds the longest match, we had to
  move some of the logic outside of the scanner.

* Fix quadratic parsing issue with repeated `<!--` (this was not
  introduced by the previous fix, and not in a released version of cmark).
  Resolves CVE-2023-22484. Add new pathological test.

* Update HTML comment scanner to accord with commonmark/commonmark-spec#713.

* Pathological tests: half the number of repetitions, and the timeout.
  This reduces the time needed for the pathological tests.

* Shrink `struct cmark_node`. The `internal_offset` member is
  only used for headings and can be moved to `struct cmark_heading`.
  This reduces the size of `struct cmark_node` from 112 to 104 bytes on
  64-bit systems.

* Add `-Wstrict-prototypes` and fix offending functions.

* Fix quadratic behavior involving `get_containing_block`.
  Instead of searching for the containing block, update the tight list
  status when entering a child of a list item or exiting a list.

* Fix `pathological_tests.py`:
  - Use a multiprocessing.Queue to actually get results from spawned
    tests processes.
  - Fix the `allowed_failures` test.
  - Truncate actual output when printed.
  - Prepare for testing pathological behavior of the Commonmark renderer.

* Fix source position bug with backticks (kyle).
kraj pushed a commit to YoeDistro/meta-openembedded that referenced this pull request Feb 7, 2023
@wangmingyu84 @kraj
cmark: upgrade 0.30.2 -> 0.30.3
69aea3d 
Changelog:
===========
  * Fix quadratic complexity bug with repeated '![[]()'.
    Resolves CVE-2023-22486. Add new pathological test.

  * Allow declarations with no space, as per spec

  * Set 'enumi*' counter correctly in LaTeX output

  * Allow '<!DOCTYPE' to be case-insensitive.
    existing spec.)

  * Fixed HTML comment scanning. Need to handle this case: '<!--> and -->'.
    Since the scanner finds the longest match, we had to
    move some of the logic outside of the scanner.

  * Fix quadratic parsing issue with repeated '<!--' (this was not
    introduced by the previous fix, and not in a released version of cmark).
    Resolves CVE-2023-22484. Add new pathological test.

  * Update HTML comment scanner to accord with commonmark/commonmark-spec#713

  * Pathological tests: half the number of repetitions, and the timeout.
    This reduces the time needed for the pathological tests.

  * Shrink 'struct cmark_node' (openembedded#446). The 'internal_offset' member is
    only used for headings and can be moved to 'struct cmark_heading'.
    This reduces the size of 'struct cmark_node' from 112 to 104 bytes on
    64-bit systems.

  * Add '-Wstrict-prototypes' and fix offending functions.

  * Fix quadratic behavior involving 'get_containing_block' (openembedded#431).
    Instead of searching for the containing block, update the tight list
    status when entering a child of a list item or exiting a list.

  * Fix 'pathological_tests.py'
    - Use a multiprocessing.Queue to actually get results from spawned
      tests processes.
    - Fix the 'allowed_failures' test.
    - Truncate actual output when printed.
    - Prepare for testing pathological behavior of the Commonmark renderer.

  * Fix source position bug with backticks

Signed-off-by: Wang Mingyu <wangmy@fujitsu.com>
Signed-off-by: Khem Raj <raj.khem@gmail.com>
wooorm added a commit to micromark/micromark that referenced this pull request May 29, 2023
@wooorm
Add support for empty comments in HTML (text)
5469c59 
Related-to: commonmark/commonmark-spec#713.
colinodell added a commit to thephpleague/commonmark that referenced this pull request Feb 2, 2024
@colinodell
Remove restrictive limitation on inline comments
42ca918 
See commonmark/commonmark-spec#713
colinodell added a commit to thephpleague/commonmark that referenced this pull request Feb 2, 2024
@colinodell
Remove restrictive limitation on inline comments
cc92f02 
See commonmark/commonmark-spec#713
daregit pushed a commit to daregit/yocto-combined that referenced this pull request May 22, 2024
@wangmingyu84 @kraj
src/meta-openembedded:cmark: upgrade 0.30.2 -> 0.30.3
8afb3cc 
Changelog:
===========
  * Fix quadratic complexity bug with repeated '![[]()'.
    Resolves CVE-2023-22486. Add new pathological test.

  * Allow declarations with no space, as per spec

  * Set 'enumi*' counter correctly in LaTeX output

  * Allow '<!DOCTYPE' to be case-insensitive.
    existing spec.)

  * Fixed HTML comment scanning. Need to handle this case: '<!--> and -->'.
    Since the scanner finds the longest match, we had to
    move some of the logic outside of the scanner.

  * Fix quadratic parsing issue with repeated '<!--' (this was not
    introduced by the previous fix, and not in a released version of cmark).
    Resolves CVE-2023-22484. Add new pathological test.

  * Update HTML comment scanner to accord with commonmark/commonmark-spec#713

  * Pathological tests: half the number of repetitions, and the timeout.
    This reduces the time needed for the pathological tests.

  * Shrink 'struct cmark_node' (#446). The 'internal_offset' member is
    only used for headings and can be moved to 'struct cmark_heading'.
    This reduces the size of 'struct cmark_node' from 112 to 104 bytes on
    64-bit systems.

  * Add '-Wstrict-prototypes' and fix offending functions.

  * Fix quadratic behavior involving 'get_containing_block' (#431).
    Instead of searching for the containing block, update the tight list
    status when entering a child of a list item or exiting a list.

  * Fix 'pathological_tests.py'
    - Use a multiprocessing.Queue to actually get results from spawned
      tests processes.
    - Fix the 'allowed_failures' test.
    - Truncate actual output when printed.
    - Prepare for testing pathological behavior of the Commonmark renderer.

  * Fix source position bug with backticks

Signed-off-by: Wang Mingyu <wangmy@fujitsu.com>
Signed-off-by: Khem Raj <raj.khem@gmail.com>
colinodell added a commit to thephpleague/commonmark that referenced this pull request Jul 22, 2024
@colinodell
Remove restrictive limitation on inline comments
f385597 
See commonmark/commonmark-spec#713
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

(Intentional?) inconsistency between 4.6 block HTML and 6.6 raw HTML comments
2 participants
@wooorm @jgm