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

Commit 9e3bf36

Browse files
authored
Merge pull request #15047 from brave/fix/15045
second fix for URL open in new tab
2 parents cb1c815 + 6595b82 commit 9e3bf36

File tree

4 files changed

+48
-4
lines changed

4 files changed

+48
-4
lines changed

app/browser/reducers/tabsReducer.js

+3-2
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ const {getFlashResourceId} = require('../../../js/flash')
3737
const {l10nErrorText} = require('../../common/lib/httpUtil')
3838
const flash = require('../../../js/flash')
3939
const {isSourceAboutUrl, isTargetAboutUrl, isNavigatableAboutPage} = require('../../../js/lib/appUrlUtil')
40-
const {isFileScheme} = require('../../../js/lib/urlutil')
40+
const {isFileScheme, openableByContextMenu} = require('../../../js/lib/urlutil')
4141
const {shouldDebugTabEvents} = require('../../cmdLine')
4242

4343
const getWebRTCPolicy = (state, tabId) => {
@@ -250,7 +250,8 @@ const tabsReducer = (state, action, immutableAction) => {
250250
windows.focus(windowId)
251251
}
252252
const url = action.getIn(['createProperties', 'url'])
253-
if (isFileScheme(url) && !action.get('allowFile')) {
253+
if (!openableByContextMenu(url) ||
254+
(isFileScheme(url) && !action.get('allowFile'))) {
254255
// Don't allow 'open in new tab' to open file:// URLs for security
255256
action = action.setIn(['createProperties', 'url'], 'about:blank')
256257
}

app/browser/reducers/windowsReducer.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ const {makeImmutable, isImmutable} = require('../../common/state/immutableUtil')
2222
const electron = require('electron')
2323
const BrowserWindow = electron.BrowserWindow
2424
const firstDefinedValue = require('../../../js/lib/functional').firstDefinedValue
25-
const {isFileScheme} = require('../../../js/lib/urlutil')
25+
const {isFileScheme, openableByContextMenu} = require('../../../js/lib/urlutil')
2626
const settings = require('../../../js/constants/settings')
2727
const getSetting = require('../../../js/settings').getSetting
2828

@@ -269,7 +269,7 @@ const handleCreateWindowAction = (state, action = Immutable.Map()) => {
269269
} else {
270270
// Don't allow 'open in new window' to open a file:// URL for
271271
// security reasons
272-
if (isFileScheme(frameOpts.location)) {
272+
if (isFileScheme(frameOpts.location) || !openableByContextMenu(frameOpts.location)) {
273273
frameOpts.location = 'about:blank'
274274
}
275275
frames = [ frameOpts ]

js/lib/urlutil.js

+15
Original file line numberDiff line numberDiff line change
@@ -430,6 +430,21 @@ const UrlUtil = {
430430
return urlParse(url).protocol === 'file:'
431431
},
432432

433+
/**
434+
* Checks if URL is safe to open via 'open in new tab/window' context menu
435+
* @param {string} url - URL to check
436+
* @return {boolean}
437+
*/
438+
openableByContextMenu: function (url) {
439+
if (!url) {
440+
return false
441+
}
442+
const protocol = urlParse(url).protocol
443+
// file: is untrusted but handled in a separate check
444+
return ['http:', 'https:', 'ws:', 'wss:', 'magnet:', 'file:', 'data:',
445+
'blob:', 'about:', 'chrome-extension:'].includes(protocol)
446+
},
447+
433448
/**
434449
* Gets the origin of a given URL
435450
* @param {string} url The URL to get the origin from

test/unit/lib/urlutilTestComponents.js

+28
Original file line numberDiff line numberDiff line change
@@ -346,6 +346,34 @@ module.exports = {
346346
}
347347
},
348348

349+
'openableByContextMenu': {
350+
'returns true when input:': {
351+
'is an absolute file path with scheme': (test) => {
352+
test.equal(urlUtil().openableByContextMenu('file:///file/path/to/file'), true)
353+
},
354+
'is http': (test) => {
355+
test.equal(urlUtil().openableByContextMenu('http://example.com'), true)
356+
},
357+
'is https': (test) => {
358+
test.equal(urlUtil().openableByContextMenu('HTtpS://brave.com/?abc=1#test'), true)
359+
},
360+
'is about:blank': (test) => {
361+
test.equal(urlUtil().openableByContextMenu('about:blank'), true)
362+
}
363+
},
364+
'returns false when input:': {
365+
'is chrome:': (test) => {
366+
test.equal(urlUtil().openableByContextMenu('chrome://brave/etc/passwd'), false)
367+
},
368+
'is ssh:': (test) => {
369+
test.equal(urlUtil().openableByContextMenu('ssh://[email protected]'), false)
370+
},
371+
'is null': (test) => {
372+
test.equal(urlUtil().openableByContextMenu(null), false)
373+
}
374+
}
375+
},
376+
349377
'getDisplayHost': {
350378
'url is http': (test) => {
351379
const result = urlUtil().getDisplayHost('http://brave.com')

0 commit comments

Comments
 (0)