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

Commit

Permalink
Fix for multiple registrations on same session
Browse files Browse the repository at this point in the history
Turns out (obvious in hindsight) that each window shares the same session object.  This refactors and fixes some bugs

Auditors: @diracdeltas
  • Loading branch information
bbondy committed Jan 17, 2016
1 parent e6ea975 commit 5370f82
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 22 deletions.
33 changes: 24 additions & 9 deletions app/filtering.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,42 +5,53 @@
'use strict'

const messages = require('../js/constants/messages')
const electron = require('electron')
const session = electron.session
const BrowserWindow = electron.BrowserWindow

const filteringFns = []

module.exports.registerFilteringCB = filteringFn => {
filteringFns.push(filteringFn)
}

module.exports.registerWindow = wnd => {
wnd.webContents.session.webRequest.onBeforeSendHeaders(function (wnd, details, cb) {
/**
* Register for notifications for webRequest notifications for
* a particular session.
* @param {object} The session to add webRequest filtering on
*/
function registerForSession (session) {
session.webRequest.onBeforeSendHeaders(function (details, cb) {
// Using an electron binary which isn't from Brave
if (!details.firstPartyUrl) {
cb({})
return
}

let results
let cbArgs
for (let i = 0; i < filteringFns.length; i++) {
results = filteringFns[i](details)
cbArgs = cbArgs || results.cbArgs
if (results.shouldBlock) {
break
}
}

if (!results) {
cb({})
} else if (results.cbArgs) {
cb(results.cbArgs)
} else {
if (results.shouldBlock) {
wnd.webContents.send(messages.BLOCKED_RESOURCE, results.resourceName, details)
}
} else if (results.shouldBlock) {
// We have no good way of knowing which BrowserWindow the blocking is for
// yet so send it everywhere and let listeners decide how to respond.
BrowserWindow.getAllWindows().forEach(wnd =>
wnd.webContents.send(messages.BLOCKED_RESOURCE, results.resourceName, details))
cb({
cancel: results.shouldBlock
})
} else {
cb(cbArgs || {})
}
}.bind(null, wnd))
})
}

module.exports.isThirdPartyHost = (baseContextHost, testHost) => {
Expand All @@ -52,3 +63,7 @@ module.exports.isThirdPartyHost = (baseContextHost, testHost) => {
return c !== '.' && c !== undefined
}

module.exports.init = () => {
registerForSession(session.fromPartition(''))
registerForSession(session.fromPartition('private-1'))
}
23 changes: 16 additions & 7 deletions app/httpsEverywhere.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ const sqlite3 = require('sqlite3')
const path = require('path')
const urlParse = require('url').parse
const DataFile = require('./dataFile')
const electron = require('electron')
const session = electron.session

let httpsEverywhereInitialized = false
var dbLoaded = false
Expand Down Expand Up @@ -229,14 +231,11 @@ function canonicalizeUrl (url) {
}

/**
* Loads HTTPS Everywhere
* Register for notifications for webRequest notifications for
* a particular session.
* @param {object} The session to add webRequest filtering on
*/
module.exports.init = () => {
DataFile.init('httpsEverywhere', startHttpsEverywhere, loadRulesets)
}

module.exports.registerWindow = wnd => {
var session = wnd.webContents ? wnd.webContents.session : null
function registerForSession (session) {
if (!session) {
console.log('could not get window session')
return null
Expand All @@ -255,3 +254,13 @@ module.exports.registerWindow = wnd => {
urls: ['https://*/*']
}, onBeforeRedirect)
}

/**
* Loads HTTPS Everywhere
*/
module.exports.init = () => {
DataFile.init('httpsEverywhere', startHttpsEverywhere, loadRulesets)
registerForSession(session.fromPartition(''))
registerForSession(session.fromPartition('private-1'))
}

2 changes: 2 additions & 0 deletions app/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ const SessionStore = require('./sessionStore')
const AppStore = require('../js/stores/appStore')
const CrashHerald = require('./crash-herald')
const PackageLoader = require('./package-loader')
const Filtering = require('./filtering')
const TrackingProtection = require('./trackingProtection')
const AdBlock = require('./adBlock')
const HttpsEverywhere = require('./httpsEverywhere')
Expand Down Expand Up @@ -125,6 +126,7 @@ app.on('ready', function () {
// Load HTTPS Everywhere browser "extension"
HttpsEverywhere.init()

Filtering.init()
TrackingProtection.init()
AdBlock.init()
SiteHacks.init()
Expand Down
6 changes: 0 additions & 6 deletions js/stores/appStore.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@ const LocalShortcuts = require('../../app/localShortcuts')
const AppActions = require('../actions/appActions')
const firstDefinedValue = require('../lib/functional').firstDefinedValue
const Serializer = require('../dispatcher/serializer')
const Filtering = require('../../app/filtering')
const HttpsEverywhere = require('../../app/httpsEverywhere')

let appState

Expand Down Expand Up @@ -106,10 +104,6 @@ const createWindow = (browserOpts, defaults) => {
webPreferences: defaults.webPreferences
}, browserOpts))

// Load HTTPS Everywhere browser "extension"
HttpsEverywhere.registerWindow(mainWindow)
Filtering.registerWindow(mainWindow)

mainWindow.on('resize', function (evt) {
// the default window size is whatever the last window resize was
AppActions.setDefaultWindowSize(evt.sender.getSize())
Expand Down

1 comment on commit 5370f82

@diracdeltas
Copy link
Member

Choose a reason for hiding this comment

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

++

Please sign in to comment.