Closed Bug 80918 Opened 24 years ago Closed 20 years ago

Proxy: "No Proxy for:" does not support ip address masks - format: ipaddr[/mask][:port]

Categories

(Core :: Networking, enhancement, P4)

enhancement

Tracking

()

VERIFIED FIXED
mozilla1.4beta

People

(Reporter: benc, Assigned: darin.moz)

References

(Blocks 1 open bug)

Details

(Whiteboard: checklinux, checkwin)

Attachments

(1 file, 7 obsolete files)

Numbers Bug 31284 ------- Additional Comments From Scott A. Colcord 2000-04-10 17:15 ------- What if someone wants/needs to use IP addresses? Without the '*', how would someone indicate that they wanted to bypass the proxy for "10.0.*"? That could be way too many hosts to list individually. Bug 48718 ------- Additional Comments From GAThrawn 2001-03-16 09:21 ------- We have a number of test servers referred to purely by their IP addresses, currently the only way to put these into Mozilla's "No proxy for" box is to specify each IP address individually. As they're class B IP addresses the first two bytes of the IP addresses are always the same, it's the last two that differ. eg The IP addresses 151.101.20.3, 151.101.20.4 and 151.101.55.2 all need to be added in, currently they would all need to be entered individually. There doesn't seem to be any way to specify a wildcard for the end of an address, as "151.101.*" doesn't work, neither does "151.101." or "151.101". -- Need to find out how this worked in Communicator. Also need to see if this works by network addresses (10.0.0.0). If we re-do this, need to support arbitrary length subnet masks.
4.x only handled the first * (so *.netscape.com would work... but www.*.com would not!) I think for the first cut, 4.x compatibility would be ok. this should be fixed in-- http://lxr.mozilla.org/seamonkey/source/netwerk/base/src/nsProtocolProxyService.cpp#536
Target Milestone: --- → mozilla0.9.2
mass move, v2. qa to me.
QA Contact: tever → benc
Priority: -- → P3
I find that 4.x does not support the first * in *.mozilla.org. Looks like wildcards are not supported in 4.x, I was asked the username/password for www.mozilla.org, bugzilla.mozilla.org, lxr.mozilla.org. Infact, the help dialog for the prefrence panel for proxies in 4.x states "The wildcard character[*] cannot be used"
Neeit, this bug is in relation to ip address ranges and wildcard masks as a suffix. There are several bugs about dns wildcards, I can reference them if you like.
can some one find out if 4.x supports wildcards in "No Proxy for:"
Keywords: qawanted
Target Milestone: mozilla0.9.2 → mozilla0.9.3
I believe that there is currently a workaround by using the network addresses -- I have have gotten it to work by specifying no proxy for 192.168.13.0 for the whole /24 network.
*** Bug 87777 has been marked as a duplicate of this bug. ***
No proxy for: 172.16.0.0,127.0.0.1 does not work for me. http://172.16.42.71/ still goes thru proxy (in fact, fails to, because 172.16.*.* is behind the firewall) 172.16.42.71 is my own IP
What if you try them one at a time?
Actually, I think he may be right, and our proxy server is just smarter than I had thought. *sigh*
moving out.
Target Milestone: mozilla0.9.3 → mozilla1.0
nsProtocolProxyService::CanUseProxy() is very simplistic in its handling of this -- it simply does string comparisons against everything listed in "No Proxy for". It isn't even aware of the possibility of IP addresses; the hostname passed in is never resolved. I can fix this (or at least produce a patch) -- but I need to know what sort of syntax should be allowed. Possibilities: 1) "192.168.0.0". Consistent with isInNet() PAC syntax, has the problem of being unable to distinguish 10.0.0.0/8 from 10.0.0.0/24. 2) "192.168.0.0/16" People who haven't seen this syntax before may be confused. 3) "192.168.*.*". I hear IE accepts this. 4) "192.168." Stupidly easy to implement (straight prefix match), but bizarre. Thoughts?
1 & 2 - Probably what we need. This might require a lot of bitwise math for supporting the subnets. 3 - Lacks support for non-default subnetting. Not realistic for almost every corporate network and multi-IP DSL user. 4- Might be easy to do with existing parsing features, but pretty ugly. I still don't understand how this worked in communicator, but I wish we didn't do this hanging "dot" thing.
Ok, I've got something for 1 & 2 mostly ready to go. The remaining question is: the "No proxy for.." syntax will currently allow a port specification: .foo.com:8080 If I'm dealing with an address with a mask length, where is the best place to wedge in the port number? 192.168.0.0/16:8080?
I didn't know we supported port numbers. I'm not sure we should, but probably the format: [hostname|ipaddr]:port/masklen would be the prefered way.
I think option "4" (prefix match) is very easy to implement and backward compatible with Nav 4.x .
*** Bug 107528 has been marked as a duplicate of this bug. ***
-qawanted - don't know why it said that... +mozilla 1.0, nsenterprise, 4xp.
I was tinkering with this a few months ago. I'll pick it up again.
Assignee: neeti → tingley
Attached patch ip wildcards support, ver. 0.1 (obsolete) (deleted) — — Splinter Review
This patch allow using ip address wildcards in form <address>/<masklen> and also allows * as prefix for domain names wildcards (i.e. *.mozilla.org) Note that 1. It's for IPv4 addresses only 2. Not ports allowed when /<masklen> used 3. ... there's something else, but I can't recall for now :-(
Wow, the third thing is that I'm not doing dns lookup (thanks Chase for reminder). This is by design - I don't think this is good idea
Attached patch version 0.2 of patch (obsolete) (deleted) — — Splinter Review
Only minor changes and get rid of <TAB>s in this version
It doesn't support IP masks in the form 111.222.* Is it planned to add such thing ?
> It doesn't support IP masks in the form 111.222.* > Is it planned to add such thing ? Nope. See comment #13 above for reasons (option #3)
I still don't understand how to make IP prefixes work (build 2001112703 on Win2000Pro). 192.168.0.0/16 doesn't work, nor 192.168.0.0, when I replace part with zeros with xxx.yy (numbers here) in actual URL
Until this bug is fixed, you have to list IP addresses individually... This is what this bug about ;-)
Keywords: patch, review
*** Bug 118565 has been marked as a duplicate of this bug. ***
*** Bug 115948 has been marked as a duplicate of this bug. ***
Anyones cares to r=/sr= ? Mozilla 1.0 is not so far away :-) Cc'ing gagan, in hope he'll find some time for this...
Wow. I don't believe this bug isn't done yet. I can't promise but I'll try reviewing this soon. From the first glance I am a little concerned about the IP parsing-- specially from IPv6 perspective. cc'ing Mr. IPv6 for his comments. (jgmyers)
I should mention here, because it might be obvious to some (but wasn't to me until I tested it), putting IP addresses in this field only affects URL's that explicitly have the IP address in them. In other words, this feature is purely a string compare feature, it is not robust in any networking sense (if you block 127.0.0.1, http://localhost still will proxy). In this light, I think that supporting Proxy blocking on an address basis is not possible without going to a PAC-based Proxy prefs backend and/or some kind of PAC2 (PACng?) for supporting IPv6.
I don't think that's really true. The nsProxyProtocolService code could be updated to use make DNS resolve calls just like PAC does. I don't like that idea because it would lead to the same problem experienced by PACs that use dnsResolve(): the calls are synchronous and block the UI thread. (see bug 97605, or quick synopsis: this is not a problem that can be fixed easily). I like the idea of supporting this feature for purely numerical IPs. It improves functionality without getting us into trouble by trying to be overly cute. (note also that we already have a semi-functional wildcard scheme -- I can at least cover both cases by specifying |192.168.32.0/24, mylocalnetwork|). Oh, and no wonder this bug's getting no attention, it's assigned to me (oops). -> component owner to push the patch through
Assignee: tingley → new-network-bugs
Knock-knock. Are we gonna miss 1.0? ;-) Let's leave IPv6 out for now. Could anyone comment on the patch?
+ helpwanted - problem definied, solution patch submitted. Is there any nsenterprise representative out there?
Keywords: helpwanted
Chase, check out bug 136789...
I'm trying to figure out a different way to resolve this problem. IMHO, We should treat wildcards in ip addresses or hostnames in the same way.
This patch can support one wildcard in ip addresses (v4/v6) or hostnames. Actually it treat them in the same way. Supported formats are: *.yourcompany.com; 192.168.*, www.*.com Previous formats such like '.foo.com' are also supported. Two wildcards like '*x*' will be ignored. There is one remaining problem: We need to set the port to default port if aURI->GetPort(&port) returns -1 in CanUseProxy (). I tried nsIProtocolHandler.GetDefaultPort(), but it's not implemented in nsHttpHandler. :(
I wouldn't be so confident about IPv6.
IPv4 networks aren't necessarily on octet boundaries. Likewise, IPv6 network prefixes aren't necessarily on word boundaries. IP network matches should support the standard net/mask syntax. For example, the prefix 131.155.72.0/23 should match every address in the range 131.155.72.0 through 131.155.73.255. This notation is documented in section 2.3 of RFC 2373 (for IPv6). I recommend using PR_StringToNetAddr() for parsing IP addresses instead of writing another ad-hoc address parser. That makes IPv6 support much easier.
Attached patch IPv6 ready patch using PRStringToNetAddr() (obsolete) (deleted) — — Splinter Review
Here is new version of patch. Now I use PR_StringToNetAddr() function to parse ip addresses, so it should work with IPv6 addresses as well (not sure about abbreviated IPv6 adresses though; I wonder if underlying inet_pton() calls are capable of parsing /xx mask notation. Currently I parse it by hands). There is also some work left to make AddNoProxyFor() and RemoveNoProxyFor() work with numeric addresses. As for single * anywhere in symbolic no-proxy-for entry, it can be merged here, if anyone want...
Attachment #59326 - Attachment is obsolete: true
Attachment #59512 - Attachment is obsolete: true
Attached patch added port check for ip address entries (obsolete) (deleted) — — Splinter Review
Now ip entry can take form of ipaddr[:port][/mask]
Attachment #80381 - Attachment is obsolete: true
wtc,jgmyers: do you know if inet_pton() is capable of parsing /xx mask notation? does this patch need to handle this notation explicitly?
inet_pton() and PRStringToNetAddr() do not handle /mask notation. One must parse out the /mask part (drop a NUL where the '/' is) before passing the rest of the address to PRStringToNetAddr().
Could you please take a look at the latest patch?
*** Bug 144850 has been marked as a duplicate of this bug. ***
denis: my appologies for not looking at your patch again... i've been very distracted working on last minute bug fixes for mozilla 1.0. i'm hoping to have time to carefully review your patch sometime by next week. reassigning to myself.
Assignee: new-network-bugs → darin
Status: NEW → ASSIGNED
Target Milestone: mozilla1.0 → Future
Blocks: 48718
-> 1.2beta
Target Milestone: Future → mozilla1.2beta
Severity: major → enhancement
Priority: P3 → P4
Blocks: 172083
No longer blocks: 48718
-> moz 1.3
Target Milestone: mozilla1.2beta → mozilla1.3alpha
Attachment #80571 - Flags: superreview?(darin)
Attached patch revised patch: merged to trunk and fixes some bugs (obsolete) (deleted) — — Splinter Review
this patch revises attachment 80571 [details] [diff] [review]. instead of having separate code paths for IPv4 and IPv6 (conditionally compiled into the codebase), i'm simply storing all IP addresses as IPv6. this how the rest of necko handles IP addresses (see PR_ConvertIPv4AddrToIPv6). there was an interesting bug in the original patch. the expression: (~0L << (32 - mask_len)) gives unexpected results when mask_len == 0 on intel processors. both GCC 3.2 and MSVC 6 appear to generate nearly the same code for this expression, and for some reason if mask_len is zero, the result of this expression is ~0L and not 0L as one would expect. GCC 2.95.2 running on OSX 10.1.5 gives a result of 0L as expected. i'm not sure yet what's going on w/ this expression, but i basically just avoided the problem by never letting the code do: (~0L << 32). this code still lacks support for IPv6 address literals, since IPv6 address literals may contain ':' chars. i think IPv6 address literals will need to be []-escaped (e.g., "[::10.169.0.0/16]:80" should be a valid entry in the no proxy for preference). this is the same escaping scheme used w/ URLs (see RFC 2732). the patch also still lacks support for adding/removing IP address literals via the nsIProtocolProxyService::AddNoProxyFor/RemoveNoProxyFor methods. there aren't currently any callers of those functions, so i don't think it's a big deal. while working on this patch, i also cleaned up a good portion of the neighboring code ;-)
Attachment #80571 - Attachment is obsolete: true
Intel CPUs mask out shift counter with 0x1f. That's why whan you try to shiftby 32, you end up doinfg nothing. Seems like GCC bug (generates wrong code).
Vlasenko: yeah, that's sort of what i figured as well. interesting that both GCC and MSVC would both have this bug.
;) Do you have a .c testcase for gcc bug report?
it turns out that it is NOT a compiler bug. in fact the C standard says the behavior is undefined. here's an email conversation i had about this w/ bbaetz: >> i had some fun today, while reviewing/testing a patch, that i thought i >> might share w/ everyone. >> >> what would you expect the value of x to be after evaluating the >> following expression? >> >> int x = (~0 << 32); >> > >"Undefined behaviour". > >C99 says (6.5.7/3): > >"... If the value of the right operand is negative or is greater than or >equal to the width of the promoted left operand, the behaviour is >undefined" > >Don't have C++ handy, but I'd be surprised if it was different. (I think >its like this mainly because of sign bits) > >Bradley
Target Milestone: mozilla1.3alpha → mozilla1.3beta
Attachment #80571 - Flags: superreview?(darin)
How does this relate to bug 48718? This bug break's Chimera's ability to get proxy settings from Internet Config (bug 185649).
Target Milestone: mozilla1.3beta → mozilla1.4alpha
*** Bug 191274 has been marked as a duplicate of this bug. ***
*** Bug 193114 has been marked as a duplicate of this bug. ***
i'll get this in for beta.
Target Milestone: mozilla1.4alpha → mozilla1.4beta
Attachment #107530 - Flags: review?(bbaetz)
Attachment #107530 - Flags: superreview?(bzbarsky)
Comment on attachment 107530 [details] [diff] [review] revised patch: merged to trunk and fixes some bugs Don't we have generic nspr functions for some of this ipv6 stuff? Or at least somewhere outside the proxy stuff? Why IS_ASCII_SPACE - whats wrong with isspace? Why the void->void* change for HandlePACLoadEvent - everywhere just returns NULL
bbaetz: >Don't we have generic nspr functions for some of this ipv6 stuff? Or at least >somewhere outside the proxy stuff? nope, though it is definitely something i'd like to generalize at some point. >Why IS_ASCII_SPACE - whats wrong with isspace? isspace() checks for white-space characters. In the "C" and "POSIX" locales, these are: space, form-feed ('\f'), newline ('\n'), carriage return ('\r'), horizontal tab ('\t'), and vertical tab ('\v'). i only care about ' ' and '\t' >Why the void->void* change for HandlePACLoadEvent - everywhere just returns >NULL to avoid the cast when PL_InitEvent is called. otherwise, we'd miss out on compiler warnings if the signature happened to be slightly off :)
Comment on attachment 107530 [details] [diff] [review] revised patch: merged to trunk and fixes some bugs +nsProtocolProxyService::LoadFilters(const char *filters) { - char* np = (char*)filters; + const char *np = (const char *) filters; You don't need the cast, and it looks like you can just use |filters| directly + nsDependentCString str(host); + hinfo->name.host = ToNewCString(str); + if (!hinfo->name.host) { + delete hinfo; return NS_ERROR_OUT_OF_MEMORY; } - hp->port = iPort; + hinfo->name.host_len = str.Length(); + hinfo->port = port; Why do you need str at all? And why do you need the length? Just for a comparison optimisation? Looks fine apart from that.
Attached patch v1.1 patch (obsolete) (deleted) — — Splinter Review
ok, i eliminated the local variable named |np|. bbaetz: thanks for catching that. as for host_len, i am just storing that as an optimization... it's not much of an optimization, but since i have room in the data structure, why not? (NOTE: the data structure is a union.)
Attachment #79995 - Attachment is obsolete: true
Attachment #107530 - Attachment is obsolete: true
Attachment #107530 - Flags: superreview?(bzbarsky)
Attachment #107530 - Flags: review?(bbaetz)
Attachment #119876 - Flags: review?(bbaetz)
Attachment #119876 - Flags: superreview?(bzbarsky)
Comment on attachment 119876 [details] [diff] [review] v1.1 patch r=bbaetz, but comment that Add/Remove are not used. Also,the prefs panel has documentation for some .foo.bar examples - add an ip address one there, maybe?
Attachment #119876 - Flags: review?(bbaetz) → review+
Before this is checked in, can we get a description of what formats will and won't be supported? Also, a general guess as to how well this will work with older 4xp entries would be nice.
benc: the format supported by the patch is: ipaddr[:port][/mask] on second thought i'm not sure this what we want. it seems like the mask should come before the port, since that will be necessary when we move to support IPv6 address literals. ipaddr[/mask]:[port] makes more sense to me. consider the IPv6 address literal "12AB:0:0:CD30::/60" appending a port number to this would require escaping... [12AB:0:0:CD30::/60]:8080 the current approach would suggest this instead: [12AB:0:0:CD30::]:8080/60 which just doesn't seem natural (or correct) at all.
See, that is why I asked :) Do we really have a use for a "no proxy for ports across CDIR subnet" syntax?" It seems few people asked for port granularity, and they just wanted to snuff out a specific socket. I also think it sounds more complicated, but then again, I don't know much IPv6 addressing. Too many uses of ":"...
if no one objects, i'm going to revert the sense of the check to support this format: ipaddr[/mask]:[port] it just makes better sense. speak up now if you disagree! ;-)
Attached patch v1.2 patch (deleted) — — Splinter Review
few changes: 1- parse ipaddr[/mask][:port] instead of ipaddr[:port][/mask] 2- eliminate AddNoProxyFor and RemoveNoProxyFor since they are unused 3- cleanup IDL (move CID into nsNetCID.h) otherwise no changes since the last patch
Attachment #119876 - Attachment is obsolete: true
Attachment #119876 - Flags: superreview?(bzbarsky)
Attachment #120166 - Flags: superreview?(alecf)
Attachment #120166 - Flags: review?(bbaetz)
Comment on attachment 120166 [details] [diff] [review] v1.2 patch r=bbaetz. Remove the DEBUG_DUMP_FILTERS #define before checking in, though. Oh, and I'm not set up to test the ipv6 stuff at all.
Attachment #120166 - Flags: review?(bbaetz) → review+
bbaetz: thanks for catching the errant #define. as for IPv6.. i've tested it by entering an IPv6 address literal in the address bar (even though i don't have an IPv6 network) to test out whether or not the filters are hit. if not hit, then my proxy server will give me an error page. otherwise, i'll get the usual connection refused message.
of course, since there is no support for IPv6 filters at the moment, all i can really do is check against IPv6 address ranges that fall within the range of IPv4 addresses :-/ another bug will be spawned off of this one to cover supporting IPv6 filters.
Comment on attachment 120166 [details] [diff] [review] v1.2 patch +#define DEBUG_DUMP_FILTERS I assume this will be turned off :) other than that, sr=alecf
Attachment #120166 - Flags: superreview?(alecf) → superreview+
fixed-on-trunk
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Blocks: 201685
i filed bug 201685 to cover IPv6 address literals in the no-proxy-for filters.
Blocks: 48718
*** Bug 210084 has been marked as a duplicate of this bug. ***
*** Bug 226658 has been marked as a duplicate of this bug. ***
The reporter of bug 226658 (nick.howitt@ntlworld.com) claims this problem is back. Can anyone confirm that?
This bug still exists in FB Build Mozilla/5.0 (Windows; U; Win 9x 4.90; en-US; rv:1.6b) Gecko/20031203 Firebird/0.7+ This is not independant confirmation as I was the one who posted that this bug had re-appeared.
Bug still exists on Mozilla/5.0 (Windows; U; Win 9x 4.90; en-US; rv:1.6) Gecko/20040114 Firebird/0.7+
Reopening by request...
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
This bug still exists in Mozilla/5.0 (Windows; U; Win 9x 4.90; en-US; rv:1.8a2) Gecko/20040611 Firefox/0.8.0+ and has existed all year. I've just tried this: 1 - Close FF 2 - Uninstall FF 3 - Rename FF profile directiory to firefox.old 4 - reinstall FF 5 - Start FF 6 - set http proxy to 127.0.0.1 7 - set bypass proxy for 192.168.* Result: New profile created Cannot access my router on 192.168.1.1 Cannot access my cable modem on 192.168.100.1 8 - set bypass proxy for 192.168.1.1 and 192.168.100.1 Result: Can access router and CM Conclsion: Bug is still there on totally clean install.
I've just tested 192.168.0.0/16 and that works. It is only 192.168.* which does not.
the bug is fixed. we did not claim to support the wildcard format.
Status: REOPENED → RESOLVED
Closed: 22 years ago20 years ago
Resolution: --- → FIXED
Summary: Proxy: "No Proxy for:" does not support ip address wildcards → Proxy: "No Proxy for:" does not support ip address masks - format: ipaddr[/mask][:port]
Sad. This neans that Camino can't reliably use the OS proxy settings. See bug 185649.
I still experience this bug on 1.6 even with the 1.2.3.4/12 syntax. Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.6) Gecko/20040113 My "No Proxy For:" line shows the following: localhost, 127.0.0.1, 129.144.0.0/12, 192.168.0.0/16, 10.0.0.0/8, 192.18.0.0/16 However, Mozilla still attempts to access the local IMAP server over SOCKS5. The server name is entered fully qualified in my account settings. The Ethereal trace shows that Mozilla asks the SOCKS server for IP address 129.148.9.54. The SOCKS server rejects this request since it's a local address.
I am very disappointed that the 192.168.* notation will not be allowed. If you are trying to attract PC users, surely it makes sense to use a form which is used on arguably the world's most used browser rather than force a Unix style notation which is not intuitive and not widely known about outside the Unix community.
Disagree. Mozilla IE 10.0/16 == 10.0.* 10.0.0/18 == ???? Mozilla shouldn't support inferior, less capable syntax just because IE uses it. A little explanation on a relevant preferences page will clear user confusion.
If you disagree, why not support the more conventional address/net mask notation rather than the abbreviated one eg 192.168.* == 192.168/16 == 192.168.0.0/255.255.0.0 10.0.0/18 == 10.0.0.0/255.255.192.0 (I guess this is what 10.0.0/18 means) Are you willing to explain to the average Joe Soap who has no understanding of Binary/Octal/Hex how to convert his basic Windows style masks? OK, the windows style may be less functional, but for windows users it does the job and is intuitive. Neither form of net masking is intuitive and (imo) the the least intuitive is the 10.0.0/18 form. Why not allow the * format? It can easily be translated into a mask format by a small routine without the user needing to know anything about masking.
There is no particular reason why Mozilla cannot handle multiple syntaxes, given the alternatives can be unambiguously parsed. Supporting addititonal syntaxes is, however, another bug.
*** Bug 113988 has been marked as a duplicate of this bug. ***
> There is no particular reason why Mozilla cannot handle multiple syntaxes... My reason: code bloat. Dont do it.
Code bloat vs. intuitive usability If that is the argument and you want to make the browser popular, go for the intuitive option i.e. the * and remove all sub-netting options. The * option is the easiest to program so takes less code and is the most intuitive. The expense is function which millions of MSIE users will not miss.
This is not the forum to be having this discussion. As jgmyers said, please move this to another bug. I believe there is value in implementing '*' and Vlasenko, with all due respect, is not a Mozilla networking peer, and therefore does not have any final say in this matter. Please do not fill this bug, which has been resolved, with discussions that belong elsewhere. Thank you.
V/fixed: Finally had time to test this. IP addresses, IP ranges w/ netmasks, port numbers work. Testcases and docs are coming up.
Status: RESOLVED → VERIFIED
Whiteboard: checklinux, checkwin
shouldn't this feature be documented somewhere, like in the Example under No Proxy For in mozilla & firefox?
I think that is bug 222127.
Blocks: 154784
Blocks: 238262
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: