Closed
Bug 385280
Opened 18 years ago
Closed 17 years ago
should send proxy settings to the breakpad reporter
Categories
(Toolkit :: Crash Reporting, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: dcamp, Assigned: alfred.peng)
References
Details
Attachments
(3 files, 2 obsolete files)
(deleted),
patch
|
ted
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
damons
:
approval1.9+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
ted
:
review+
beltzner
:
approval1.9+
|
Details | Diff | Splinter Review |
The linux reporter needs proxy settings for uploading minidumps.
Assignee | ||
Comment 2•17 years ago
|
||
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.
Attachment #316605 -
Flags: review?(ted.mielczarek)
Assignee | ||
Comment 3•17 years ago
|
||
Is it more reasonable to get the proxy information from gconf?
Comment 4•17 years ago
|
||
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.
Attachment #316605 -
Flags: review?(ted.mielczarek) → review-
Assignee | ||
Comment 6•17 years ago
|
||
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.
Attachment #316605 -
Attachment is obsolete: true
Attachment #317974 -
Flags: review?(ted.mielczarek)
Comment 7•17 years ago
|
||
Someone can tell me if this bug, when fixed, will cover Windows SO too ? or just Linux?
Thank you
Comment 8•17 years ago
|
||
The Windows crash reporter already uses the system proxy settings. (whatever you have configured for IE)
Comment 9•17 years ago
|
||
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.
Attachment #317974 -
Flags: review?(ted.mielczarek) → review+
Comment 10•17 years ago
|
||
(Also the SSL bits are unrelated to this bug.)
Assignee | ||
Comment 11•17 years ago
|
||
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•17 years ago
|
||
Comment on attachment 318195 [details] [diff] [review]
patch v2-2
a1.9+=damons
Attachment #318195 -
Flags: approval1.9? → approval1.9+
Assignee | ||
Comment 13•17 years ago
|
||
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
Assignee | ||
Updated•17 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Comment 14•17 years ago
|
||
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•17 years ago
|
||
Andrew: we were already checking and using that in configure.in, afaict, this shouldn't have been anything new.
Comment 16•17 years ago
|
||
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•17 years ago
|
||
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•17 years ago
|
||
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•17 years ago
|
||
please rework this as a dynamic load.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Comment 20•17 years ago
|
||
Ted, can we fix it this way?
Comment 21•17 years ago
|
||
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.
Attachment #318979 -
Flags: review-
Assignee | ||
Comment 22•17 years ago
|
||
Attachment #318979 -
Attachment is obsolete: true
Attachment #319107 -
Flags: review?(ted.mielczarek)
Updated•17 years ago
|
Attachment #319107 -
Flags: review?(ted.mielczarek) → review+
Assignee | ||
Updated•17 years ago
|
Attachment #319107 -
Flags: approval1.9?
Comment 23•17 years ago
|
||
Comment on attachment 319107 [details] [diff] [review]
follow up fix v2
a1.9=beltzner
Attachment #319107 -
Flags: approval1.9? → approval1.9+
Assignee | ||
Comment 24•17 years ago
|
||
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•17 years ago
|
||
Is it fixed?
Assignee | ||
Comment 26•17 years ago
|
||
=> FIXED. Sorry for the delay.
Status: REOPENED → RESOLVED
Closed: 17 years ago → 17 years ago
Resolution: --- → FIXED
Comment 27•17 years ago
|
||
(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.
Depends on: 434997
Comment 28•17 years ago
|
||
(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•17 years ago
|
||
Sorry, that's my fault, I should have verified my assertions.
Comment 30•17 years ago
|
||
BTW, I'm not sure that Breakpad always works on a 64-bit system running a 32-bit binary anyway.
Comment 31•7 years ago
|
||
For anyone looking for current information on why crash reports don't work through a proxy, see bug 1333125.
You need to log in
before you can comment on or make changes to this bug.
Description
•