Last Comment Bug 783600 - Sanitize the No Proxies Preference more (network.proxy.no_proxies_on).
: Sanitize the No Proxies Preference more (network.proxy.no_proxies_on).
Status: RESOLVED FIXED
:
Product: SeaMonkey
Classification: Client Software
Component: Preferences (show other bugs)
: Trunk
: All All
: -- normal (vote)
: ---
Assigned To: Edmund Wong (:ewong)
:
Mentors:
Depends on: 613198
Blocks:
  Show dependency treegraph
 
Reported: 2012-08-17 10:17 PDT by Philip Chee
Modified: 2012-10-23 20:46 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Sanitize the No Proxies Preference more. (v1) (822 bytes, patch)
2012-08-20 23:58 PDT, Edmund Wong (:ewong)
iann_bugzilla: review-
Details | Diff | Review
Sanitize the No Proxies Preference more. (v2) (897 bytes, patch)
2012-09-14 07:37 PDT, Edmund Wong (:ewong)
iann_bugzilla: review-
Details | Diff | Review
Sanitize the No Proxies Preference more. (v3) (979 bytes, patch)
2012-09-25 07:30 PDT, Edmund Wong (:ewong)
iann_bugzilla: review-
Details | Diff | Review
Sanitize the No Proxies Preference more. (v4) (1.10 KB, patch)
2012-10-03 01:47 PDT, Edmund Wong (:ewong)
iann_bugzilla: review-
Details | Diff | Review
Sanitize the No Proxies Preference more. (v5) (967 bytes, patch)
2012-10-09 19:38 PDT, Edmund Wong (:ewong)
neil: feedback-
Details | Diff | Review
Sanitize the No Proxies Preference more. (v6) (1.00 KB, patch)
2012-10-12 19:42 PDT, Edmund Wong (:ewong)
neil: feedback-
Details | Diff | Review
Sanitize the No Proxies Preference more. (v7) (804 bytes, patch)
2012-10-17 01:41 PDT, Edmund Wong (:ewong)
neil: feedback+
Details | Diff | Review
Sanitize the No Proxies Preference more. (v8) (795 bytes, patch)
2012-10-17 18:32 PDT, Edmund Wong (:ewong)
iann_bugzilla: review+
ewong: feedback+
Details | Diff | Review

Description Philip Chee 2012-08-17 10:17:31 PDT
In Bug 613198 Ewong changed the "No Proxy for" list UI from a textbox to a multiline textarea.

This is similar to the Firefox Bug 566444 (Make Proxy Exceptions MultiLine) except that they made an additional change to the sanitize regexp:

https://hg.mozilla.org/mozilla-central/annotate/218771a2c9db/browser/components/preferences/connection.js#l162

  sanitizeNoProxiesPref: function()
  {
    var noProxiesPref = document.getElementById("network.proxy.no_proxies_on");
    // replace substrings of ; and \n with commas if they're neither immediately
    // preceded nor followed by a valid separator character 
    noProxiesPref.value = noProxiesPref.value.replace(/([^, \n;])[;\n]+(?![,\n;])/g, '$1,');
    // replace any remaining ; and \n since some may follow commas, etc.
    noProxiesPref.value = noProxiesPref.value.replace(/[;\n]/g, '');
  },
