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

Kotlin and Java generics broken #2684

Closed
JorgeCastilloPrz opened this issue Jun 3, 2020 · 3 comments
Closed

Kotlin and Java generics broken #2684

JorgeCastilloPrz opened this issue Jun 3, 2020 · 3 comments

Comments

@JorgeCastilloPrz
Copy link

JorgeCastilloPrz commented Jun 3, 2020

This might well be related to highlight.js, but in later versions of reveal, adding this Kotlin code snippet to markdown:

sealed class Either<out A, out B> {
  data class Left<out A>(val a: A) : Either<A, Nothing>()
  data class Right<out B>(val b: B) : Either<Nothing, B>()
}

You see how the printed code gets all generic types broken, understanding those as html tags:

Captura de pantalla 2020-06-03 a las 0 11 52

I'd say there's a problem with highlight.js related to this issue on their repo from 2014. The thing is we've been able to write Kotlin and Java code snippets with generic types lately without the need for escaping those characters "manually". It worked for Reveal version 3.9.2.

We can always replace the < / > characters by their html representation &lt; / &gt; to get it fixed, but that is very counterproductive when you need to do it in lots of code snippets across a presentation.

Seems to be related to the RevealHighlight plugin but, maybe I'm missing something here? 🤔 thanks in advance.

Which language seems to have the issue?
kotlin, java

@joshgoebel
Copy link

joshgoebel commented Jun 4, 2020

Speaking for HLJS at least this is the correct behavior. Content inside a <code> block MUST be properly escaped. HTML inside <code> is literally actual HTML. There are potentially VERY serious security consequences to anyone highlighting random code and not doing this escaping properly. HTML injection, JS injection, etc...

In a complex system this is really a step that an intermediate parser should handle... such as if you had a blog where content was Markdown with code snippets... the Markdown parser should proper escape HTML inside code snippets as it converts the Markdown content to HTML, etc.

@hakimel
Copy link
Owner

hakimel commented Jun 5, 2020

Thanks for chiming in and for your work on highlight.js @yyyc514 🙌

I just pushed a small update to the reveal.js highlight plugin which lets you avoid code hitting the HTML parser by wrapping it in <script type="text/template">. If you want to include unescaped HTML entities like < and > you can use it like this:

<pre><code><script type="text/template">
sealed class Either<out A, out B> {
  data class Left<out A>(val a: A) : Either<A, Nothing>()
  data class Right<out B>(val b: B) : Either<Nothing, B>()
}
</script></code></pre>

This is only available in the dev branch so far but you can copy the highlight plugin from there if you want to try it out ahead of the next release.

37d8337

@hakimel hakimel closed this as completed Jun 5, 2020
@joshgoebel
Copy link

joshgoebel commented Jun 5, 2020

The only problem with that is if someone inserts an unescaped closing script tag then they break out of the script element and now you’re opening yourself up to a HTML or JavaScript injection attack all over again.

So if you’re using content provided by the user you need to be very careful which ever way you go here.

