783600 - Sanitize the No Proxies Preference more (network.proxy.no_proxies_on).

Status: RESOLVED FIXED

(SeaMonkey :: Preferences, defect)

Description

[Philip Chee]

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)]

Attachment: Sanitize the No Proxies Preference more. (v1)

Comment 2

[Ian Neal]

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)]

(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]

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)]

Attachment: Sanitize the No Proxies Preference more. (v2)

Comment 6

[Ian Neal]

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)]

Attachment: Sanitize the No Proxies Preference more. (v3)

Comment 8

[Ian Neal]

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)]

Attachment: Sanitize the No Proxies Preference more. (v4)

Comment 10

[Ian Neal]

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]

(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)]

Attachment: Sanitize the No Proxies Preference more. (v5)

Comment 13

[[e-mail]]

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)]

Attachment: Sanitize the No Proxies Preference more. (v6)

Comment 15

[[e-mail]]

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)]

Attachment: Sanitize the No Proxies Preference more. (v7)

Comment 17

[[e-mail]]

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)]

Attachment: Sanitize the No Proxies Preference more. (v8)

Comment 19

[Ian Neal]

Comment on attachment 672630 [details] [diff] [review]
Sanitize the No Proxies Preference more. (v8)

Great :)

Comment 20

[Edmund Wong (:ewong)]

Pushed to comm-central:
http://hg.mozilla.org/comm-central/rev/0c16b668c0a3