Skip to content

Commit

Permalink
allow Shields to block urls with invalid origins
Browse files Browse the repository at this point in the history
fix brave/brave-browser#5346
fix brave/brave-browser#5483

shields can't manually block urls with invalid origins, making
blob:// and data:// scripts impossible to allow once, for example.
this channge ensures that Shields see such scripts the same way as
scripts with valid origins.
  • Loading branch information
cezaraugusto authored and bsclifton committed Aug 20, 2019
1 parent b3b7461 commit 50b8e6b
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { NoScriptInfo, NoScriptEntry } from '../types/other/noScriptInfo'
import { getLocale } from '../background/api/localeAPI'

// Helpers
import { getOrigin } from './urlUtils'
import { getOrigin, isHttpOrHttps } from './urlUtils'

/**
* Filter resources by origin to be used for generating NoScriptInfo.
Expand Down Expand Up @@ -121,5 +121,5 @@ export const getBlockScriptText = (haveUserInteracted: boolean, isBlocked: boole
export const getAllowedScriptsOrigins = (modifiedNoScriptInfo: NoScriptInfo): Array<string> => {
const getAllowedOrigins = Object.entries(modifiedNoScriptInfo)
.filter(url => url[1].actuallyBlocked === false)
return getAllowedOrigins.map(url => getOrigin(url[0]) + '/')
return getAllowedOrigins.map(url => isHttpOrHttps(url[0]) ? getOrigin(url[0]) + '/' : url[0])
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,27 @@ export const isHttpOrHttps = (url?: string) => {
* Get the URL origin via Web API
* @param {string} url - The URL to get the origin from
*/
export const getOrigin = (url: string) => new window.URL(url).origin
export const getOrigin = (url: string) => {
// for URLs such as blob:// and data:// that doesn't have a
// valid origin, return the full url.
if (!isHttpOrHttps(url)) {
return url
}
return new window.URL(url).origin
}

/**
* Get the URL hostname via Web API
* @param {string} url - The URL to get the origin from
*/
export const getHostname = (url: string) => new window.URL(url).hostname
export const getHostname = (url: string) => {
// for URLs such as blob:// and data:// that doesn't have a
// valid origin, return the full url.
if (!isHttpOrHttps(url)) {
return url
}
return new window.URL(url).hostname
}

/**
* Strip http/https protocol
Expand Down
8 changes: 8 additions & 0 deletions components/test/brave_extension/helpers/urlUtils_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,12 +49,20 @@ describe('urlUtils test', () => {
const url = 'https://pokemons-invading-tests-breaking-stuff.com/you-knew-that.js'
expect(getOrigin(url)).toBe('https://pokemons-invading-tests-breaking-stuff.com')
})
it('returns the full URL if origin is not valid', () => {
const url = 'data:application/javascript;base64,c3z4r4vgvst0n00b'
expect(getOrigin(url)).toBe('data:application/javascript;base64,c3z4r4vgvst0n00b')
})
})
describe('getHostname', () => {
it('properly gets the hostname from an URL', () => {
const url = 'https://pokemons-invading-tests-breaking-stuff.com/you-knew-that.js'
expect(getHostname(url)).toBe('pokemons-invading-tests-breaking-stuff.com')
})
it('returns the full URL if origin is not valid', () => {
const url = 'data:application/javascript;base64,c3z4r4vgvst0n00b'
expect(getHostname(url)).toBe('data:application/javascript;base64,c3z4r4vgvst0n00b')
})
})
describe('stripProtocolFromUrl', () => {
it('properly strips out an HTTP protocol', () => {
Expand Down
3 changes: 2 additions & 1 deletion renderer/brave_content_settings_observer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,8 @@ bool BraveContentSettingsObserver::IsScriptTemporilyAllowed(
const GURL& script_url) {
// check if scripts from this origin are temporily allowed or not
return base::ContainsKey(temporarily_allowed_scripts_,
script_url.GetOrigin().spec());
script_url.GetOrigin().spec()) ||
base::ContainsKey(temporarily_allowed_scripts_, script_url.spec());
}

void BraveContentSettingsObserver::BraveSpecificDidBlockJavaScript(
Expand Down

0 comments on commit 50b8e6b

Please sign in to comment.