Skip to content

Commit

Permalink
Fix URL parsing consistency issue
Browse files Browse the repository at this point in the history
Fix brave#10270 by preferring muon.url.parse over Node's legacy (and non-standards compliant) URL parser. This is subobtimal because unit tests are running a different URL parser from the actual browser, but seems like the best trade off for now.

Test Plan:
1. go to brave.com and disable shields
2. go to http://brave.com%60x.code-fu.org/. shields should not be disabled.
  • Loading branch information
diracdeltas authored and dfperry5 committed Aug 18, 2017
1 parent 43d9fe5 commit d36a1c4
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 6 deletions.
2 changes: 1 addition & 1 deletion app/browser/webtorrent.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ const appUrlUtil = require('../../js/lib/appUrlUtil')
const appActions = require('../../js/actions/appActions')
const messages = require('../../js/constants/messages')
const Filtering = require('../filtering')
const urlParse = require('url').parse
const urlParse = require('../common/urlParse')

// Set to see communication between WebTorrent and torrent viewer tabs
const DEBUG_IPC = false
Expand Down
11 changes: 10 additions & 1 deletion app/common/urlParse.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,17 @@
* You can obtain one at http://mozilla.org/MPL/2.0/. */

const LRUCache = require('lru-cache')
const urlParse = require('url').parse
const config = require('../../js/constants/config')

// muon.url.parse is not available in all environments (ex: unittests)
let urlParse
try {
urlParse = muon.url.parse
} catch (e) {
// TODO: move to the new node URL API: https://nodejs.org/api/url.html#url_url
urlParse = require('url').parse
}

let cachedUrlParse = new LRUCache(config.cache.urlParse)

module.exports = (url, ...args) => {
Expand Down
9 changes: 5 additions & 4 deletions js/webtorrent/entry.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,12 @@ const path = require('path')
const querystring = require('querystring')
const React = require('react')
const ReactDOM = require('react-dom')
const url = require('url')
const urlFormat = require('url').format
const WebTorrentRemoteClient = require('webtorrent-remote/client')

const App = require('./components/app')
const messages = require('../constants/messages')
const urlParse = require('../../app/common/urlParse')

// Stylesheets
require('../../less/webtorrent.less')
Expand All @@ -37,7 +38,7 @@ window.addEventListener('hashchange', init)
function init () {
store.torrentId = window.decodeURIComponent(window.location.hash.substring(1))

const parsedUrl = url.parse(store.torrentId)
const parsedUrl = urlParse(store.torrentId)
store.torrentIdProtocol = parsedUrl.protocol

// `ix` param can be set by query param or hash, e.g. ?ix=1 or #ix=1
Expand Down Expand Up @@ -182,10 +183,10 @@ function stop () {
}

function saveTorrentFile () {
const parsedUrl = url.parse(store.torrentId, true)
const parsedUrl = urlParse(store.torrentId, true)
parsedUrl.query.download = true
const name = path.basename(parsedUrl.pathname) || 'untitled.torrent'
const href = url.format(parsedUrl)
const href = urlFormat(parsedUrl)

const a = document.createElement('a')
a.rel = 'noopener'
Expand Down

0 comments on commit d36a1c4

Please sign in to comment.