[New Sniff] Check for hardcoded URLs

[khacoder]

Rule:

WARNING Verify that no hard-coded URLs are found. Manual verification necessary. An exception is made for the Author URI and (Theme) URI as set in the style.css header as well as links to wordpress.org. Links in the text of PHP/JS comments should be excluded from this check.

Ref: https://make.wordpress.org/themes/handbook/review/required/theme-check-plugin/#info

Theme check file covering this rule:

https://github.com/Otto42/theme-check/blob/master/checks/links.php

To Do:

• [ ] Create unit tests
• [ ] Create new sniff

[fklein-lu]

The sniff in the PR corresponds to the rule, but I consider that the rule is not precise enough. The reason is that the automation should alleviate the work of the reviewer. But the current rule and implementation only assists the reviewer, and does not reduce the workload.

Hardcoded URLs are related to several issues, these come to mind:

1. Scripts and styles that are hardcoded into the templates, instead of being enqueued.
2. JavaScript files that are being loaded from CDNs instead of local.
3. CSS files that are being loaded from CDNs instead of from local.
4. Links that are placed in the templates.
5. URLs that are included in JavaScript files.

All of these need a different sniff:

• For the first issue, I think we would have to resolved to parsing the templates with regular expressions to look for <link> and <script> tags that link to these types of assets.
• For the second issue, we would need to look for all occurrences of wp_register_script(), and wp_enqueue_script(), and ensure that no URL of any kind is passed to those functions.
• For stylesheets, one would have to do something similar, but here one must consider whitelisted URLs, such as those pointing to the Google Fonts CDN.
• Links that might be spam, here we can use regex again, and warn reviewers about these links.
• For JavaScript, I think we would just flag all URLs in scripts. It is perfectly possible to inject malicious code this way, and most people skip JavaScript files during review, so having this is very important.

I'd recommend that we look at the validity of the cases outlined above, and that @grappler or someone more familiar with the rules can give an opinion on whether these are valid according to the review rules.

If so, we would need a dedicated issue for each of the cases, with a dedicated sniff.

[grappler]

Totally agree with the additional sniffs. We want to check all of the URLs added in theme to check that there are no spam links. Maybe we need to rename the sniff to make it clearer.

[khacoder]

The NoHardCosedUrls sniff currently identifies urls in php, css and js files, and contains some white listed strings to exclude some urls, such as Google Fonts. So I believe your bottom three points are looked after in this sniff.

Checking for specific urls contained in links in templates (comment 4 above) is more problematic because quite often urls are contained in variables or constants. The NoHardCodedUrls sniff only looks for urls and identifies many more urls then are found in the current Themecheck sniff, which focuses on urls in links.

Issue #20 covers discouraging creating CDN's. It could be switched from WARNING to ERROR and re titled to say NoBlacklistedCDN or something like that.

A separate sniff should be developed to detect urls in links and script tags and not allow them.

[fklein-lu]

I consider that it is important to keep in mind what the sniffs should do, and that is reduce the amount of human reviewer time needed to review a theme. A sniff that outputs all hardcoded URLs found in a theme has no advantage over using grep.

I think that having a sniff for detecting spam links is very useful, and we should focus on that for this ticket, and implement more precise and automated checks for the issues I mentioned above.

Issue 1 is now tracked in #39.

[justintadlock]

Just some quick ideas. We could whitelist some URLs:

• wordpress.org(.*?)
• fonts.googleapis
• Theme URI
• Author URI

We don't want to block something like a link to theme docs, for example. It seems like we could get the Theme URI and Author URI domains and check other links against those. If the Theme URI is example.com, we'd allow for other URIs like example.com/docs (this should still be manually checked).

Other URIs that don't match probably shouldn't exist. But, there's probably some edge cases.

[joyously]

Would that preclude links in the admin area to a support forum or where to go for premium versions? Or comments in code or CSS giving credit to original authors? An example is the Meyer's reset CSS or the GPL licensing.

[justintadlock]

Support and premium versions links should be fine under the above.

I don't really think a whitelist is realistic, I was just giving a quick thought about a foundation to build from if we were to go such a route. Hardcoded links are tricky, so I want us to exhaust all of our options.

[jrfnl]

Just my two pennies on a few points mentioned above:

A sniff that outputs all hardcoded URLs found in a theme has no advantage over using grep.

It does up to a point - in that the theme reviewer does not have to use a number of different tools, but gets all (most) the issues which can be checked in a semi-automated way in one place using just one tool.

I think that having a sniff for detecting spam links is very useful

We could whitelist some URLs

Both of these would change the current rule. Similarly, a number of other points brought up are related, but not the same issue. So maybe the whole spectrum of rules which relate to urls should be clarified first ?

[dingo-d]

Is this something we want to work on? Any info is appreciated 🙂