Skip to content
This repository has been archived by the owner on Jun 17, 2022. It is now read-only.

Remove tld checking #100

Closed
wants to merge 3 commits into from
Closed

Conversation

KazuAlex
Copy link

@KazuAlex KazuAlex commented May 3, 2020

Fix bitwarden/clients#2811

Uri doesn't systematically have a tld. Even more from the list defined in the regex "tldEndingRegex" (/.*.(com|net|org|edu|uk|gov|ca|de|jp|fr|au|ru|ch|io|es|us|co|xyz|info|ly|mil)$/) in src/misc/utils.ts

Considere the uri as a "http" resource if the user doesn't provide a scheme.

@CLAassistant
Copy link

CLAassistant commented May 3, 2020

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@@ -95,6 +95,6 @@ export class LoginUriView implements View {
}

get launchUri(): string {
return this.uri.indexOf('://') < 0 && Utils.tldEndingRegex.test(this.uri) ? ('http://' + this.uri) : this.uri;
return this.uri.indexOf('://') < 0 ? ('http://' + this.uri) : this.uri;
Copy link

Choose a reason for hiding this comment

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

This change should also be done to the isWebsite code above.

Copy link
Author

Choose a reason for hiding this comment

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

Absolutely !

@Nokel81
Copy link

Nokel81 commented May 4, 2020

However, I think that it probably would be a good idea to check the hostname if there is one. But I don't know the format of this.uri...

@KazuAlex
Copy link
Author

KazuAlex commented May 4, 2020

However, I think that it probably would be a good idea to check the hostname if there is one. But I don't know the format of this.uri...

There are a RFC about the uri format. But i don't know exactly the acceptations.
But i'm sure about that : checking the TLD is really a bad idea. For example if you use IPs or http://localhost as hostname.

@Nokel81
Copy link

Nokel81 commented May 4, 2020

Fair I guess

@clayadams5226
Copy link

We use GitHub issues as a place to track bugs and other development related issues. The Bitwarden Community Forums has a section for submitting, voting for, and discussing product feature requests like this one.

Please sign up on our forums and search to see if this request already exists. If so, you can vote for it and contribute to any discussions about it. If not, you can re-create the request there so that it can be properly tracked.

This issue will now be closed. Thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Some TLD's not recognized as in URLs
4 participants