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

Whitespace in url fixes #3167 #3168

Merged
merged 1 commit into from
Aug 15, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions js/components/urlBar.js
Original file line number Diff line number Diff line change
Expand Up @@ -106,9 +106,7 @@ class UrlBar extends ImmutableComponent {
// Filter javascript URLs to prevent self-XSS
location = location.replace(/^(\s*javascript:)+/i, '')
const isLocationUrl = isUrl(location)
// If control key is pressed and input has no space in it add www. as a prefix and .com as a suffix.
// For whitepsace we want a search no matter what.
if (!isLocationUrl && !/\s/g.test(location) && e.ctrlKey) {
if (!isLocationUrl && e.ctrlKey) {
windowActions.loadUrl(this.activeFrame, `www.${location}.com`)
} else if (this.shouldRenderUrlBarSuggestions && (this.urlBarSuggestions.activeIndex > 0 || this.props.locationValueSuffix)) {
// Hack to make alt enter open a new tab for url bar suggestions when hitting enter on them.
Expand Down
2 changes: 1 addition & 1 deletion js/lib/appUrlUtil.js
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ module.exports.isTargetAboutUrl = function (input) {
*/
module.exports.isUrl = function (input) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

@bbondy we should just get rid of this method and use UrlUtil

Copy link
Member

Choose a reason for hiding this comment

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

yes I agree it was only ever added here at some point because urlUtil used to be an external module we couldn't modify.

input = input.trim()
return UrlUtil.isURL(input) && !/\s/g.test(input)
return UrlUtil.isURL(input)
}

/**
Expand Down
16 changes: 13 additions & 3 deletions js/lib/urlutil.js
Original file line number Diff line number Diff line change
Expand Up @@ -91,30 +91,40 @@ const UrlUtil = {
return true
}

// for cases where we have scheme and we dont want spaces in domain names
const caseDomain = /^[\w]{2,5}:\/\/[^\s\/]+\//
// for cases, quoted strings
const case1Reg = /^".*"$/
// for cases, ?abc and "a? b" which should searching query
const case2Reg = /^(\?)|(\?.+\s)/
// for cases, pure string
const case3Reg = /[\?\.\/\s:]/
// for cases, data:uri and view-source:uri
const case4Reg = /^\w+:.*/
// for cases, data:uri, view-source:uri and about
const case4Reg = /^data|view-source|about|chrome-extension:.*/

let str = input.trim()
let scheme = this.getScheme(str)

if (str.toLowerCase() === 'localhost') {
return false
}

if (case1Reg.test(str)) {
return true
}

if (case2Reg.test(str) || !case3Reg.test(str) ||
this.getScheme(str) === str) {
(scheme === undefined && /\s/g.test(str))) {
return true
}
if (case4Reg.test(str)) {
return !this.canParseURL(str)
}

if (scheme && (scheme !== 'file://')) {
return !caseDomain.test(str + '/')
}

str = this.prependScheme(str)
return !this.canParseURL(str)
},
Expand Down
18 changes: 18 additions & 0 deletions test/unit/lib/urlutilTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,24 @@ describe('urlutil', function () {
it('returns true when input is a pure string (no TLD)', function () {
assert.equal(UrlUtil.isNotURL('brave'), true)
})
it('returns false when input is a string with whitespace but has schema', function () {
assert.equal(UrlUtil.isNotURL('https://wwww.brave.com/test space.jpg'), false)
})
it('returns true when input is a string with schema but invalid domain name', function () {
assert.equal(UrlUtil.isNotURL('https://www.bra ve.com/test space.jpg'), true)
})
it('returns true when input contains more than one word', function () {
assert.equal(UrlUtil.isNotURL('brave is cool'), true)
})
it('returns false when input has custom protocol', function () {
assert.equal(UrlUtil.isNotURL('brave://test'), false)
})
it('returns true when input has space in schema', function () {
assert.equal(UrlUtil.isNotURL('https ://brave.com'), true)
})
it('returns false when input is chrome-extension', function () {
assert.equal(UrlUtil.isNotURL('chrome-extension://fmfcbgogabcbclcofgocippekhfcmgfj/cast_sender.js'), false)
})
describe('search query', function () {
it('returns true when input starts with ?', function () {
assert.equal(UrlUtil.isNotURL('?brave'), true)
Expand Down