Closed Bug 1258977 Opened 4 years ago Closed 3 years ago

Wrong interpretation of system Proxy exceptions

Categories

(Core :: Networking: HTTP, defect)

45 Branch
All
Windows
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla49
Tracking Status
firefox49 --- fixed

People

(Reporter: paroulek, Assigned: xeonchen)

References

Details

(Whiteboard: [proxy][necko-active])

Attachments

(4 files)

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:45.0) Gecko/20100101 Firefox/45.0
Build ID: 20160315153207

Steps to reproduce:

In Control panel - Internet settings use proxy and have this exceptions: 192.168.*;10.101.*;10.102.*;10.103.*;10.104.*;10.105.*;10.106.*;10.108.*;172.17.*;*.domena.cz;helpdesk.customercare.cz

In FireFox is "use system proxy settings".


Actual results:

When i try to open website www.jinadomena.cz then FireFox don't use proxy to connect (wrong).

If I copy the settings to the FireFox and select to use FireFox proxy settings, then web site www.jinadomena.cz is opened trought proxy (correct).


Expected results:

FireFox will open www.jinadomena.cz trought proxy when the same settings is use in system or in FireFox.
Component: Untriaged → Networking: HTTP
Product: Firefox → Core
Whiteboard: [proxy][necko-backlog]
I cannot reproduce this issue on:
User Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:46.0) Gecko/20100101 Firefox/46.0

Hi paroulek,
I'm sorry that I don't have a Windows 7 platform now,
would you try different versions of Firefox or Windows?
Flags: needinfo?(paroulek)
OS: Unspecified → Windows
Hardware: Unspecified → All
(In reply to Gary Chen [:xeonchen] from comment #1)
> I cannot reproduce this issue on:
> User Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:46.0) Gecko/20100101
> Firefox/46.0
> 
> Hi paroulek,
> I'm sorry that I don't have a Windows 7 platform now,
> would you try different versions of Firefox or Windows?

I try it on several computers with Windows 7 x64, Windows 7 (32b) a Windows XP (32b). Last version of FF 46.0.1 do the same.

Exact data of proxy exclusion is: 192.168.*;10.101.*;10.102.*;10.103.*;10.104.*;10.105.*;10.106.*;10.108.*;172.17.*;*.security.cz;helpdesk.net4gas.cz
and I try to open website www.adsecurity.cz
Hi paroulek, I still cannot reproduce this issue on Win 7 x64, my setting is attached.

