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

[subscriptions] Email content corrupted after a [code] block #920

Closed
Averell7 opened this issue Apr 7, 2016 · 20 comments
Closed

[subscriptions] Email content corrupted after a [code] block #920

Averell7 opened this issue Apr 7, 2016 · 20 comments
Labels
Milestone

Comments

@Averell7
Copy link

Averell7 commented Apr 7, 2016

This bug was tested locally and also on the sandbox of discussion.flarum.org.

Simple to reproduce :

  • Follow a discussion
  • ask someone to post a message with :
    [code]OK[/code]

or anything which contains a code block.

See example here : https://discuss.flarum.org/d/2451-lol/9

In the email received to inform that there was a new post, the text is correct until the end of the code block. Immediately after, there is a sequence of a js file (always the same sequence exactly, even after recompiling). The text following the code block is not present in the email.

In the example on the sandbox, here is the email I received :


Hey Averell!

datitisev made a post in a discussion you're following: Lol

To view the new activity, check out the following link:
https://discuss.flarum.org/d/2451/9

---

Okif("undefined"!==typeof hljs)hljs._ha();else if("undefined"===typeof hljsLoading){hljsLoading=1;var a=document.getElementsByTagName("head")[0],e=document.createElement("link");e.type="text/css";e.rel="stylesheet";e.href="//cdnjs.cloudflare.com/ajax/libs/highlight.js/9.0.0/styles/default.min.css";a.appendChild(e);e=document.createElement("script");e.type="text/javascript";e.onload=function(){var d={},f=0;hljs._hb=function(b){b.removeAttribute("data-hljs");var c=b.innerHTML;c in d?b.innerHTML=d[c]:(7

---

You won't receive any more notifications about this discussion until you're up-to-date.


The same thing happens if you create a code block by an indent (four spaces).

@dsevillamartin
Copy link
Member

The actual JavaScript isn't like that, because it is plain text, so the HTML hasn't been parsed.
This is the actual script, all I did was parse it:

if("undefined"!==typeof hljs)hljs._ha();else if("undefined"===typeof hljsLoading){hljsLoading=1;var a=document.getElementsByTagName("head")[0],e=document.createElement("link");e.type="text/css";e.rel="stylesheet";e.href="//cdnjs.cloudflare.com/ajax/libs/highlight.js/9.0.0/styles/default.min.css";a.appendChild(e);e=document.createElement("script");e.type="text/javascript";e.onload=function(){var d={},f=0;hljs._hb=function(b){b.removeAttribute("data-hljs");var c=b.innerHTML;c in d?b.innerHTML=d[c]:(7

If I use a beautify-er, the code now looks like this:

if ("undefined" !== typeof hljs) hljs._ha();
else if ("undefined" === typeof hljsLoading) {
    hljsLoading = 1;
    var a = document.getElementsByTagName("head")[0],
        e = document.createElement("link");
    e.type = "text/css";
    e.rel = "stylesheet";
    e.href = "//cdnjs.cloudflare.com/ajax/libs/highlight.js/9.0.0/styles/default.min.css";
    a.appendChild(e);
    e = document.createElement("script");
    e.type = "text/javascript";
    e.onload = function () {
        var d = {}, f = 0;
        hljs._hb = function (b) {
            b.removeAttribute("data-hljs");
            var c = b.innerHTML;
            c in d ? b.innerHTML = d[c] : (7

And we can see, that it added the code to highlight the code block 😃
You are welcome, guys! Saved a lot of time 😉

@Averell7
Copy link
Author

Averell7 commented Apr 8, 2016

afaik, the source of the code is : vendor\s9e\text-formatter\src\Plugins\BBCodes\Configurator\repository.xml (line 53)
Then it is "compressed" in vendor\s9e\text-formatter\src\Bundles\Forum\Renderer.php (line 46)
and then in storage\formatter\Renderer_1bb0f654d83b8c65f588e83ef8e10662ef06d0a9.php

I didn't know exactly what was this code, but you are right, it is in the block <bbcode name="CODE">.

@dsevillamartin
Copy link
Member

Why does it stop suddenly after the 7 ? Because of the < which follows ?

I wouldn't think so...
As you know, the emails are plain text...
I also wondered about it.
Hopefully @JoshyPHP can tell us more of s9e/TextFormatter about that script :|

@JoshyPHP
Copy link
Contributor

JoshyPHP commented Apr 8, 2016

@Averell7 Correct on both points. The view uses strip_tags() on the post's HTML and strip_tags() removes any text that follows the <.

@Averell7
Copy link
Author

Averell7 commented Apr 8, 2016

An interesting news ! I changed the code which extracts this data from : strip_tags($blueprint->post->contentHtml)
to
$blueprint->post->contentHtml
The result makes sense :

The text of the message was : toto3

The message now contains :

<pre data-hljs="" data-s9e-livepreview-postprocess="if('undefined'!==typeof hljs)hljs._hb(this)"><code class="">toto3</code></pre><script>if("undefined"!==typeof hljs)hljs._ha();else if("undefined"===typeof hljsLoading){hljsLoading=1;var a=document.getElementsByTagName("head")[0],e=document.createElement("link");e.type="text/css";e.rel="stylesheet";e.href="//cdnjs.cloudflare.com/ajax/libs/highlight.js/9.0.0/styles/default.min.css";a.appendChild(e);e=document.createElement("script");e.type="text/javascript";e.onload=function(){var d={},f=0;hljs._hb=function(b){b.removeAttribute("data-hljs");var c=b.innerHTML;c in d?b.innerHTML=d[c]:(7<++f&&(d={},f=0),hljs.highlightBlock(b.firstChild),d[c]=b.innerHTML)};hljs._ha=function(){for(var b=document.querySelectorAll("pre[data-hljs]"),c=b.length;0<c;)hljs._hb(b.item(--c))};hljs._ha()};e.async=!0;e.src="//cdnjs.cloudflare.com/ajax/libs/highlight.js/9.0.0/highlight.min.js";a.appendChild(e)}</script>

So it is the full CDATA block.

And if now I copy below this block, without the back sticks, you will just see the text (toto3) highlighted :

toto3
<script>if("undefined"!==typeof hljs)hljs._ha();else if("undefined"===typeof hljsLoading){hljsLoading=1;var a=document.getElementsByTagName("head")[0],e=document.createElement("link");e.type="text/css";e.rel="stylesheet";e.href="//cdnjs.cloudflare.com/ajax/libs/highlight.js/9.0.0/styles/default.min.css";a.appendChild(e);e=document.createElement("script");e.type="text/javascript";e.onload=function(){var d={},f=0;hljs._hb=function(b){b.removeAttribute("data-hljs");var c=b.innerHTML;c in d?b.innerHTML=d[c]:(7<++f&&(d={},f=0),hljs.highlightBlock(b.firstChild),d[c]=b.innerHTML)};hljs._ha=function(){for(var b=document.querySelectorAll("pre[data-hljs]"),c=b.length;0

@dsevillamartin
Copy link
Member

Well, this probably references #537, about the HTML emails an all that 😉

@Averell7
Copy link
Author

Averell7 commented Apr 8, 2016

UPDATE : Yes, you are perfectly right. I understood suddenly what happens, see next post.

@Averell7
Copy link
Author

Averell7 commented Apr 8, 2016

Now what happens is clear. The cause is the following code in newPost.blade.php (see issue #537 for details on this file) :

The content of the message is displayed by the following line :
{{ strip_tags($blueprint->post->contentHtml) }}
which php equivalent is :
htmlentities(strip_tags($blueprint->post->contentHtml))

This means :

  • message is extracted from the database
  • converted to html, using the normal process of Flarum, when it displays a message, and the result is stored in $blueprint->post->contentHtml
  • then striptags removes the html specific codes
  • and htmlentities converts all special characters so that they are no longer html code

It is just nonsense. Building html code to have a beautiful display, then removing all html codes for security reasons.

I think there is no security issue, because all html tags typed by the author of the post are already converted in html entities in the process used by Flarum. If you type <b> in a message, it will appear as is, it is no longer an html code. So, it is possible to use html directly in the email, provided the email is in html format ( #537 discusses this matter).

conclusion : To fix the present bug :

  1. In html format, use : {!! $blueprint->post->contentHtml) !!}
  2. in text format, use : {!! $blueprint->post->content !!}

This will be implemented when #537 will be fixed, and the present bug is only a consequence of #537

Note that in html format, formating (bold, italics, titles, lists, background for quotes, colors...) will be preserved and the text in the email will look veray similar to the text on the forum (provided html formatting is supported by the email program.

@Averell7 Averell7 closed this as completed Apr 9, 2016
@Averell7 Averell7 reopened this Apr 12, 2016
@Averell7
Copy link
Author

since #537 is still pending, this bug is still pending also.

@franzliedke
Copy link
Contributor

@tobscure Shouldn't we use contentPlain instead of contentHtml for the plain text emails? After all, Markdown works nice as plain-text formatting... (And once we add HTML emails, most people will see the pretty emails anyway.)

@franzliedke
Copy link
Contributor

And if we simply dump the HTML in those HTML emails, will the script tags cause any problems with email clients? Does anybody know that? I could imagine that could make the messages look suspicious to anti-spam tools.

Or is there an easy way to render the HTML without any JS, @JoshyPHP ?

@tobyzerner tobyzerner removed this from the 0.1.0 milestone Jul 22, 2017
@PeopleInside
Copy link

PeopleInside commented Dec 11, 2018

This issue seems to be present also if a message is quoted in the forum and there is a code

@franzliedke
Copy link
Contributor

I had a fix for this lying around for a long time now... 🙈

@PeopleInside Please do me a favor and tell me whether this commit fixes the issue for you.

@matteocontrini
Copy link
Contributor

matteocontrini commented Dec 11, 2018

I think the same issue might be there in the mentions extension as well?

postMentioned.blade.php
userMentioned.blade.php

@franzliedke
Copy link
Contributor

@matteocontrini Yes, I am just waiting for confirmation (and also testing myself right now) before pushing the fixes there. Likely fixing #537 in the process. :)

@PeopleInside
Copy link

For me seems not work but not sure.
I done the edit on my server than cleared cache and refreshed the page.

but quoted test still have issue on the forum and on the email if code is present.

00
01

I changed only the file fixed by @franzliedke

@franzliedke
Copy link
Contributor

Hmm... it works locally. Please clear your cache.

@PeopleInside
Copy link

PeopleInside commented Dec 11, 2018

Work if i do not quote the text so email code arrive correct.
Issue still be present if i quote some text. Screenshot above was a quote of this:

02

I made this below test and work also by email notification:

03

mmmm seems if i have two translation in email issue still persist..

04

@franzliedke
Copy link
Contributor

Must be a cache or op-cache issue on your server. For me, the issue is fixed on dev-master.

franzliedke added a commit to flarum/mentions that referenced this issue Dec 11, 2018
@PeopleInside
Copy link

PeopleInside commented Dec 11, 2018

Issue resolved in email but still be present on forum post if text with code is quoted. Visual issue on the forum.

You can see the issue here: https://discuss.flarum.org/d/17993-broke-quote-test @franzliedke
https://discuss.flarum.org/d/17993-broke-quote-test/2

askvortsov1 pushed a commit to flarum/mentions that referenced this issue Mar 11, 2022
askvortsov1 pushed a commit to flarum/subscriptions that referenced this issue Mar 11, 2022
askvortsov1 pushed a commit to flarum/mentions that referenced this issue May 10, 2022
askvortsov1 pushed a commit to flarum/subscriptions that referenced this issue May 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants