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

Commit

Permalink
Require Flash to be explicitly enabled in preferences before it is us…
Browse files Browse the repository at this point in the history
…able

Auditors: @diracdeltas

Fix #6739
  • Loading branch information
bbondy committed Jan 20, 2017
1 parent 4af2b11 commit 054e7af
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 15 deletions.
2 changes: 1 addition & 1 deletion app/browser/reducers/flashReducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ const flashReducer = (state, action) => {
action = makeImmutable(action)
switch (action.get('actionType')) {
case appConstants.APP_SET_STATE:
flash.init()
flash.init(state)
break
case appConstants.APP_FLASH_PERMISSION_REQUESTED:
flash.showFlashMessageBox(action.get('location'), action.get('senderTabId'))
Expand Down
1 change: 1 addition & 0 deletions js/about/preferences.js
Original file line number Diff line number Diff line change
Expand Up @@ -1552,6 +1552,7 @@ class SecurityTab extends ImmutableComponent {
}
onToggleFlash (e) {
aboutActions.setResourceEnabled(flash, e.target.value)
ipc.send(messages.PREFS_RESTART, flash, e.target.value)
}
onToggleWidevine (e) {
aboutActions.setResourceEnabled(widevine, e.target.value)
Expand Down
26 changes: 18 additions & 8 deletions js/flash.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ const locale = require('../app/locale')
const messages = require('./constants/messages')
const settings = require('./constants/settings')
const {memoize} = require('underscore')
const appConfig = require('./constants/appConfig')

// set to true if the flash install check has succeeded
let flashInstalled = false
Expand Down Expand Up @@ -86,32 +87,41 @@ module.exports.showFlashMessageBox = (location, tabId) => {
})
}

module.exports.checkFlashInstalled = (cb) => {
module.exports.checkFlashInstalled = (state, cb) => {
try {
const pepperFlashSystemPluginPath = getPepperFlashPath()
const pepperFlashManifestPath = path.resolve(pepperFlashSystemPluginPath, '..', 'manifest.json')
fs.readFile(pepperFlashManifestPath, (err, data) => {
let manifest
try {
if (err || !data) {
flashInstalled = false
} else {
const manifest = JSON.parse(data)
app.commandLine.appendSwitch('ppapi-flash-path', pepperFlashSystemPluginPath)
app.commandLine.appendSwitch('ppapi-flash-version', manifest.version)
manifest = JSON.parse(data)
flashInstalled = true
}
} finally {
appActions.changeSetting(settings.FLASH_INSTALLED, flashInstalled)
cb && cb(flashInstalled)
cb && cb({flashInstalled, manifest, pepperFlashSystemPluginPath})
}
})
} catch (e) {
cb && cb(flashInstalled)
cb && cb({flashInstalled, manifest: undefined})
}
}

module.exports.init = () => {
setImmediate(module.exports.checkFlashInstalled)
module.exports.init = (state) => {
const siteSettings = require('./state/siteSettings')
const braveryDefaults = siteSettings.braveryDefaults(state, appConfig)
setImmediate(() => module.exports.checkFlashInstalled(state, ({flashInstalled, manifest, pepperFlashSystemPluginPath}) => {
if (!flashInstalled) {
return
}
if (braveryDefaults.flash) {
app.commandLine.appendSwitch('ppapi-flash-path', pepperFlashSystemPluginPath)
app.commandLine.appendSwitch('ppapi-flash-version', manifest.version)
}
}))
}

module.exports.resourceName = 'flash'
18 changes: 12 additions & 6 deletions js/stores/appStore.js
Original file line number Diff line number Diff line change
Expand Up @@ -351,12 +351,18 @@ function handleChangeSettingAction (settingKey, settingValue) {
}
}

const applyReducers = (state, action) => [
const preReducers = [
require('../../app/browser/reducers/downloadsReducer'),
require('../../app/browser/reducers/flashReducer'),
require('../../app/browser/reducers/tabsReducer'),
require('../../app/browser/reducers/clipboardReducer')
].reduce(
]

const postReducers = [
require('../../app/browser/reducers/flashReducer')
]

const applyReducers = (reducers, state, action) =>
reducers.reduce(
(appState, reducer) => {
const newState = reducer(appState, action)
assert.ok(action.actionType === appConstants.APP_SET_STATE || Immutable.Map.isMap(newState),
Expand All @@ -371,7 +377,7 @@ const handleAppAction = (action) => {

const ledger = require('../../app/ledger')

appState = applyReducers(appState, action)
appState = applyReducers(preReducers, appState, action)

switch (action.actionType) {
case appConstants.APP_SET_STATE:
Expand Down Expand Up @@ -571,8 +577,6 @@ const handleAppAction = (action) => {
case appConstants.APP_ALLOW_FLASH_ONCE:
{
const propertyName = action.isPrivate ? 'temporarySiteSettings' : 'siteSettings'
console.log(siteUtil.getOrigin(action.url))
console.log(propertyName)
appState = appState.set(propertyName,
siteSettings.mergeSiteSetting(appState.get(propertyName), siteUtil.getOrigin(action.url), 'flash', 1))
break
Expand Down Expand Up @@ -826,6 +830,8 @@ const handleAppAction = (action) => {
default:
}

appState = applyReducers(postReducers, appState, action)

emitChanges()
}

Expand Down

2 comments on commit 054e7af

@diracdeltas
Copy link
Member

Choose a reason for hiding this comment

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

confirmed that it fixes #6739 for me.

however it seems that https://get.adobe.com/flashplayer/ is no longer able to detect that flash has been installed when flash is not enabled, since navigator.plugins is empty. (see #6739 (comment) - we had decided that Adobe/Macromedia should always be able to detect the Flash plugin so users don't end up in an install loop.)

minor issue: the 'Allow / Allow once' context menu shouldn't be available when the placeholder says 'this plugin is not supported'

both of these are not blockers though

@bbondy
Copy link
Member Author

@bbondy bbondy commented on 054e7af Jan 20, 2017

Choose a reason for hiding this comment

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

I think @bridiver will try a less aggressive approach with content settings instead

Please sign in to comment.