Skip to content

Add support for custom version#47

Open
groenroos wants to merge 2 commits into
MindscapeHQ:masterfrom
groenroos:feature/version-var
Open

Add support for custom version#47
groenroos wants to merge 2 commits into
MindscapeHQ:masterfrom
groenroos:feature/version-var

Conversation

@groenroos
Copy link
Copy Markdown

Comment thread main.php
add_action('wp_enqueue_script', 'load_jquery');
add_action('plugins_loaded', 'rg4wp_checkUser');

function rg4wp_version(): string {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we add a defensive (string) cast on the constant value?

define() accepts any scalar, so someone writing define('RAYGUN_VERSION', 1.0) or define('RAYGUN_VERSION', 2) would probably trigger a TypeError here.

Suggested change:

function rg4wp_version(): string {
    static $version = null;
    if ($version === null) {
        $version = defined('RAYGUN_VERSION') ? (string) RAYGUN_VERSION : get_bloginfo('version');
    }
    return $version;
}

Comment thread main.php
</script>
<script type="text/javascript">
rg4js("apiKey", "%s");
rg4js("setVersion", "%s");
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a potential XSS together with its usage further down around lines 80ish?

You could instead do:

$script .= 'rg4js("setVersion", ' . wp_json_encode(rg4wp_version()) . ');' . "\n";

Comment thread main.php

$script .= '</script>';
printf($script, get_option('rg4wp_apikey'), get_bloginfo('version'));
printf($script, get_option('rg4wp_apikey'), rg4wp_version());
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comment above for context

If you append it to the script code after encoding it, then this line would become:

printf($script, get_option('rg4wp_apikey'));

Copy link
Copy Markdown

@TheRealAgentK TheRealAgentK left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thx for the PR @groenroos - added a few suggestions for hardening it a bit.

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

Successfully merging this pull request may close these issues.

Ability to override version

2 participants