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.

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

Fix: respect DisableTags in OutTag func #301

Closed
wants to merge 1 commit into from

Conversation

shwarcu
Copy link

@shwarcu shwarcu commented Jan 25, 2024

Explanation of the issue

I was checking how DisableTags works and I think intention of the autor was to disable HTML tags completely.
However at the moment when this field was set, only closing tags were removed.

Reproduction

I added

renderer.DisableTags = 1

after line https://github.com/gomarkdown/markdown/blob/master/cmd/printast/main.go#L56
and copied content of the README.md into example.md

(these are first few lines of README.md)

 markdown git:(master) ✗ go run ./cmd/printast -to-html example.md
HTML of file 'example.md':
<h1>Markdown Parser and HTML Renderer for Go

<a href="https://pkg.go.dev/github.com/gomarkdown/markdown">pkg.go.dev

Package github.com/gomarkdown/markdown is a Go library for parsing Markdown text and rendering as HTML.

after my changes it seems to work correctly

markdown git:(fix/disable-opening-tags) ✗ go run ./cmd/printast -to-html example.md              
HTML of file 'example.md':
Markdown Parser and HTML Renderer for Go

pkg.go.dev

Package github.com/gomarkdown/markdown is a Go library for parsing Markdown text and rendering as HTML.

(I have tested this on entire README.md content, here for readability I only included partial sample).

Motivations

I'm proposing this fix for 2 reasons:

  • I think this option was supposed to disable all HTML tags
  • this change would allow to effectively strip markdown and have plain text as an output which is what I'm looking for (no need to implement another renderrer). Plus it still gives flexibility to handle some nodes in a custom way - super close to what I need

@shwarcu shwarcu mentioned this pull request Jan 25, 2024
@kjk
Copy link
Contributor

kjk commented Jan 26, 2024

This doesn't seem right.

The intent seems to be to "sanitize" the html output by stripping out the HTML present in markdown (to avoid potentially malicious html propagating from markdown to output HTML).

It was not generate output that is not HTML.

Plus it would be backwards-incompatible change and that alone is reason enough not to change it.

@kjk kjk closed this Jan 26, 2024
@shwarcu
Copy link
Author

shwarcu commented Jan 26, 2024

Ok, I understand.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants