1476899 - Web Proxy auto-detect doesn't check DHCP option 252 on Mac OS X

Status: NEW

(Core :: Networking: Proxy, enhancement, P5)

Description

[polly.shaw]

When Firefox is used on a Mac in a network where the location of the proxy PAC script is broadcast using DHCP Option 252 and Firefox is set to auto-detect the proxy, Firefox fails to detect the proxy.

Comment 1

[polly.shaw]

I'm hoping to fix this by implementing an OS X implementation of nsIDHCPClient. Planning to submit a patch this evening.

Comment 2

[polly.shaw]

Attachment: Bug 1476899: Web Proxy auto-detect doesn't check DHCP on Mac OS X.

This patch implements nsIDHCPClient (an interface defined in netwerk/base and
used in the process of proxy auto-detection) on the Mac OS X platform, by making
calls to the Mac OS X platform API as documented here https://developer.apple.com/documentation/systemconfiguration/scdynamicstorecopydhcpinfo?language=objc

It makes the following changes:
* toolkit/moz.build: changed to compile the contents of toolkit/system/osxDHCPClient
when building for OS X.
* toolkit/system/osxDHCPClient/moz.build: added to control the compilation of
the osxDHCPClient directory
* toolkit/system/osxDHCPClient/nsOSXDHCPClient.h: added header file for
implementation of the nsIDHCPClient interface
* toolkit/system/osxDHCPClient/nsOSXDHCPClient.mm: added to implement the
functionality of the interface by calling native OS X methods.

This patch does not contain tests because there is no logic other than calling
the API methods, and therefore unit tests would test nothing, and existing
end-to-end tests will call the auto-detect functionality.

Review commit: https://reviewboard.mozilla.org/r/258236/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/258236/

Comment 3

[polly.shaw]

Comment on attachment 8993539 [details]
Bug 1476899: Web Proxy auto-detect doesn't check DHCP on Mac OS X.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/258236/diff/1-2/

Comment 4

[polly.shaw]

Attachment: Bug 1476899 - Web Proxy auto-detect doesn't check DHCP option 252 on Mac OS

This patch fixes the bug by implementing nsIDHCPClient (an interface defined in
netwerk/base and used in the process of proxy auto-detection) on the Mac OS X
platform, by using methods of the Mac OS X platform API documented here:
https://developer.apple.com/documentation/systemconfiguration/scdynamicstorecopydhcpinfo?language=objc

It makes the following changes:
* toolkit/moz.build: changed to compile the contents of toolkit/system/osxDHCPClient
when building for OS X.
* toolkit/system/osxDHCPClient/moz.build: added to control the compilation of
the osxDHCPClient directory
* toolkit/system/osxDHCPClient/nsOSXDHCPClient.h: added header file for
implementation of the nsIDHCPClient interface
* toolkit/system/osxDHCPClient/nsOSXDHCPClient.mm: added to implement the
functionality of the interface by calling native OS X methods.

This patch does not contain tests because there is very little logic other than
calling the API methods, and introducing mocks for the API would risk adding more
untested code than they would remove. Existing end-to-end tests will call the
auto-detect functionality, although admittedly not in an environment which has
the PAC URL broadcast using DHCP.

This work builds on the patch for bug 356831, which was also implemented by this
author.

Review commit: https://reviewboard.mozilla.org/r/258502/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/258502/

Comment 5

[Sylvestre Ledru [:Sylvestre]]

Sorry Polly, it seems that your patch went unnoticed.
Without set reviewer, this can happen.
Are you still interested in landing this change in Firefox?

Comment 6

[polly.shaw]

I’m ambivalent about landing it - it does reduce noise from the original patch in this area but the stack traces suggest that this code hasn’t been reached. So on the whole, no I don’t think it’s worth landing.

Comment 7

[Sylvestre Ledru [:Sylvestre]]

OK, let me close it then. Let me know if you change your mind. Thanks

Comment 8

[polly.shaw]

The bug is still there, but I don’t think my patch would actually fix it. I think there may be some deadlock in the Windows proxy query. If true though i’m not sure why this would be manifesting now. I want to write an app to reproduce this but i’m struggling to find the time.

Comment 9

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee didn't login in Bugzilla in the last 7 months.
:dragana, could you have a look please?
For more information, please visit auto_nag documentation.

Comment 10

[BMO Automation]

Moving bug to Core/Networking: Proxy.