Skip to content
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

Use cliqz-url-parser for URL processing. #410

Merged
merged 4 commits into from Jul 17, 2019
Merged
Changes from 1 commit
Commits
File filter
Filter file types
Jump to
Jump to file
Failed to load files.

Always

Just for now

Next

Use cliqz-url-parser for URL processing.

  • Loading branch information
sammacbeth committed Jul 5, 2019
commit 60441172775f9f04f99e52265dfde9cb71b30b0e
@@ -42,6 +42,7 @@
},
"homepage": "https://github.com/ghostery/ghostery-extension#readme",
"dependencies": {
"@cliqz/url-parser": "^1.0.2",
"base64-js": "^1.2.1",
"browser-core": "https://github.com/cliqz-oss/browser-core/releases/download/v7.37.4/browser-core-7.37.4.tgz",
"classnames": "^2.2.5",
@@ -19,7 +19,7 @@
* @namespace BackgroundUtils
*/
import { debounce } from 'underscore';
import url from 'url';
import { URL } from '@cliqz/url-parser';
import tabInfo from '../classes/TabInfo';
import globals from '../classes/Globals';
import { log, objectEntries } from './common';
@@ -168,7 +168,16 @@ export function processFpeUrl(src) {
* @return {Object} contains url parts as properties
*/
export function processUrl(src) {
if (!src) {
try {
const res = new URL(src);
return {
protocol: res.protocol ? res.protocol.substr(0, res.protocol.length - 1) : '',
host: res.hostname || '',
path: res.pathname ? res.pathname.substr(1) : '',
host_with_path: (res.host || '') + (res.pathname || ''),

This comment has been minimized.

@sammacbeth

sammacbeth Jul 5, 2019
Author Contributor

This was carried over as-is from the previous version, but I wonder if it intentional that you use host here instead of hostname. The former includes the port number if it is non-standard. Are there patterns that use this information? If not, we could use hostname here to be more consistent. @christophertino

This comment has been minimized.

@jsignanini

jsignanini Jul 11, 2019
Member

I think that since we were creating a somewhat custom object being returned from processUrl(), and not aiming to exactly replicate the URL() function, we just named it host for convenience but I agree that it could lead to confusion.

anchor: res.hash ? res.hash.substr(1) : '',
};
} catch (e) {
return {
protocol: '',
host: '',
@@ -177,15 +186,6 @@ export function processUrl(src) {
anchor: '',
};
}
const res = url.parse(src);

return {
protocol: res.protocol ? res.protocol.substr(0, res.protocol.length - 1) : '',
host: res.hostname || '',
path: res.pathname ? res.pathname.substr(1) : '',
host_with_path: (res.host || '') + (res.pathname || ''),
anchor: res.hash ? res.hash.substr(1) : '',
};
}

/**
@@ -199,7 +199,15 @@ export function processUrlQuery(src) {
return {};
}

return url.parse(src, true).query;
try {
const res = {};
for (const [key, value] of new URL(src).searchParams.entries()) {
res[key] = value;
}
return res;
} catch (e) {
return {};
}
}

/**
@@ -311,6 +311,13 @@
tslib "^1.9.3"
tsmaz "^1.2.1"

"@cliqz/url-parser@^1.0.2":
version "1.0.2"
resolved "https://registry.yarnpkg.com/@cliqz/url-parser/-/url-parser-1.0.2.tgz#0c42d73dbe354efad572d9ef39c0aa5d7f6151ea"
integrity sha512-4Y5DQqUv41SWoP7nDRO9PBMH0sSor6aiBT/t1wvjCrUDpG4yhjvAvwRaBN2GoGuHPkWUxT7mZN/FKWtSRZ/FIQ==
dependencies:
tldts "^5.0.3"

"@cnakazawa/watch@^1.0.3":
version "1.0.3"
resolved "https://registry.yarnpkg.com/@cnakazawa/watch/-/watch-1.0.3.tgz#099139eaec7ebf07a27c1786a3ff64f39464d2ef"
ProTip! Use n and p to navigate between commits in a pull request.