And it will use proxy to connect if I remove *.proxy.com from the exception list.
(In reply to Gary Chen [:xeonchen] from comment #3)
> Created attachment 8755908 [details]
> Screenshot 2016-05-24 23.50.38.png
> 
> Hi paroulek, I still cannot reproduce this issue on Win 7 x64, my setting is
> attached.
> 
> And it will use proxy to connect if I remove *.proxy.com from the exception
> list.

Please, just set the settings exactly as I write. 
SYSTEM EXCEPTIONS:
192.168.*;10.101.*;10.102.*;10.103.*;10.104.*;10.105.*;10.106.*;10.108.*;172.17.*;*.security.cz;helpdesk.net4gas.cz

and open website www.adsecurity.cz
FireForx try to open website by direct connection, not over proxy as I expected.
(In reply to paroulek from comment #4)
> Please, just set the settings exactly as I write. 
> SYSTEM EXCEPTIONS:
> 192.168.*;10.101.*;10.102.*;10.103.*;10.104.*;10.105.*;10.106.*;10.108.*;172.
> 17.*;*.security.cz;helpdesk.net4gas.cz
> 
> and open website www.adsecurity.cz
> FireForx try to open website by direct connection, not over proxy as I
> expected.

I'm sorry that I misunderstood.
Now I know what happened, thank you!
Assignee: nobody → xeonchen
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Flags: needinfo?(paroulek)
Whiteboard: [proxy][necko-backlog] → [proxy][necko-active]
See Also: → 1275849
Attachment #8756786 - Flags: review?(daniel) → review+
Comment on attachment 8756786 [details]
MozReview Request: Bug 1258977 - Part 1: separate match function; r=bagder

https://reviewboard.mozilla.org/r/55406/#review52832

Apart from the little nits, looks good.

::: toolkit/system/windowsproxy/ProxyUtils.cpp:28
(Diff revision 1)
> +    if (tokenEnd == tokenStart) {
> +      star = true;
> +      tokenStart++;
> +      // If the character following the '*' is a '.' character then skip
> +      // it so that "*.foo.com" allows "foo.com".
> +      if (override.FindChar('.', tokenStart) == tokenStart)

add {} braces here! I realize you only moved the code here, but when you're at it you could just as well make it follow our code style better I think.

::: toolkit/system/windowsproxy/ProxyUtils.cpp:31
(Diff revision 1)
> +      // If the character following the '*' is a '.' character then skip
> +      // it so that "*.foo.com" allows "foo.com".
> +      if (override.FindChar('.', tokenStart) == tokenStart)
> +        tokenStart++;
> +    } else {
> +      if (tokenEnd == -1)

... and here

::: toolkit/system/windowsproxy/ProxyUtils.cpp:36
(Diff revision 1)
> +      if (tokenEnd == -1)
> +        tokenEnd = overrideLength;
> +      nsAutoCString token(Substring(override, tokenStart,
> +                                    tokenEnd - tokenStart));
> +      offset = host.Find(token, offset);
> +      if (offset == -1 || (!star && offset))

and here
Comment on attachment 8756787 [details]
MozReview Request: Bug 1258977 - Part 2: add proxy exception tests; r=bagder

https://reviewboard.mozilla.org/r/55408/#review52834
Attachment #8756787 - Flags: review?(daniel) → review+
Attachment #8756788 - Flags: review?(daniel) → review+
Comment on attachment 8756788 [details]
MozReview Request: Bug 1258977 - Part 3: handle special case to follow the test; r=bagder

https://reviewboard.mozilla.org/r/55410/#review52836

::: toolkit/system/windowsproxy/ProxyUtils.cpp:52
(Diff revision 1)
> +}
> +
> +static void
> +MaskIPv6Addr(PRIPv6Addr& aAddr, uint16_t aMaskLen)
> +{
> +  if (aMaskLen == 128)

{braces!}

::: toolkit/system/windowsproxy/ProxyUtils.cpp:156
(Diff revision 1)
> +        if (host.Equals(token)) {
> +          return true;
> +        }
> +      }
>      } else {
>        if (tokenEnd == -1)

add braces here while you're fixing this function!
Comment on attachment 8756786 [details]
MozReview Request: Bug 1258977 - Part 1: separate match function; r=bagder

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/55406/diff/1-2/
Attachment #8756786 - Attachment description: MozReview Request: Bug 1258977 - Part 1: separate match function; r?bagder → MozReview Request: Bug 1258977 - Part 1: separate match function; r=bagder
Attachment #8756787 - Attachment description: MozReview Request: Bug 1258977 - Part 2: add proxy exception tests; r?bagder → MozReview Request: Bug 1258977 - Part 2: add proxy exception tests; r=bagder
Attachment #8756788 - Attachment description: MozReview Request: Bug 1258977 - Part 3: handle special case to follow the test; r?bagder → MozReview Request: Bug 1258977 - Part 3: handle special case to follow the test; r=bagder
Comment on attachment 8756787 [details]
MozReview Request: Bug 1258977 - Part 2: add proxy exception tests; r=bagder

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/55408/diff/1-2/
Comment on attachment 8756788 [details]
MozReview Request: Bug 1258977 - Part 3: handle special case to follow the test; r=bagder

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/55410/diff/1-2/
Comment on attachment 8756786 [details]
MozReview Request: Bug 1258977 - Part 1: separate match function; r=bagder

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/55406/diff/2-3/
Comment on attachment 8756787 [details]
MozReview Request: Bug 1258977 - Part 2: add proxy exception tests; r=bagder

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/55408/diff/2-3/
Comment on attachment 8756788 [details]
MozReview Request: Bug 1258977 - Part 3: handle special case to follow the test; r=bagder

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/55410/diff/2-3/
Pushed by gachen@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d264fcc9f04a
Part 1: separate match function; r=bagder
https://hg.mozilla.org/integration/mozilla-inbound/rev/fd981e994ec9
Part 2: add proxy exception tests; r=bagder
https://hg.mozilla.org/integration/mozilla-inbound/rev/285bc19d2b16
Part 3: handle special case to follow the test; r=bagder
https://hg.mozilla.org/mozilla-central/rev/d264fcc9f04a
https://hg.mozilla.org/mozilla-central/rev/fd981e994ec9
https://hg.mozilla.org/mozilla-central/rev/285bc19d2b16
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
Duplicate of this bug: 1283227
You need to log in before you can comment on or make changes to this bug.