Closed
Bug 412610
Opened 17 years ago
Closed 3 years ago
MAXPATHLEN too small for glibc's realpath()
Categories
(Toolkit :: Startup and Profile System, defect)
Tracking
()
RESOLVED
FIXED
mozilla1.9.2a1
Tracking | Status | |
---|---|---|
status1.9.1 | --- | ? |
People
(Reporter: wolfiR, Unassigned)
References
Details
(Keywords: fixed1.9.0.12)
Attachments
(6 files, 9 obsolete files)
5.95 KB,
patch
|
Details | Diff | Splinter Review | |
7.50 KB,
patch
|
benjamin
:
review-
|
Details | Diff | Splinter Review |
2.47 KB,
patch
|
dveditz
:
approval1.9.0.12+
|
Details | Diff | Splinter Review |
2.99 KB,
patch
|
Details | Diff | Splinter Review | |
5.19 KB,
patch
|
Details | Diff | Splinter Review | |
5.21 KB,
patch
|
Details | Diff | Splinter Review |
(that also applies to 1.8)
With recent gcc and glibc versions (and certain compile options (_FORTIFY_SOURCE))
running Firefox aborts immediately because of:
#3 0x00007fff0c373397 in *__GI___fortify_fail (
msg=0x7fff0c3abd2f "buffer overflow detected") at fortify_fail.c:32
No locals.
#4 0x00007fff0c3717a0 in *__GI___chk_fail () at chk_fail.c:29
No locals.
#5 0x00007fff0c371ebb in __realpath_chk (
buf=0x71d1 <Address 0x71d1 out of bounds>,
resolved=0x71d1 <Address 0x71d1 out of bounds>, resolvedlen=6)
at realpath_chk.c:30
No locals.
#6 0x0000000000448909 in XRE_GetBinaryPath (
argv0=0x71d1 <Address 0x71d1 out of bounds>, aResult=0x7fff1a234a00)
That's in http://mxr.mozilla.org/seamonkey/source/toolkit/xre/nsAppRunner.cpp#1435
It seems MAXPATHLEN is 1024 here while glibc is using 4096 (AFAIK).
MAXPATHLEN in
http://mxr.mozilla.org/mozilla1.8/source/toolkit/xre/nsAppRunner.h#45
should use PATH_MAX as in
http://mxr.mozilla.org/mozilla1.8/source/xpcom/build/nsXPCOMPrivate.h#247
Reporter | ||
Comment 1•17 years ago
|
||
I'm actually checking locally if that _really_ is the issue and will attach a patch then.
Comment 2•17 years ago
|
||
Yes, that's exactly the issue confirmed with jakub jelinek.
Comment 3•17 years ago
|
||
I thought stransky said he attached this to a bug before.... I can't find it though :(
Reporter | ||
Comment 4•17 years ago
|
||
Comment on attachment 297358 [details] [diff] [review]
Patch from Martin Stransky
>diff -up mozilla/nsprpub/config/pathsub.h.old mozilla/nsprpub/config/pathsub.h
>--- mozilla/nsprpub/config/pathsub.h.old 2004-04-25 17:00:34.000000000 +0200
>+++ mozilla/nsprpub/config/pathsub.h 2007-09-25 18:57:51.000000000 +0200
>@@ -50,7 +50,7 @@
> #endif
>
> #ifndef PATH_MAX
>-#define PATH_MAX 1024
>+#error "PATH_MAX is not defined!"
> #endif
Is PATH_MAX available on all platforms we need to support (thinking about NSPR and NSS)?
So is it safe to remove that default?
Comment 5•17 years ago
|
||
I have run into the same issue with realpath().
I don't know whether PATH_MAX available on all platforms that Firefox needs to support, but the value 1024 is a very bad choice for default. Accordingly man for realpath(), the size of the buffer for it is defined as:
#ifdef PATH_MAX
path_max = PATH_MAX;
#else
path_max = pathconf (path, _PC_PATH_MAX);
if (path_max <= 0)
path_max = 4096;
#endif
So, 4096 would be much better value.
Besides, the definition of MAXPATHLEN is inconsistent. In nsXPCOMPrivate.h,
it uses PATH_MAX when it is available
http://mxr.mozilla.org/mozilla1.8/source/xpcom/build/nsXPCOMPrivate.h#248
but, in many other places, it does not...
IMHO, it would be better if MAXPATHLEN was defined as PATH_MAX when the latter is available... At least, this closes the problem for those platforms that have PATH_MAX defined.
Comment 6•17 years ago
|
||
This patch solves the problem for those platforms that have PATH_MAX defined. It does not change anything for platforms that do not have PATH_MAX. The patch makes the definition of MAXPATHLEN the same as it is defined in nsXPCOMPrivate.h.
Comment on attachment 309860 [details] [diff] [review]
define MAXPATHLEN as PATH_MAX when PATH_MAX is available
Index: browser/components/migration/src/nsProfileMigrator.cpp
+#ifdef PATH_MAX
+#define MAXPATHLEN PATH_MAX
...
+#elif defined(PATH_MAX)
+#define MAXPATHLEN PATH_MAX
this is wrong :)
Comment 8•17 years ago
|
||
I am sorry for my mistake... I started adding definition of MAXPATHLEN as PATH_LEN after all others, but then saw that nsXPCOMPrivate.h does this in the different order. So, I decided to add "#define MAXPATHLEN PATH_MAX" at the beginning as in nsXPCOMPrivate.h but forgot remove the old lines. Here is a corrected version of that patch.
Attachment #309860 -
Attachment is obsolete: true
Comment on attachment 309958 [details] [diff] [review]
define MAXPATHLEN as PATH_MAX when PATH_MAX is available
well, someone should review this, and it won't be me.
Attachment #309958 -
Flags: review?(shaver)
Comment 11•16 years ago
|
||
On mandriva, here is a patch that seems to work for firefox 3b1:
http://svn.mandriva.com/cgi-bin/viewvc.cgi/packages/cooker/mozilla-firefox/branches/firefox3/current/SOURCES/firefox-3.0rc1-glibc28-max_path-fix.patch?view=markup&pathrev=212641
Comment 12•16 years ago
|
||
Comment on attachment 309958 [details] [diff] [review]
define MAXPATHLEN as PATH_MAX when PATH_MAX is available
I'd rather have Benjamin look at this, if possible -- I'm not au courant on the portability of things like limits.h
Attachment #309958 -
Flags: review?(shaver) → review?(benjamin)
Updated•16 years ago
|
Attachment #309958 -
Flags: review?(benjamin) → review+
Comment 13•16 years ago
|
||
I found the patch to fix this in SRPM package from Fedora.
Here is the patch to fit to the latest trunk (be010ac940e0).
Attachment #325241 -
Flags: review+
Updated•16 years ago
|
Attachment #325241 -
Flags: review+ → review?
Updated•16 years ago
|
Attachment #325241 -
Flags: review?
Updated•16 years ago
|
Attachment #325241 -
Flags: review?(benjamin)
Comment 14•16 years ago
|
||
Comment on attachment 325241 [details] [diff] [review]
Fix based on the patch used in Fedora
How is this patch better than the first patch in this bug? In any case, the comment about "works for SCO" is no longer relevant with this patch.
Attachment #325241 -
Flags: review?(benjamin) → review-
Comment 15•16 years ago
|
||
Oh, attachment 325241 [details] [diff] [review] fixes bug 421889.
I don't know which is the better fix, attachment 309958 [details] [diff] [review] or attachment 325241 [details] [diff] [review].
I only introduce how Fedora fixes this bug and hope to fix this.
Fedora 9 (glibc-2.8 and gcc-4.3.0) on x86_64 requires the attachment 325441 [details] [diff] [review] for getting well-worked firefox with the compiler option of -DFORTIFY_SOURCE=2.
Comment 16•16 years ago
|
||
bug 421889 comment #7 says bug 421889 is the dup. of this bug.
Component: XRE Startup → Startup and Profile System
QA Contact: xre.startup → startup
Comment 17•16 years ago
|
||
This is exactly the same as attachment #309958 [details] [diff] [review] but updated to work with mercurial (queues).
The (old) patch has r+ and works fine, someone should get it pushed.
Attachment #309958 -
Attachment is obsolete: true
Comment 19•16 years ago
|
||
I duped bug #421889 against this one.
Maybe someone can create a followup bug to use safer functions as suggested in
bug #421889 comment #7.
Comment 20•16 years ago
|
||
This ruined most of my day on a new ubuntu intrepid (beta) 64 bit build system. One of the fixes should be propagated.
in my case MAXPATHLEN is already defined in sys/param.h - I cannot think of a reason unix platforms should not all use that.
iff -r a95a069f3372 xpcom/build/nsXPCOMPrivate.h
--- a/xpcom/build/nsXPCOMPrivate.h Mon Sep 29 14:28:15 2008 -0400
+++ b/xpcom/build/nsXPCOMPrivate.h Mon Sep 29 18:04:29 2008 -0400
@@ -260,7 +260,7 @@
#error need_to_define_your_file_path_separator_and_illegal_characters
#endif
-#ifdef AIX
+#if defined (XP_UNIX)
#include <sys/param.h>
#endif
Comment 21•16 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/adf33dbb59d0
Thanks for the patch!
(In reply to comment #20)
> in my case MAXPATHLEN is already defined in sys/param.h - I cannot think of a
> reason unix platforms should not all use that.
Want to file another bug on changing this, please? Thanks!
Assignee: nobody → dpotapov
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.1b2
Comment 22•16 years ago
|
||
I built Spicebird for Fedora 10 and encountered this crash even with the fix checked in above. A quick grep indicated that some cases may not have been covered by the checked-in patch (although some of the patches in this bug seem to touch upon them). So, I made a patch a long the lines of the checked in patch that covers others cases also (esp. the one in sqlite module).
The stack trace for the crash I encountered in f10 is as follows:
#0 0x0000003281632ed5 in raise () from /lib64/libc.so.6
#1 0x0000003281634a43 in abort () from /lib64/libc.so.6
#2 0x0000003281672408 in __libc_message () from /lib64/libc.so.6
#3 0x00000032816ff497 in __fortify_fail () from /lib64/libc.so.6
#4 0x00000032816fd340 in __chk_fail () from /lib64/libc.so.6
#5 0x00000032816fd9fb in __realpath_chk () from /lib64/libc.so.6
#6 0x000000000043d8a6 in sqlite3_bind_null ()
#7 0x000000000043cc5f in sqlite3_bind_null ()
#8 0x000000328161e546 in __libc_start_main () from /lib64/libc.so.6
#9 0x000000000043cac9 in sqlite3_bind_null ()
#10 0x00007fffffffe0a8 in ?? ()
#11 0x000000000000001c in ?? ()
#12 0x0000000000000001 in ?? ()
#13 0x00007fffffffe3a3 in ?? ()
#14 0x0000000000000000 in ?? ()
Attachment #355004 -
Flags: review?
Updated•16 years ago
|
Attachment #355004 -
Flags: review? → review?(benjamin)
Comment 23•16 years ago
|
||
Similar patch for mail/components/migration and mailnews/import.
Attachment #355007 -
Flags: review?
Comment 24•16 years ago
|
||
I just recollect that the crash occurred without the fix that has been checked in. So, I can't say for sure that the changes in the above 2 patches are required. However, the stack trace seems to suggests that.
Updated•16 years ago
|
Attachment #355004 -
Flags: review?(benjamin) → review-
Comment 25•16 years ago
|
||
Comment on attachment 355004 [details] [diff] [review]
Patch covering more cases on the lines of the one checked in
>Index: db/sqlite3/src/sqlite3.c
The sqlite3 code is imported from SQLite without modification. Please file and fix this upstream, and we'll pick it up at the next release update.
>Index: modules/lcms/include/lcms.h
LCMS is also external code.
I'm technically not a JS peer, but I think I can review this patch for jsfile.cpp given the circumstances and content.
Comment 26•16 years ago
|
||
(In reply to comment #25)
> >Index: modules/lcms/include/lcms.h
>
> LCMS is also external code.
>
> I'm technically not a JS peer, but I think I can review this patch for
> jsfile.cpp given the circumstances and content.
Now LCMS used by mozilla is modified by Bobby in many parts and we cannot use external one for building mozilla. See bug 469558.
If this fix for lcms.h is required, we need to make a patch which include only lcms.h part and request review to Bobby.
Comment 27•16 years ago
|
||
Thank you for your comments.
> The sqlite3 code is imported from SQLite without modification. Please file and
> fix this upstream, and we'll pick it up at the next release update.
I commented on an existing bug here:
http://www.sqlite.org/cvstrac/tktview?tn=3373
> I'm technically not a JS peer, but I think I can review this patch for
> jsfile.cpp given the circumstances and content.
I am splitting the patch into individual modules and uploading. I am asking for your review on the jsfile.cpp. Sorry about grouped patch earlier.
Attachment #355004 -
Attachment is obsolete: true
Attachment #355537 -
Flags: review?(benjamin)
Updated•16 years ago
|
Attachment #355007 -
Flags: review? → review?(dmose)
Comment 28•16 years ago
|
||
I can't confirm that the patch is absolutely required because I don't have crashes and stack traces for all the changes.
I could not find Bobby on module owners page to ask for review.
Updated•16 years ago
|
Attachment #355538 -
Flags: review?(bholley)
Comment 29•16 years ago
|
||
Attachment #355539 -
Flags: review?
Comment 30•16 years ago
|
||
Attachment #355541 -
Flags: review?
Comment 31•16 years ago
|
||
Attachment #355542 -
Flags: review?(benjamin)
Updated•16 years ago
|
Attachment #355537 -
Flags: review?(benjamin) → review+
Updated•16 years ago
|
Assignee: dpotapov → sunil
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Updated•16 years ago
|
Attachment #355542 -
Flags: review?(benjamin) → review+
Updated•16 years ago
|
Attachment #355541 -
Flags: review? → review?(roc)
Updated•16 years ago
|
Attachment #355539 -
Flags: review? → review?(dveditz)
Attachment #355541 -
Flags: review?(roc) → review+
Comment 32•16 years ago
|
||
Comment on attachment 355538 [details] [diff] [review]
lcms part of the earlier patch
looks good. r=bholley on the lcms part.
Attachment #355538 -
Flags: review?(bholley) → review+
Comment 33•16 years ago
|
||
Comment on attachment 355007 [details] [diff] [review]
Patch for mail/migration and mailnew/import on the lines of browser/migration
r+sr=dmose; thanks for the patch
Attachment #355007 -
Flags: superreview+
Attachment #355007 -
Flags: review?(dmose)
Attachment #355007 -
Flags: review+
Comment 34•16 years ago
|
||
I could not find anyone to review the modules/libreg part of the patch, can someone suggest a reviewer? Also, I guess superreview is need for last four patches. Benjamin, can I trouble you for that, since you looked at the some patches already?
Updated•16 years ago
|
Attachment #355539 -
Flags: review?(dveditz) → review+
Comment 35•16 years ago
|
||
general sr=me
Updated•16 years ago
|
Keywords: checkin-needed
Comment 36•16 years ago
|
||
Please, flag as obsolete / (visibly) mark as checked in / etc, then state which patch(s) to c-n.
Comment 37•16 years ago
|
||
Checked in:
==========
attachment 335369 [details] [diff] [review]
Check in needed:
===============
attachment 355007 [details] [diff] [review]
attachment 355537 [details] [diff] [review]
attachment 355538 [details] [diff] [review]
attachment 355539 [details] [diff] [review]
attachment 355541 [details] [diff] [review]
attachment 355542 [details] [diff] [review]
Summary of the bug so far:
=========================
One chunk in sqlite3 needs fixing in upstream, later it could be imported into Mozilla code. http://www.sqlite.org/cvstrac/tktview?tn=3373
Changes that were suggested in the initial patch (attachment 297358 [details] [diff] [review]) and are not checked-in are in following files:
config/pathsub.h
nsprpub/config/pathsub.h
security/coreconf/nsinstall/pathsub.h
xpcom/obsolete/nsFileSpecUnix.cpp
dbm/include/mcom_db.h
xpcom/typelib/xpidl/xpidl_java.c
modules/libjar/nsZipArchive.cpp
Comment 38•16 years ago
|
||
without those patches, xulrunner/firefox-3.0.6 and firefox-2.20 fails to launch due to this on Ubuntu Intreip, which is really bad once many products rely on these versions of firefox.
flagging for request 3.0.7 blocker at least.
Flags: blocking1.9.0.7?
Updated•16 years ago
|
Attachment #335369 -
Attachment description: updated patch to work with mercurial queues → updated patch to work with mercurial queues
[Checkin: Comment 21]
Comment 39•16 years ago
|
||
(In reply to comment #37)
Could you attach 1 m-c and 1 c-c Hg diffs, with detailed comments ?
> attachment 355007 [details] [diff] [review]
Why is this one already marked 'checked-in' ?
Comment 40•16 years ago
|
||
Comment on attachment 355007 [details] [diff] [review]
Patch for mail/migration and mailnew/import on the lines of browser/migration
> > attachment 355007 [details] [diff] [review] [details]
> Why is this one already marked 'checked-in' ?
I didn't know checked-in keyword is used to mark already checked-in patches. I used it to mean "... similar to a patch that is already checked-in". I removed it now.
Attachment #355007 -
Attachment description: Patch for mail/migration and mailnew/import on the lines of browser/migration checked-in → Patch for mail/migration and mailnew/import on the lines of browser/migration
Comment 41•16 years ago
|
||
This patch is hg diff created from attachment 355007 [details] [diff] [review] (already reviewed, checkin ready). It contains fixes for the following modules:
mail/migration
mailnews/import
Attachment #355007 -
Attachment is obsolete: true
Comment 42•16 years ago
|
||
This patch is a moz-central hg diff of attachments 355537, 355538, 355539, 355541 and 355542. All these patches have been reviewed and are ready for check in. They fix the problems in following modules:
js/
modules/lcms/
modules/libreg/
widget/src/
xpcom/io/
Attachment #355537 -
Attachment is obsolete: true
Attachment #355538 -
Attachment is obsolete: true
Attachment #355539 -
Attachment is obsolete: true
Attachment #355541 -
Attachment is obsolete: true
Attachment #355542 -
Attachment is obsolete: true
Comment 43•16 years ago
|
||
Not blocking a 1.9.0.x release. Request branch approval on a patch after it has landed and been tested on the trunk (we don't have enough nightly testers on the 1.9.0 branch to safely take untested non-critical patches).
Flags: blocking1.9.0.7?
Comment 44•16 years ago
|
||
Do the two patches need to land simultaneously ?
Comment 45•16 years ago
|
||
(In reply to comment #44)
> Do the two patches need to land simultaneously ?
No, they are independent of one another.
Comment 46•16 years ago
|
||
Comment on attachment 363289 [details] [diff] [review]
Moz-central patch combining attachments (355537 355538 355539 355541 355542)
[Checkin: Comment 46]
http://hg.mozilla.org/mozilla-central/rev/edb1bcc890c0
Attachment #363289 -
Attachment description: Moz-central patch combining attachments (355537 355538 355539 355541 355542) → Moz-central patch combining attachments (355537 355538 355539 355541 355542)
[Checkin: Comment 46]
Comment 47•16 years ago
|
||
Comment on attachment 363288 [details] [diff] [review]
Comm-central patch of attachment 355007 [details] [diff] [review] for fixes in mail/migration and mailnews/import
[Checkin: Comment 47]
http://hg.mozilla.org/comm-central/rev/d7831ab9ae38
Attachment #363288 -
Attachment description: Comm-central patch of attachment 355007 for fixes in mail/migration and mailnews/import → Comm-central patch of attachment 355007 for fixes in mail/migration and mailnews/import
[Checkin: Comment 47]
Comment 48•16 years ago
|
||
(In reply to comment #37)
> One chunk in sqlite3 needs fixing in upstream, later it could be imported into
> Mozilla code. http://www.sqlite.org/cvstrac/tktview?tn=3373
You may want to file a dependent bug.
> Changes that were suggested in the initial patch (attachment 297358 [details] [diff] [review]) and are
> not checked-in are in following files:
Leaving bug open, as I don't know what you plan to do about these.
Keywords: checkin-needed
Target Milestone: mozilla1.9.1b2 → mozilla1.9.2a1
Comment 49•16 years ago
|
||
Here's a backported patch for 1.9.0.x, based on the mozilla-central patch.
(The posted mozilla-central patch *almost* applies cleanly to 190x -- I just had to retarget the "jsfile.cpp" chunk to "jsfile.c" instead, because that file was renamed on mozilla-central in bug 387935 / changeset bd9c9cbf9ec8.)
Comment 50•16 years ago
|
||
Comment on attachment 374325 [details] [diff] [review]
190x version of "Moz-central patch combining attachments"
The backported 1.9.0.x patch doesn't completely fix the bug there -- I still get an immediate startup crash in an optimized CVS trunk build, with attachment 374325 [details] [diff] [review] applied.
*** buffer overflow detected ***: ./dist/bin/firefox-bin terminated
======= Backtrace: =========
/lib/tls/i686/cmov/libc.so.6(__fortify_fail+0x48)[0xb7357da8]
[snip]
So it looks like more needs to be done to fix this on 1.9.0.x (if we want to fix this on 1.9.0.x)
(For anyone else hitting this bug, you can work around it by using gcc / g++ version 4.2 instead of 4.3)
Attachment #374325 -
Attachment description: 190x patch → Backported 190x patch v1 [doesn't completely fix bug]
http://hg.mozilla.org/users/dbaron_mozilla.com/patches-1.9/raw-file/4d2adcbc307d/maxpathlen-fix has worked for me for a while as a patch backported to 1.9.0.
Comment 52•16 years ago
|
||
(In reply to comment #51)
> http://hg.mozilla.org/users/dbaron_mozilla.com/patches-1.9/raw-file/4d2adcbc307d/maxpathlen-fix
> has worked for me for a while as a patch backported to 1.9.0.
Ah, that's actually exactly attachment 335369 [details] [diff] [review] (but with more contextual lines). I missed that patch in the attachment list, amid all the obsoleted patches surrounding/following it. :)
So it looks like *that* patch landed on mozilla-central a while ago (in comment 21), and the later patch that I backported is extra stuff that was discovered to also need fixing so that other mozilla-based apps will work (per comment 22).
So if I understand correctly, 1.9.0.x needs both attachment 335369 [details] [diff] [review] and attachment 374325 [details] [diff] [review]. (though Firefox itself only needs the first of those two in order to work)
Updated•16 years ago
|
Attachment #374325 -
Attachment description: Backported 190x patch v1 [doesn't completely fix bug] → 190x patch combining attachments (backported)
Updated•16 years ago
|
Attachment #374325 -
Flags: approval1.9.0.12?
Updated•16 years ago
|
Attachment #335369 -
Flags: approval1.9.0.12?
Comment 53•15 years ago
|
||
The 1.9.0 release drivers are confused. Is this or is this not fixed on trunk? Checkins appear to have happened but the bug is in a reopened state.
Has this been fixed in 1.9.1 ? If not why not? If it's not important enough to get into the active branch (I see no approval or blocking requests) why is it important to land on 1.9.0?
Comment 54•15 years ago
|
||
Comment on attachment 374325 [details] [diff] [review]
190x version of "Moz-central patch combining attachments"
(In reply to comment #53)
> Checkins appear to have happened but the bug is in a reopened state.
This was originally closed in comment 21, when the initial patch landed. It was later reopened, as more similar patches were posted to fix this in more places.
The last "new" patch was landed in comment 48, but the bug was left open because additional fixes were possibly still coming.
> Has this been fixed in 1.9.1 ? If not why not? If it's not important enough to
> get into the active branch (I see no approval or blocking requests) why is it
> important to land on 1.9.0?
Actually, it looks like it hasn't! The first patch landed before 1.9.1 split off (in comment 21), so that one made it into 1.9.1, but the later patches did not.
However, I think this is "fixed" on 1.9.1 in the sense that I can start a 1.9.1 build without immediately crashing. So perhaps the first patch is all that was needed there? (and all that would need backporting to 1.9.0.x)
Anyway -- canceling 190x approval request for now, since the patch I requested approval on contains stuff that isn't in 1.9.1. (the second round of patches on this bug)
Attachment #374325 -
Flags: approval1.9.0.12?
Comment 55•15 years ago
|
||
(In reply to comment #54)
> However, I think this is "fixed" on 1.9.1 in the sense that I can start a 1.9.1
> build without immediately crashing. So perhaps the first patch is all that was
> needed there? (and all that would need backporting to 1.9.0.x)
Ah -- yes, the above is correct, based on Comment 52.
So, to sum up:
- The patch in attachment 335369 [details] [diff] [review] fixes this bug *in Firefox*
- That patch is already in both mozilla-central and 1.9.1.
- That patch needs approval1.9.0.x, to fix this bug in Firefox 3.0.x
- (The later patches are for correctness, to fix other places where this bug could hypothetically pop up in mozilla-based software. However, those later patches don't affect Firefox's behavior, and they aren't included in 1.9.1.)
Hence, I'm leaving the approval request open on attachment 335369 [details] [diff] [review].
Comment 56•15 years ago
|
||
Comment on attachment 335369 [details] [diff] [review]
updated patch to work with mercurial queues
[Checkin: Comment 21]
Approved for 1.9.0.12, a=dveditz for release-drivers
Attachment #335369 -
Flags: approval1.9.0.12? → approval1.9.0.12+
Comment 57•15 years ago
|
||
Comment on attachment 335369 [details] [diff] [review]
updated patch to work with mercurial queues
[Checkin: Comment 21]
Landed on CVS trunk for 1.9.0.12:
mozilla/browser/components/migration/src/nsDogbertProfileMigrator.cpp 1.29
mozilla/browser/components/migration/src/nsProfileMigrator.cpp 1.27
mozilla/toolkit/mozapps/update/src/updater/updater.cpp 1.40
mozilla/toolkit/xre/nsAppRunner.h 1.27
mozilla/xpcom/build/nsXPCOMPrivate.h 1.47
Updated•15 years ago
|
Keywords: fixed1.9.0.12
Comment 58•15 years ago
|
||
(In reply to comment #55)
> - (The later patches are for correctness, to fix other places where this bug
> could hypothetically pop up in mozilla-based software. However, those later
> patches don't affect Firefox's behavior, and they aren't included in 1.9.1.)
Daniel: Do we want the later patches (for correctness) on 1.9.1?
Flags: wanted1.9.1.x?
Updated•15 years ago
|
Attachment #374325 -
Attachment description: 190x patch combining attachments (backported) → 190x version of "Moz-central patch combining attachments"
Comment 59•15 years ago
|
||
If the later patches are landed on 1.9.1 please also land the fix in bug 502723 that this bug introduced. Thanks
http://mxr.mozilla.org/mozilla1.9.1/source/toolkit/mozapps/update/src/updater/updater.cpp#123
Comment 60•15 years ago
|
||
(In reply to comment #58)
> Daniel: Do we want the later patches (for correctness) on 1.9.1?
First of all, to be clear -- there's actually only one additional patch that'd be under consideration here, for the branches: attachment 363289 [details] [diff] [review] (and its backported-to-1.9.0 version, attachment 374325 [details] [diff] [review]) All other patches here are either obsolete (from merging into a combined patch), already landed on all branches, or are for comm-central.
In response to Sam's question, I'm not really sure whether attachment 363289 [details] [diff] [review] is branch-appropriate. I haven't run into any crashes caused by its absence, and I'm not entirely sure in what situations it's needed. (Comment 22 suggests that SpiceBird needs it -- maybe Thunderbird as well?)
The patch author & reviewers would probably be better judges / vouchers than I, regarding the appropriateness on the branches.
(In reply to comment #59)
> If the later patches are landed on 1.9.1 please also land the fix in bug 502723
> that this bug introduced
That (minor) issue was actually from attachment 335369 [details] [diff] [review], the patch that's already included in both 1.9.1 and 1.9.0. Hence, I'm requesting branch approval on bug 502723's patch. (any further discussion on that should go on bug 502723)
Updated•15 years ago
|
status1.9.1:
--- → ?
Flags: wanted1.9.1.x?
See Also: → https://launchpad.net/bugs/263014
Comment 61•3 years ago
|
||
The bug assignee didn't login in Bugzilla in the last 7 months.
:mossop, could you have a look please?
For more information, please visit auto_nag documentation.
Assignee: sunil → nobody
Flags: needinfo?(dtownsend)
Comment 62•3 years ago
|
||
Sounds like maybe this is actually resolved?
Flags: needinfo?(dtownsend) → needinfo?(dholbert)
Comment 63•3 years ago
|
||
I think so, yeah.
Status: REOPENED → RESOLVED
Closed: 16 years ago → 3 years ago
Flags: needinfo?(dholbert)
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•