Treat different ports similarly to subdomains

[duckbrain]

General information

• Operating system + version: Arch Linux (up to date)
• Browser + version: Google Chrome 76.0.3809.132 (Official Build) (64-bit) stable
• Information about the host app:
  • How did you install it?
    pacman browserpass and browserpass-chrome (aur)
  • Version 3.0.6
• Information about the browser extension:
  • How did you install it? Chrome Extension Store
  • Browserpass extension version as reported by your browser: 3.1.1

---

Currently, it appears that the ports are excluded when comparing domain names to include in the initial drop-down lists.

I have a domain with several ports for various services. My pass tree looks something to the effect of below:

├── example.com:2031
│   └── root
├── example.com:2083
│   └── user

When I navigate to example.com:2031 it lists root and user. I think it would be better to keep the ports separate. Browsers treat different ports as different origins.

Currently subdomains listings include parent domain logins. (Logging in to www.example.com lists the logins for example.com) I think something like that would be appropriate to have a similar behavior for ports, so logging into example.com:2031 would include the logins for example.com, but not visa-versa, and example.com (no explicit port 80 or 443) would not show

[maximbaz]

I looked into it, this is a bit tricky because the library we use is not able to parse port, it just strips it entirely.

While I did create a feature request thom4parisot/tld.js#136, that repo looks abandoned, so I think we will have to implement our in-house solution for this.

PR would be welcome, the relevant place to modify would be pathToDomain function in helper.js

[duckbrain]

Hmm, I'm not sure port makes sense in tld.js. It seems to be concerned strictly with domains as opposed to other parts of the URI. Because the parts are already split on the /, parsing out the port separately should be pretty simple: var portSuffix = parts[key].match(/:[0-9]+$/)[0]; The other issue then become that URI#hostname is used (being the domain or the IP address) as opposed to .host which is the hostname with the port if applicable.

[maximbaz]

Well spotted about hostname, you are right we should be using host everywhere (e.g. when setting settings.host).

Would be nice to have parsing hidden behind a single call to tldjs.parse(), but I dont mind either way.

[remusao]

Hi,

I used to be a contributor to tldjs for a while but had to eventually create and maintain my own fork because of lack of activity on the project. I just wanted to give some insight about parsing the ports (and more parts from URLs) in tldjs/tldts as I put a bit of thought in this in the past. As pointed out, the library is focused on parsing hostnames and domains as accurately and as fast as possible. This means that the code is fine-tuned to look for hostnames then identifying domain, suffix, etc. ignoring everything else. Although doing more than that with a single API like parse(...) would be convenient, I think it belongs to a URL parsing library instead.

If you would like a library which does a bit more, maybe you can have a look at url-parser, which is a pure JavaScript implementation of URL parsing and is using tldts behind the scene to provide information about domains as well.

const { URL } = require('@cliqz/url-parser')

const url = new URL('https://sub.foo.com:42/path');

console.log(url.port); // '42'
console.log(url.host); // 'sub.foo.com:42'
console.log(url.hostname); // 'sub.foo.com'
console.log(url.domainInfo.domain); // 'foo.com'
console.log(url.domainInfo.publicSuffix); // 'com' 

I hope this helps,
Rémi

[maximbaz]

thanks for sharing! yes I'd like us to switch to one of these libraries you mention.

[erayd]

Or in-house it. The primary issue is really having a full list of TLDs. If we have a good source for that, the native API is actually sufficient, and it will remove a dependency.

For reference, https://publicsuffix.org/ seems to host a current list.

[maximbaz]

Implemented in #179, next release is coming out this weekend. Thanks everyone for the help!