Merge lp:~angeloc/unity-lens-files/fix-for-948086 into lp:unity-lens-files

Proposed by Angelo Compagnucci on 2012-03-07
Status: Merged
Approved by: Michal Hruby on 2012-03-13
Approved revision: 225
Merged at revision: 218
Proposed branch: lp:~angeloc/unity-lens-files/fix-for-948086
Merge into: lp:unity-lens-files
Prerequisite: lp:~angeloc/unity-lens-files/fix-for-773841
Diff against target: 33 lines (+15/-1)
1 file modified
src/url-checker.vala (+15/-1)
To merge this branch: bzr merge lp:~angeloc/unity-lens-files/fix-for-948086
Reviewer Review Type Date Requested Status
Mikkel Kamstrup Erlandsen (community) Needs Fixing on 2012-03-20
Michal Hruby (community) 2012-03-07 Approve on 2012-03-07
Review via email: mp+96310@code.launchpad.net

This proposal supersedes a proposal from 2012-03-06.

Description of the change

More robust url validation.

To post a comment you must log in.
Michal Hruby (mhr3) wrote : Posted in a previous version of this proposal

Can you please set your branch which fixes the mountable uris as a prerequisite branch for this?

review: Resubmit
Michal Hruby (mhr3) wrote :

Can we move all the TLDs into a const string, so that we'll have just:

web_regex = new Regex ("(http[s]{0,1}://.+){0,1}[a-zA-Z0-9\\-\\.]+\\." + TLD_REGEX + "$");

Also, this regex should be definitely compiled with RegexCompileFlags.OPTIMIZE

review: Needs Fixing
Angelo Compagnucci (angeloc) wrote :

Done!

Splitting regex into const string
Use OPTIMIZE flag for regexp

Angelo Compagnucci (angeloc) wrote :

Done!

Codestyle fixed!

Michal Hruby (mhr3) wrote :

Looking good to me.

review: Approve
Unity Merger (unity-merger) wrote :

The prerequisite lp:~angeloc/unity-lens-files/fix-for-773841 has not yet been merged into lp:unity-lens-files.

Sorry, seems this branch slipped past my watchful eye

This branch caused a regression. You can no longer type in a URL with path, query, or fragment components. The regex doesn't match beyond the tld.

review: Needs Fixing

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/url-checker.vala'
2--- src/url-checker.vala 2012-03-07 15:17:18 +0000
3+++ src/url-checker.vala 2012-03-07 15:17:18 +0000
4@@ -34,6 +34,18 @@
5 /* Regexes URLs must match */
6 private Regex web_regex;
7 private Regex mountable_regex;
8+ private const string TLD_REGEX = "(ac|ad|ae|aero|af|ag|ai|al|am|an|ao|aq|ar|arpa|" +
9+ "as|asia|at|au|aw|ax|az|ba|bb|bd|be|bf|bg|bh|bi|biz|bj|bm|bn|bo|br|bs|bt|bv|bw|" +
10+ "by|bz|ca|cat|cc|cd|cf|cg|ch|ci|ck|cl|cm|cn|co|com|coop|cr|cu|cv|cw|cx|cy|cz|de|" +
11+ "dj|dk|dm|do|dz|ec|edu|ee|eg|er|es|et|eu|fi|fj|fk|fm|fo|fr|ga|gb|gd|ge|gf|gg|gh|" +
12+ "gi|gl|gm|gn|gov|gp|gq|gr|gs|gt|gu|gw|gy|hk|hm|hn|hr|ht|hu|id|ie|il|im|in|info|" +
13+ "int|io|iq|ir|is|it|je|jm|jo|jobs|jp|ke|kg|kh|ki|km|kn|kp|kr|kw|ky|kz|la|lb|lc|" +
14+ "li|lk|lr|ls|lt|lu|lv|ly|ma|mc|md|me|mg|mh|mil|mk|ml|mm|mn|mo|mobi|mp|mq|mr|ms|mt|" +
15+ "mu|museum|mv|mw|mx|my|mz|na|name|nc|ne|net|nf|ng|ni|nl|no|np|nr|nu|nz|om|org|pa|" +
16+ "pe|pf|pg|ph|pk|pl|pm|pn|pr|pro|ps|pt|pw|py|qa|re|ro|rs|ru|rw|sa|sb|sc|sd|se|sg|" +
17+ "sh|si|sj|sk|sl|sm|sn|so|sr|st|su|sv|sx|sy|sz|tc|td|tel|tf|tg|th|tj|tk|tl|tm|tn|" +
18+ "to|tp|tr|travel|tt|tv|tw|tz|ua|ug|uk|us|uy|uz|va|vc|ve|vg|vi|vn|vu|wf|ws|xn|xxx|" +
19+ "ye|yt|za|zm|zw|local|[0-9]{1,3})";
20
21 public UrlChecker ()
22 {
23@@ -41,7 +53,9 @@
24 mountable_icon = new ThemedIcon ("folder-remote").to_string ();
25
26 try {
27- web_regex = new Regex ("([a-zA-Z0-9\\-]+\\...+|http[s]{0,1}://.+)");
28+ web_regex = new Regex ("(http[s]{0,1}://.+){0,1}" +
29+ "[a-zA-Z0-9\\-\\.]+\\." + TLD_REGEX + "$",
30+ RegexCompileFlags.OPTIMIZE);
31 mountable_regex = new Regex ("(\\\\|(ftp|ssh|sftp|smb|dav)://).+");
32 } catch (RegexError e) {
33 warning ("Error compiling regular expressions for URL matching. URL launching will not work: %s", e.message);

Subscribers

People subscribed via source and target branches