385280 - should send proxy settings to the breakpad reporter

Status: RESOLVED FIXED

(Toolkit :: Crash Reporting, defect)

Description

[Dave Camp (:dcamp)]

The linux reporter needs proxy settings for uploading minidumps.

Comment 2

[Alfred Peng]

Attachment: patch v1

ted, could we approach this proxy issue by adding something like the following line to application.ini: HttpProxy=http://your.proxy.com:18023/? And the patch will pass the proxy info to libcurl.

Comment 3

[Alfred Peng]

Is it more reasonable to get the proxy information from gconf?

Comment 4

[(not currently active) Ted Mielczarek]

Comment on attachment 316605 [details] [diff] [review]
patch v1

Getting the proxy info from gconf would be acceptable, it would match what we're doing on Mac/Windows. If you were going to pass the proxy info from the main application, I would prefer to do it in an environment variable. I'm not sure what you mean about putting the proxy in application.ini, that's not configured per-user.

Comment 6

[Alfred Peng]

Attachment: patch v2

Get the proxy information from gconf.

1. This patch doesn't include the "Ignore Host List".
2. Still working on the lack of certificates on Solaris. Need to push some guy to deliver them. Provide a workaround in this patch. Not sure whether it's acceptable.

Comment 7

[Flávio Henrique]

Someone can tell me if this bug, when fixed, will cover Windows SO too ? or just Linux?

Thank you

Comment 8

[(not currently active) Ted Mielczarek]

The Windows crash reporter already uses the system proxy settings. (whatever you have configured for IE)

Comment 9

[(not currently active) Ted Mielczarek]

Comment on attachment 317974 [details] [diff] [review]
patch v2

r=me on the proxy bits, but r- on the SSL bits. You'll have to fix that on the OS side, disabling SSL auth is not a good solution.

Comment 10

[(not currently active) Ted Mielczarek]

(Also the SSL bits are unrelated to this bug.)

Comment 11

[Alfred Peng]

Attachment: patch v2-2

Remove the r- part from the previous patch and ask for approval1.9. The patch enables sending crash report through http proxy from gconf for Linux/Solaris and should be safe.

Comment 12

[Damon Sicore (:damons)]

Comment on attachment 318195 [details] [diff] [review]
patch v2-2

a1.9+=damons

Comment 13

[Alfred Peng]

Checking in toolkit/crashreporter/client/Makefile.in;
/cvsroot/mozilla/toolkit/crashreporter/client/Makefile.in,v  <--  Makefile.in
new revision: 1.18; previous revision: 1.17
done
Checking in toolkit/crashreporter/client/crashreporter_linux.cpp;
/cvsroot/mozilla/toolkit/crashreporter/client/crashreporter_linux.cpp,v  <--  crashreporter_linux.cpp
new revision: 1.20; previous revision: 1.19
done

Comment 14

[Andrew Schultz]

This introduced a dependency (libgconf and libORBit) which we did not have before and is not listed http://developer.mozilla.org/en/docs/Linux_Build_Prerequisites or http://wiki.mozilla.org/Linux/Runtime_Requirements.  Or http://wiki.mozilla.org/Firefox3/Firefox_Requirements#Linux although that list has far more serious problems.

Comment 15

[(not currently active) Ted Mielczarek]

Andrew: we were already checking and using that in configure.in, afaict, this shouldn't have been anything new.

Comment 16

[Andrew Schultz]

Ah, OK. toolkit/system/gnome also links against libgconf.  But configure.in automagically disables that if libgconf is not found, whereas crashreporter will happily try to compile (and fail).

Apparently people on gnome-less distros (slack, kubuntu, perhaps) have been compiling happily with defaultish options (of course, they wouldn't want crash reporter anyway, but...)

Comment 17

[timeless]

this isn't cool. it means that mozilla.org builds which should have been able to report crashes before will not be able to run the crash reporter on those systems.

this should be reworked as a dynamically loaded library.

Comment 18

[(not currently active) Ted Mielczarek]

timeless: doubtful, since those builds would be linked against gconf anyway, so they won't even start. I think we could fix things to not try to link against gconf if it's not present, but I think that's all we need to do.

Comment 19

[timeless]

please rework this as a dynamic load.

Comment 20

[Alfred Peng]

Attachment: Follow up fix

Ted, can we fix it this way?

Comment 21

[(not currently active) Ted Mielczarek]

Comment on attachment 318979 [details] [diff] [review]
Follow up fix

Almost. You'll need to:
1) add MOZ_ENABLE_GCONF to config/autoconf.mk.in
2) add an AC_DEFINE(MOZ_ENABLE_GCONF) in configure.in (inside an if block testing that it's set)

With those tweaks this should be ok.

Comment 22

[Alfred Peng]

Attachment: follow up fix v2

Comment 23

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 319107 [details] [diff] [review]
follow up fix v2

a1.9=beltzner

Comment 24

[Alfred Peng]

Checking in configure.in;
/cvsroot/mozilla/configure.in,v  <--  configure.in
new revision: 1.1992; previous revision: 1.1991
done
Checking in config/autoconf.mk.in;
/cvsroot/mozilla/config/autoconf.mk.in,v  <--  autoconf.mk.in
new revision: 3.462; previous revision: 3.461
done
Checking in toolkit/crashreporter/client/Makefile.in;
/cvsroot/mozilla/toolkit/crashreporter/client/Makefile.in,v  <--  Makefile.in
new revision: 1.19; previous revision: 1.18
done
Checking in toolkit/crashreporter/client/crashreporter_linux.cpp;
/cvsroot/mozilla/toolkit/crashreporter/client/crashreporter_linux.cpp,v  <--  crashreporter_linux.cpp
new revision: 1.21; previous revision: 1.20
done

Comment 25

[Tomas]

Is it fixed?

Comment 26

[Alfred Peng]

=> FIXED. Sorry for the delay.

Comment 27

[Karl Tomlinson (:karlt)]

(In reply to comment #18)
> timeless: doubtful, since those builds would be linked against gconf anyway,
> so they won't even start.

They do start as the Gnome dependency is checked at runtime.

> I think we could fix things to not try to link against
> gconf if it's not present, but I think that's all we need to do.

We can and should do runtime detection (bug 434997).
There will be plenty of people running without a 32-bit libgconf installed.

Comment 28

[Karl Tomlinson (:karlt)]

(In reply to comment #27)
> the Gnome dependency is checked at runtime.

I guess that happens implicitly:

http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/toolkit/system/unixproxy/nsUnixSystemProxySettings.cpp&rev=1.1&mark=75#73
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/uriloader/exthandler/unix/nsGNOMERegistry.cpp&rev=1.19&mark=56#54
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/browser/components/shell/src/nsGNOMEShellService.cpp&rev=1.22&mark=112#105

Comment 29

[(not currently active) Ted Mielczarek]

Sorry, that's my fault, I should have verified my assertions.

Comment 30

[(not currently active) Ted Mielczarek]

BTW, I'm not sure that Breakpad always works on a 64-bit system running a 32-bit binary anyway.

Comment 31

[Josh Triplett]

For anyone looking for current information on why crash reports don't work through a proxy, see bug 1333125.