<pre><code><script type="text/template">
sealed class Either<out A, out B> {
  data class Left<out A>(val a: A) : Either<A, Nothing>()
  data class Right<out B>(val b: B) : Either<Nothing, B>()
  data class Middle</script>

<!-- do all sorts of EVIL nefarious stuff here -->

<script>
</script></code></pre>

github-actions bot added a commit to vlaci/nix-doom-emacs that referenced this issue Sep 11, 2020
## Changelog for reveal.js:
Commits: [hakimel/reveal.js@15815efe...e09437f4](hakimel/reveal.js@15815ef...e09437f)

* [`942304d8`](hakimel/reveal.js@942304d) add --slide-width/height css variables
* [`cd5c9c5b`](hakimel/reveal.js@cd5c9c5) build js
* [`61624aea`](hakimel/reveal.js@61624ae) 🤦
* [`7ebade72`](hakimel/reveal.js@7ebade7) remove 20px vertical padding on slide sections
* [`c9107476`](hakimel/reveal.js@c910747) don't write '#/' to url on first slide, remove history api feature detection
* [`d272628f`](hakimel/reveal.js@d272628) add support for data-visibility=hidden
* [`ac79c7cd`](hakimel/reveal.js@ac79c7c) leave the progress bar empty if there's < 2 slides
* [`166af893`](hakimel/reveal.js@166af89) all themes now have contrasting text colors based on slide bg
* [`37d83374`](hakimel/reveal.js@37d8337) add support for wrapping code in script tempalte to avoid html parser hakimel/reveal.js#2684
* [`66cbd66f`](hakimel/reveal.js@66cbd66) fix slide numbering issue with uncounted horizontal slides (fixes hakimel/reveal.js#2675)
* [`1b6a3b1e`](hakimel/reveal.js@1b6a3b1) add support for auto-sized big text via r-fit-text
* [`aa667791`](hakimel/reveal.js@aa66779) fit-text helper now triggers lazyily when slide enters view distance
* [`be460814`](hakimel/reveal.js@be46081) correct scope for fit-text selector
* [`cd2a7924`](hakimel/reveal.js@cd2a792) allow images inside of h/vstacks to be proportionally downsized
* [`2fccb774`](hakimel/reveal.js@2fccb77) add 'playsinline' to all inline videos, dont mute background videos on mobile
* [`9ff27cfb`](hakimel/reveal.js@9ff27cf) bg videos remain muted on mobile, otherwise broken when navigating with swipe gestures
* [`2bfe705e`](hakimel/reveal.js@2bfe705) include /css and /js in npm package hakimel/reveal.js#2712
* [`b05e530f`](hakimel/reveal.js@b05e530) Fix URL to pdf-export documentation
* [`3a99a7b7`](hakimel/reveal.js@3a99a7b) shuffle now applies to vertical slides as well
* [`a150d0c5`](hakimel/reveal.js@a150d0c) Start relative paths in CSS with ./
* [`80d96b4f`](hakimel/reveal.js@80d96b4) upgrade rollup-plugin-terser to fix npm warning hakimel/reveal.js#2751
* [`faa8b56e`](hakimel/reveal.js@faa8b56) dependency upgrades
* [`aa62bd42`](hakimel/reveal.js@aa62bd4) Update README.md
* [`2c121d22`](hakimel/reveal.js@2c121d2) docs: Fix simple typo, transiition -> transition
* [`676936e3`](hakimel/reveal.js@676936e) revert debug change to index.html
* [`e09437f4`](hakimel/reveal.js@e09437f) escape HTML entities in code parsed from markdown, fixes hakimel/reveal.js#2744
vlaci pushed a commit to vlaci/nix-doom-emacs that referenced this issue Sep 17, 2020
## Changelog for reveal.js:
Commits: [hakimel/reveal.js@15815efe...e09437f4](hakimel/reveal.js@15815ef...e09437f)

* [`942304d8`](hakimel/reveal.js@942304d) add --slide-width/height css variables
* [`cd5c9c5b`](hakimel/reveal.js@cd5c9c5) build js
* [`61624aea`](hakimel/reveal.js@61624ae) 🤦
* [`7ebade72`](hakimel/reveal.js@7ebade7) remove 20px vertical padding on slide sections
* [`c9107476`](hakimel/reveal.js@c910747) don't write '#/' to url on first slide, remove history api feature detection
* [`d272628f`](hakimel/reveal.js@d272628) add support for data-visibility=hidden
* [`ac79c7cd`](hakimel/reveal.js@ac79c7c) leave the progress bar empty if there's < 2 slides
* [`166af893`](hakimel/reveal.js@166af89) all themes now have contrasting text colors based on slide bg
* [`37d83374`](hakimel/reveal.js@37d8337) add support for wrapping code in script tempalte to avoid html parser hakimel/reveal.js#2684
* [`66cbd66f`](hakimel/reveal.js@66cbd66) fix slide numbering issue with uncounted horizontal slides (fixes hakimel/reveal.js#2675)
* [`1b6a3b1e`](hakimel/reveal.js@1b6a3b1) add support for auto-sized big text via r-fit-text
* [`aa667791`](hakimel/reveal.js@aa66779) fit-text helper now triggers lazyily when slide enters view distance
* [`be460814`](hakimel/reveal.js@be46081) correct scope for fit-text selector
* [`cd2a7924`](hakimel/reveal.js@cd2a792) allow images inside of h/vstacks to be proportionally downsized
* [`2fccb774`](hakimel/reveal.js@2fccb77) add 'playsinline' to all inline videos, dont mute background videos on mobile
* [`9ff27cfb`](hakimel/reveal.js@9ff27cf) bg videos remain muted on mobile, otherwise broken when navigating with swipe gestures
* [`2bfe705e`](hakimel/reveal.js@2bfe705) include /css and /js in npm package hakimel/reveal.js#2712
* [`b05e530f`](hakimel/reveal.js@b05e530) Fix URL to pdf-export documentation
* [`3a99a7b7`](hakimel/reveal.js@3a99a7b) shuffle now applies to vertical slides as well
* [`a150d0c5`](hakimel/reveal.js@a150d0c) Start relative paths in CSS with ./
* [`80d96b4f`](hakimel/reveal.js@80d96b4) upgrade rollup-plugin-terser to fix npm warning hakimel/reveal.js#2751
* [`faa8b56e`](hakimel/reveal.js@faa8b56) dependency upgrades
* [`aa62bd42`](hakimel/reveal.js@aa62bd4) Update README.md
* [`2c121d22`](hakimel/reveal.js@2c121d2) docs: Fix simple typo, transiition -> transition
* [`676936e3`](hakimel/reveal.js@676936e) revert debug change to index.html
* [`e09437f4`](hakimel/reveal.js@e09437f) escape HTML entities in code parsed from markdown, fixes hakimel/reveal.js#2744
vlaci pushed a commit to vlaci/nix-doom-emacs that referenced this issue Sep 17, 2020
## Changelog for reveal.js:
Commits: [hakimel/reveal.js@15815efe...e09437f4](hakimel/reveal.js@15815ef...e09437f)

* [`942304d8`](hakimel/reveal.js@942304d) add --slide-width/height css variables
* [`cd5c9c5b`](hakimel/reveal.js@cd5c9c5) build js
* [`61624aea`](hakimel/reveal.js@61624ae) 🤦
* [`7ebade72`](hakimel/reveal.js@7ebade7) remove 20px vertical padding on slide sections
* [`c9107476`](hakimel/reveal.js@c910747) don't write '#/' to url on first slide, remove history api feature detection
* [`d272628f`](hakimel/reveal.js@d272628) add support for data-visibility=hidden
* [`ac79c7cd`](hakimel/reveal.js@ac79c7c) leave the progress bar empty if there's < 2 slides
* [`166af893`](hakimel/reveal.js@166af89) all themes now have contrasting text colors based on slide bg
* [`37d83374`](hakimel/reveal.js@37d8337) add support for wrapping code in script tempalte to avoid html parser hakimel/reveal.js#2684
* [`66cbd66f`](hakimel/reveal.js@66cbd66) fix slide numbering issue with uncounted horizontal slides (fixes hakimel/reveal.js#2675)
* [`1b6a3b1e`](hakimel/reveal.js@1b6a3b1) add support for auto-sized big text via r-fit-text
* [`aa667791`](hakimel/reveal.js@aa66779) fit-text helper now triggers lazyily when slide enters view distance
* [`be460814`](hakimel/reveal.js@be46081) correct scope for fit-text selector
* [`cd2a7924`](hakimel/reveal.js@cd2a792) allow images inside of h/vstacks to be proportionally downsized
* [`2fccb774`](hakimel/reveal.js@2fccb77) add 'playsinline' to all inline videos, dont mute background videos on mobile
* [`9ff27cfb`](hakimel/reveal.js@9ff27cf) bg videos remain muted on mobile, otherwise broken when navigating with swipe gestures
* [`2bfe705e`](hakimel/reveal.js@2bfe705) include /css and /js in npm package hakimel/reveal.js#2712
* [`b05e530f`](hakimel/reveal.js@b05e530) Fix URL to pdf-export documentation
* [`3a99a7b7`](hakimel/reveal.js@3a99a7b) shuffle now applies to vertical slides as well
* [`a150d0c5`](hakimel/reveal.js@a150d0c) Start relative paths in CSS with ./
* [`80d96b4f`](hakimel/reveal.js@80d96b4) upgrade rollup-plugin-terser to fix npm warning hakimel/reveal.js#2751
* [`faa8b56e`](hakimel/reveal.js@faa8b56) dependency upgrades
* [`aa62bd42`](hakimel/reveal.js@aa62bd4) Update README.md
* [`2c121d22`](hakimel/reveal.js@2c121d2) docs: Fix simple typo, transiition -> transition
* [`676936e3`](hakimel/reveal.js@676936e) revert debug change to index.html
* [`e09437f4`](hakimel/reveal.js@e09437f) escape HTML entities in code parsed from markdown, fixes hakimel/reveal.js#2744
R0bes pushed a commit to R0bes/Terraform-Presentation that referenced this issue Jun 7, 2021
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

No branches or pull requests

3 participants