Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Fix discrepancy between node url.parse and muon.url.parse #13916

Merged
merged 1 commit into from
May 2, 2018
Merged

Fix discrepancy between node url.parse and muon.url.parse #13916

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion js/lib/urlutil.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -481,11 +481,12 @@ const UrlUtil = {
// parsed.origin is specific to muon.url.parse
if (parsed.origin !== undefined) {
if (parsed.protocol === 'about:') {
return [parsed.protocol, parsed.path].join('')
return [parsed.protocol, parsed.path.replace(/\/.*/, '')].join('')
Copy link
Member

Choose a reason for hiding this comment

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

Is parsed.path always a string, and never null / undefined? I'm guessing so if the test cases without path are working...

Copy link
Member Author

Choose a reason for hiding this comment

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

@petemill this code only runs for muon.url.parse, where parsed.origin is not undefined. in that case, path is always a string.

}
return parsed.origin.replace(/\/+$/, '')
}
if (parsed.host && parsed.protocol) {
// parsed.slashes is specific to node's url.parse
return parsed.slashes ? [parsed.protocol, parsed.host].join('//') : [parsed.protocol, parsed.host].join('')
}
return null
Expand Down
10 changes: 7 additions & 3 deletions test/unit/lib/urlutilTestComponents.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,10 +173,10 @@ module.exports = {

'isFileType': {
'relative file': (test) => {
test.equal(urlUtil().isFileType('/file/abc/test.pdf', 'pdf'), true)
test.equal(urlUtil().isFileType('file:///file/abc/test.pdf', 'pdf'), true)
},
'relative path': (test) => {
test.equal(urlUtil().isFileType('/file/abc/test', 'pdf'), false)
test.equal(urlUtil().isFileType('file:///file/abc/test', 'pdf'), false)
},
'JPG URL': (test) => {
test.equal(urlUtil().isFileType('http://example.com/test/ABC.JPG?a=b#test', 'jpg'), true)
Expand Down Expand Up @@ -365,7 +365,7 @@ module.exports = {
test.strictEqual(urlUtil().getOrigin('https://abc.bing.com'), 'https://abc.bing.com')
},
'gets URL origin for url with port': (test) => {
test.strictEqual(urlUtil().getOrigin('https://bing.com:443/?test=1#abc'), 'https://bing.com:443')
test.strictEqual(urlUtil().getOrigin('https://bing.com:8000/?test=1#abc'), 'https://bing.com:8000')
Copy link
Member Author

@diracdeltas diracdeltas Apr 24, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

this test was failing in muon because muon gets the origin from GURL.GetOrigin (https://cs.chromium.org/chromium/src/url/gurl.h?q=gurl&sq=package:chromium&l=201), which doesn't return the port if it's 443 for HTTPS or 80 for HTTP

},
'gets URL origin for IP host': (test) => {
test.strictEqual(urlUtil().getOrigin('http://127.0.0.1:443/?test=1#abc'), 'http://127.0.0.1:443')
Expand All @@ -374,7 +374,11 @@ module.exports = {
test.strictEqual(urlUtil().getOrigin('about:preferences#abc'), 'about:preferences')
},
'gets URL origin for slashless protocol URL': (test) => {
test.strictEqual(urlUtil().getOrigin('about:test'), 'about:test')
test.strictEqual(urlUtil().getOrigin('about:test/'), 'about:test')
test.strictEqual(urlUtil().getOrigin('about:test/foo'), 'about:test')
test.strictEqual(urlUtil().getOrigin('about:test/foo/bar'), 'about:test')
test.strictEqual(urlUtil().getOrigin('about:test/foo/bar#baz'), 'about:test')
},
'returns null for invalid URL': (test) => {
test.strictEqual(urlUtil().getOrigin('abc'), null)
Expand Down