Merge lp:~daker/webbrowser-app/fix.1317428 into lp:webbrowser-app

Proposed by Adnane Belmadiaf
Status: Needs review
Proposed branch: lp:~daker/webbrowser-app/fix.1317428
Merge into: lp:webbrowser-app
Diff against target: 48 lines (+21/-15)
1 file modified
src/app/AddressBar.qml (+21/-15)
To merge this branch: bzr merge lp:~daker/webbrowser-app/fix.1317428
Reviewer Review Type Date Requested Status
Ubuntu Phablet Team Pending
Review via email: mp+225759@code.launchpad.net

Description of the change

Unfortunately the regexp doesn't work all the time with the QML JS interpreter

QML :

OK : http://gmail.com
NO : http://m.facebook.com
OK : http://www.google.com
NO : http://www.fb.com
OK : https://facebook.com
NO : http://guh.guru:8080
OK : http://userid:<email address hidden>:8080
NO : http://<email address hidden>
OK : http://192.168.1.1/
NO : http://machine.local
OK : http://192.168.1.1:8080/
NO : www fff
NO : facebook

Chrome/Firefox JS interpreter

OK : http://gmail.com
OK : http://m.facebook.com
OK : http://www.google.com
OK : http://www.fb.com
OK : https://facebook.com
OK : http://guh.guru:8080
OK : http://userid:<email address hidden>:8080
OK : http://<email address hidden>
OK : http://192.168.1.1/
OK : http://machine.local
OK : http://192.168.1.1:8080/
NO : www fff
NO : facebook

To post a comment you must log in.
lp:~daker/webbrowser-app/fix.1317428 updated
608. By Adnane Belmadiaf

Validate only hostnames with allowed characters according to RFC 3986

Revision history for this message
Olivier Tilloy (osomon) wrote :

11 + if (invalid_hostname_characters.match(address)){
12 + return false
13 + }

It doesn’t seem correct to me to apply the invalid_hostname_characters RE to the entire address: you’d want to apply it to the host part only. But since we don’t really know whether address is a valid URl yet, there’s no easy way to extract the host part. So I’m not sure this makes sense.

Revision history for this message
Olivier Tilloy (osomon) wrote :

If we really want to be strict about URL validation, I think we shouldn’t reinvent the wheel. QUrl provides us everything we need to parse a URL from a string and check whether it’s valid. Unfortunately it’s not exposed to QML (the url type in QML doesn’t match our needs), so we’d need to expose a C++ helper to QML to validate a given URL.

Unmerged revisions

608. By Adnane Belmadiaf

Validate only hostnames with allowed characters according to RFC 3986

607. By Adnane Belmadiaf

Added a new regex for URL validation

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/app/AddressBar.qml'
2--- src/app/AddressBar.qml 2014-06-30 11:01:27 +0000
3+++ src/app/AddressBar.qml 2014-07-06 17:46:49 +0000
4@@ -129,24 +129,30 @@
5 }
6
7 function looksLikeAUrl(address) {
8- var terms = address.split(/\s/)
9+ // Validate only hostnames with allowed characters according to RFC 3986
10+ var invalid_hostname_characters = /[^a-zA-Z0-9\.-]/;
11+ if (invalid_hostname_characters.match(address)){
12+ return false
13+ }
14+
15+ var address_tmp = fixUrl(address);
16+ var terms = address_tmp.split(/\s/)
17+
18 if (terms.length > 1) {
19 return false
20 }
21- if (address.substr(0, 1) == "/") {
22- return true
23- }
24- if (address.match(/^https?:\/\//) ||
25- address.match(/^file:\/\//) ||
26- address.match(/^[a-z]+:\/\//)) {
27- return true
28- }
29- if (address.split('/', 1)[0].match(/\.[a-zA-Z]{2,4}$/)) {
30- return true
31- }
32- if (address.split('/', 1)[0].match(/^(?:[0-9]{1,3}\.){3}[0-9]{1,3}/)) {
33- return true
34- }
35+
36+ if (address_tmp.substr(0, 1) == "/") {
37+ return true
38+ }
39+
40+ // Regex expression used is "gruber revised" (@gruber v2)
41+ // http://rodneyrehm.de/t/url-regex.html
42+ var re = /\b((?:[a-z][\w-]+:(?:\/{1,3}|[a-z0-9%])|www\d{0,3}[.]|[a-z0-9.\-]+[.][a-z]{2,4}\/)(?:[^\s()<>]+|\(([^\s()<>]+|(\([^\s()<>]+\)))*\))+(?:\(([^\s()<>]+|(\([^\s()<>]+\)))*\)|[^\s`!()\[\]{};:'".,<>?«»“”‘’]))/gi;
43+ if (re.test(address_tmp)){
44+ return true
45+ }
46+
47 return false
48 }
49

Subscribers

People subscribed via source and target branches

to status/vote changes: