-
Notifications
You must be signed in to change notification settings - Fork 23
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
catch rookie mistakes with usage of website #172
Conversation
Template.php
Outdated
if ($this->blank('url') && !$this->blank('website')) { // No URL, but a website | ||
$url = trim($this->get('website')); | ||
if (strtolower(substr( $url, 0, 4 )) !== "http" ) { | ||
$url = "https://" . $url; // Try it with http |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we go a step further with a regexp to catch typos such as website = ttp:/web.com
? (Might not be necessary in enough cases to justify)
Also is it worth testing http
as well as https
to catch sites that lack SSL?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Improving
Template.php
Outdated
} | ||
if (filter_var($url, FILTER_VALIDATE_URL,FILTER_FLAG_HOST_REQUIRED | FILTER_FLAG_PATH_REQUIRED ) ) { // is a valid http(s) URL with a full hostname and path of some type | ||
$this->set('url',$url); | ||
$this->forget('website'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rather than using "set" and "forget", it would be better to rename the existing parameter; this will preserve its position in the citation and its formatting/spacing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will do
also, there are bots that will convert http to https for some websites
I think this now much better. That regex is insane, but necessary if we are going to claim that a website is actually a URL. |
No description provided.