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

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

Open
xet7 opened this issue Mar 8, 2018 · 15 comments
Assignees
Labels
bug Something isn't working

Comments

@xet7
Copy link
Member

xet7 commented Mar 8, 2018

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
Copy link
Member Author

xet7 commented Mar 8, 2018

From @mrdomino on March 1, 2016 17:36

+1

@xet7
Copy link
Member Author

xet7 commented Mar 8, 2018

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
Copy link
Member Author

xet7 commented Mar 8, 2018

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
Copy link
Member Author

xet7 commented Mar 8, 2018

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
Copy link

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

@xet7
Copy link
Member Author

xet7 commented Jun 24, 2018

@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
Copy link
Member

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
Copy link

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
Copy link

cyberzenk99 commented Jun 24, 2018

@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
Copy link
Member Author

xet7 commented Jun 24, 2018

@cyberzenk99

For Wekan related contributions, you can contact me by email [email protected]

@cyberzenk99
Copy link

@JamborJan should post his email too. :-)

JamborJan added a commit that referenced this issue Jun 25, 2018
@JamborJan
Copy link
Member

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 :-)

@JamborJan JamborJan self-assigned this Jun 25, 2018
@JamborJan JamborJan added the bug Something isn't working label Jun 25, 2018
@ocdtrekkie
Copy link
Member

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
Copy link
Member

JamborJan commented Jun 25, 2018

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
Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants