idawnlight / typecho-theme-material

[LTS] Material Design theme for typecho.
GNU General Public License v3.0
395 stars 52 forks source link

[Fixed] disqusjs: Special characters in siteName #93

Closed FlyGoat closed 4 years ago

FlyGoat commented 4 years ago

There might be some special characters in siteName, such as quotation marks. Without proper escape characters, they might be interpreted as controlling characters.

So we use PHP's json_encode function to handle this string. It will emit escape characters automatically.

Signed-off-by: Jiaxun Yang jiaxun.yang@flygoat.com


In my case, my siteName is Flygoat's Blog. The original code generated js like:

var dsqjs = new DisqusJS({
  shortname: 'flygoats-blog',
  siteName: 'FlyGoat's Blog',
  identifier: '147',
  url: 'https://blog.flygoat.com/archives/147/',
  api: 'https://disqus.skk.moe/disqus/',
  apikey: '##### SECRET #####',
  admin: 'flygoat',
  adminLabel: 'mod'
});

Here 's was interpreted as the end of the string, and discus failed to load.

After this PR:

var dsqjs = new DisqusJS({
  shortname: 'flygoats-blog',
  siteName: 'FlyGoat\'s Blog',
  identifier: '147',
  url: 'https://blog.flygoat.com/archives/147/',
  api: 'https://disqus.skk.moe/disqus/',
  apikey: '##### SECRET #####',
  admin: 'flygoat',
  adminLabel: 'mod'
});

Thanks.

idawnlight commented 4 years ago

However, when the second parameter of getThemeOptions(setting, print) is set to true, the function will ALWAYS print the original string. In addition, json_encode will just return the processed string. Without an echo, the value will NEVER be printed. So, this pull request makes no changes to the output actually.

Through my tests, it should be written like the following.

var dsqjs = new DisqusJS({
    shortname: '<?php getThemeOptions("DisqusShortname", true) ?>',
    siteName: <?php echo json_encode(getThemeOptions("DisqusSiteName")) ?>,
    identifier: '<?php $this->cid() ?>',
    url: '<?php $this->permalink() ?>',
    api: '<?php getThemeOptions("DisqusApi", true) ?>',
    apikey: '<?php getThemeOptions("DisqusApiKey", true) ?>',
    admin: '<?php getThemeOptions("DisqusAdmin", true) ?>',
    adminLabel: '<?php getThemeOptions("DisqusAdminLabel", true) ?>'
});

Then FlyGoat's Blog will be processed to "FlyGoat's Blog" , and everything just works fine.

FlyGoat commented 4 years ago

@idawnlight Sorry for the problem. I had fixed the PR according to your suggestion. Somehow my code works in my local test. But yes, you're right. I'm not even a beginner of PHP.

Thanks.

xtlsoft commented 4 years ago

Actually using json_encode is not the best solution, and may cause more issues when the user input occurs to be invalid (like a fake post request sending an option array and the it can be encoded into a json object and there might be XSS issues etc). Then I suggest just to replace the quotes.

FlyGoat commented 4 years ago

@xtlsoft In this case, siteName is administrator's input. We can assume it's safe.