sandstormports / wordpress-sandstorm

WordPress Sandstorm package
Apache License 2.0
4 stars 3 forks source link

this_will_be_replaced_by_sandstorm needs to be replaced even when escaped in JS strings #11

Open xet7 opened 6 years ago

xet7 commented 6 years ago

From @kentonv on December 2, 2015 2:36

http://ninjility.com/, which appears to be hosted on Sandstorm, has this blob at the top:

    <script type="text/javascript">
        window._wpemojiSettings = {"baseUrl":"http:\/\/s.w.org\/images\/core\/emoji\/72x72\/","ext":".png","source":{"concatemoji":"http:\/\/this_will_be_replaced_by_sandstorm:10000\/wp-includes\/js\/wp-emoji-release.min.js?ver=4.3.2-alpha"}};
        !function(a,b,c){function d(a){var c=b.createElement("canvas"),d=c.getContext&&c.getContext("2d");return d&&d.fillText?(d.textBaseline="top",d.font="600 32px Arial","flag"===a?(d.fillText(String.fromCharCode(55356,56812,55356,56807),0,0),c.toDataURL().length>3e3):(d.fillText(String.fromCharCode(55357,56835),0,0),0!==d.getImageData(16,16,1,1).data[0])):!1}function e(a){var c=b.createElement("script");c.src=a,c.type="text/javascript",b.getElementsByTagName("head")[0].appendChild(c)}var f,g;c.supports={simple:d("simple"),flag:d("flag")},c.DOMReady=!1,c.readyCallback=function(){c.DOMReady=!0},c.supports.simple&&c.supports.flag||(g=function(){c.readyCallback()},b.addEventListener?(b.addEventListener("DOMContentLoaded",g,!1),a.addEventListener("load",g,!1)):(a.attachEvent("onload",g),b.attachEvent("onreadystatechange",function(){"complete"===b.readyState&&c.readyCallback()})),f=c.source||{},f.concatemoji?e(f.concatemoji):f.wpemoji&&f.twemoji&&(e(f.twemoji),e(f.wpemoji)))}(window,document,window._wpemojiSettings);
    </script>

As you can see there is a case of this_will_be_replaced_by_sandstorm that failed to be replaced in there, because the '/'s are for some reason escaped (who knew '\/' was a valid escape sequence in Javascript?).

This actually leads to an error reported on the JS console when loading the site.

Copied from original issue: dwrensha/wordpress-sandstorm#15

xet7 commented 6 years ago

From @mrdomino on March 1, 2016 17:36

+1

xet7 commented 6 years ago

From @dwrensha on November 9, 2016 14:26

Eek, I just noticed that this error shows up in the default configuration. That is, if I create a new WordPress grain, click "Rebuild Public Site", and then visit the site, I see the following error in my browser console:

Blocked loading mixed active content “http://this_will_be_replaced_by_sandstorm:10000/wp-includes/js/wp-emoji-release.min.js?ver=4.4.2”[Learn More]
xet7 commented 6 years ago

From @dwrensha on November 9, 2016 14:48

The reason we see the strange-looking host this_will_be_replaced_by_sandstorm:10000 has do with our integration with Sandstorm's web publishing feature. When you click the "Rebuild Public Site" button, we run a recursive wget to locally grab the contents of your site and copy them into /var/www/. This relies on some configuration in /etc/hosts. The idea is that after grabbing the content, we can do a find-and-replace to convert these URLs to an appropriate form for external consumption. Unfortunately, the find-and-replace does not catch all cases.

xet7 commented 6 years ago

From @JamborJan on January 16, 2017 6:59

Is there any chance that we get this bug solved? As far as I can see it has impact on other issues and possible solutions there, see issue #22. Thanks a lot!

Update: as far as I can see, my point is caused by this line in the mentioned recursive wget script. I guess the script should replace the url in any case no matter if it is a https or http link.

cyberzenk99 commented 6 years ago

Did you find a solution for this? I have the same problem.

xet7 commented 6 years ago

@cyberzenk99

I have not looked at this yet, because I'm mostly concentrating to maintaining Wekan https://wekan.github.io .

Currently @JamborJan maintains this WordPress for Sandstorm.

I presume fixing this would require somebody looking at this or related repos source code, and find where to remove that text etc. Contributions welcome :)

JamborJan commented 6 years ago

I have some experimental stuff but nothing 100% works yet. I’ll create a branche for this bug to share my current status of the bug fix.

cyberzenk99 commented 6 years ago

Thank you for the quick reply Lauri (@xet7 ). I understand there's a lot to maintain. With just a few bugs fixed, Sandstorm would be an amazing platform. By the way, you're doing a great job with Wekan too.

And thank you @JamborJan . If you could give me any kind of fix, whether it's editing some code manually, or any other ideas, I would be so thankful. Even if it's experimental, I can do it the hard way until the final fix is in place. Any suggestions, I would be appreciative.

cyberzenk99 commented 6 years ago

@xet7 @JamborJan As an added note, I would love to give a contribution, just let me know where to send it. :-) As for a programming contribution, I lack the necessary programing skills, otherwise, I would definitely contribute that way too.

xet7 commented 6 years ago

@cyberzenk99

For Wekan related contributions, you can contact me by email x@xet7.org

cyberzenk99 commented 6 years ago

@JamborJan should post his email too. :-)

JamborJan commented 6 years ago

Hey @cyberzenk99,

I was able to implement a quick and dirty fix for the java script issue (see this for details). It's not nice but it solves the issue with the replacement of this_will_be_replaced_by_sandstorm.

But in my test cases I now get another error message in the browsers console:

wp-emoji-release.min.js:1 Failed to load resource: the server responded with a status of 404 (Not Found)

Not sure if this is a Sandstorm related issue or not. Can anyone please test with the beta release and let me know the result? Please be aware that this is not much tested. So please don't upgrade any production critical stuff.

For any contributions, questions or whatever: feel free to contact me :-)

ocdtrekkie commented 6 years ago

Are you sure wp-emoji-release.min.js didn't somehow get dropped from your beta package? It doesn't look like you submitted any chances to the sandstorm-files.list file.

JamborJan commented 6 years ago

You are right, it seems so. But as I did test with vagrant-spk dev including building pages any missing files should have been added to the file list. Plus the file is in the list and has been there before:

wordpress-read-only/wp-includes/js/wp-emoji-release.min.js

Edit: when I vagrant-spk ssh into the package I see the js file too.

Thanks for your thoughts and input anyway. It always helps to see issues from another perspective.

JamborJan commented 6 years ago

This still seems to be an issue. Will investigate further.