Comment 1 Edmund Wong (:ewong) 2012-08-20 23:58:09 PDT
Created attachment 653668 [details] [diff] [review]
Sanitize the No Proxies Preference more. (v1)
Comment 2 Ian Neal 2012-08-27 14:01:03 PDT
What should happen with:
example.net \n example.com
Should all spaces and/or \n be treated as separators too?
Comment 3 Edmund Wong (:ewong) 2012-08-30 21:02:41 PDT
(In reply to Ian Neal from comment #2)
> What should happen with:
> example.net \n example.com
> Should all spaces and/or \n be treated as separators too?

i.e. example.net \n example.com
becomes example.net; example.com
Comment 4 Ian Neal 2012-09-02 02:35:55 PDT
Comment on attachment 653668 [details] [diff] [review]
Sanitize the No Proxies Preference more. (v1)

This fails in the following STR
1/ In the No Proxy for field at the end of the existing exceptions, hit the space bar then ; then CR then on the new line type something like "example.com"
2/ Click Close button
3/ Open about:config

Shows "localhost, 127.0.0.1 example.com" - note the missing separator.
Comment 5 Edmund Wong (:ewong) 2012-09-14 07:37:52 PDT
Created attachment 661220 [details] [diff] [review]
Sanitize the No Proxies Preference more. (v2)
Comment 6 Ian Neal 2012-09-24 11:03:16 PDT
Comment on attachment 661220 [details] [diff] [review]
Sanitize the No Proxies Preference more. (v2)

This fails to deal with things like multiple separators (e.g. ;; or ,;) with or without spaces between them.
Comment 7 Edmund Wong (:ewong) 2012-09-25 07:30:05 PDT
Created attachment 664499 [details] [diff] [review]
Sanitize the No Proxies Preference more. (v3)
Comment 8 Ian Neal 2012-09-30 04:59:22 PDT
Comment on attachment 664499 [details] [diff] [review]
Sanitize the No Proxies Preference more. (v3)

"example.com ;        example.net ; , ;, ,; example.org ,"
Still doesn't deal with something like the above.
Using .replace(/( |,|;|\n)+/g, ", ") seems to deal with most of the above test apart from separators at the end.
Using .replace(/( |,|;|\n)+/g, ", ").replace(/( |,|;|\n)+$/g,"") appears to deal with it all.
Comment 9 Edmund Wong (:ewong) 2012-10-03 01:47:15 PDT
Created attachment 667381 [details] [diff] [review]
Sanitize the No Proxies Preference more. (v4)
Comment 10 Ian Neal 2012-10-07 08:44:16 PDT
Comment on attachment 667381 [details] [diff] [review]
Sanitize the No Proxies Preference more. (v4)

As far as I am aware you should only need the last two lines.
Please test and also check with Neil if that is the most efficient way of doing it.
Comment 11 Ian Neal 2012-10-07 08:44:53 PDT
(In reply to Ian Neal from comment #10)
> Comment on attachment 667381 [details] [diff] [review]
> Sanitize the No Proxies Preference more. (v4)
> 
> As far as I am aware you should only need the last two lines.
i.e. .replace(/( |,|;|\n)+/g, ", ").replace(/( |,|;|\n)+$/g,"")
Comment 12 Edmund Wong (:ewong) 2012-10-09 19:38:59 PDT
Created attachment 669837 [details] [diff] [review]
Sanitize the No Proxies Preference more. (v5)
Comment 13 neil@parkwaycc.co.uk 2012-10-10 14:18:00 PDT
Comment on attachment 669837 [details] [diff] [review]
Sanitize the No Proxies Preference more. (v5)

So, the Firefox version doesn't actually sanitise spaces. You could duplicate that by replacing all semicolons and newlines with commas, then collapsing all runs of commas and spaces to a single comma and space.

Or, if you prefer IanN's version, you could simply collapse all runs of commas, semicolons, spaces and newlines to a single comma and space.

Either way you may want to remove a leading and/or trailing comma and space. Note that you don't have to worry about semicolons or newlines since you removed them earlier.

Also, please use character classes instead of alternation to match one of a selection of characters. For instance, use [ ,;\n] instead of ( |,|;|\n).

The other approach is to try to match runs of the non-separator characters, then joining the resulting array with a single comma and space. (Matching non-separator characters is of course trivial with a character class.)
Comment 14 Edmund Wong (:ewong) 2012-10-12 19:42:03 PDT
Created attachment 671016 [details] [diff] [review]
Sanitize the No Proxies Preference more. (v6)
Comment 15 neil@parkwaycc.co.uk 2012-10-14 13:49:43 PDT
Comment on attachment 671016 [details] [diff] [review]
Sanitize the No Proxies Preference more. (v6)

I'm still not sure what you're trying to do here. If you don't want to use that convoluted Firefox regexp (and I'd suggest you don't) then .replace(/[;, \n]+/g, ", ") suffices to fix up most separators for you, although it will potentially leave a leading and/or trailing comma, which you could take care of in a subsequent replace.

>+  noProxiesPref.value = noProxiesPref.value.replace(/([^, \n;])[;\n]+(?![,\n;])/g, '$1,');
>+    // replace any remaining ; and \n since some may follow commas, etc.
>+  noProxiesPref.value = noProxiesPref.value.replace(/[ ,]+/g, ", ")
>+                                           .replace(/[ ,]+$/g,"")
>+                                           .replace(/^[ ,;\n]+/g,"")
1. Avoid setting .value more than once. Instead, just use as many .replace clauses as you need in one statement.
2. No point using /^.../g or /...$/g, since a string only has one beginning and one end, although /^...|...$/g works to do both at once.
Comment 16 Edmund Wong (:ewong) 2012-10-17 01:41:34 PDT
Created attachment 672209 [details] [diff] [review]
Sanitize the No Proxies Preference more. (v7)
Comment 17 neil@parkwaycc.co.uk 2012-10-17 02:28:32 PDT
Comment on attachment 672209 [details] [diff] [review]
Sanitize the No Proxies Preference more. (v7)

This should work, although the "\n"s on the second line are unnecessary, since they will all have been replaced by the first line, and you're missing a "+" after one of the ]s. (Alternatively if you put the comma before the space then you can drop the []+ completely.)
Comment 18 Edmund Wong (:ewong) 2012-10-17 18:32:16 PDT
Created attachment 672630 [details] [diff] [review]
Sanitize the No Proxies Preference more. (v8)
Comment 19 Ian Neal 2012-10-21 14:13:24 PDT
Comment on attachment 672630 [details] [diff] [review]
Sanitize the No Proxies Preference more. (v8)

Great :)
Comment 20 Edmund Wong (:ewong) 2012-10-23 20:46:33 PDT
Pushed to comm-central:
http://hg.mozilla.org/comm-central/rev/0c16b668c0a3

Note You need to log in before you can comment on or make changes to this bug.