51266 - Selecting "block images" or "block cookies" when you're not on a site doesn't do anything

Status: VERIFIED FIXED

(Core :: Networking: Cookies, defect, P1)

Description

[Henrik Gemal]

If you select "block images" or "block cookies" when you're on a local file or 
"about:" nothing is added to the blocked sites list.

Comment 1

[Stephen P. Morse]

Hmm, I thought that worked but I just checked it and the reporter is correct.  
This falls in my code so I'll take the bug.

Comment 2

[Stephen P. Morse]

This should probably be upgraded because it is an inconsistency.  You can block 
file: cookies by checking "remember this decision" from the cookie nag box.  But 
you can't do so from the menu tasks->privacy->cookie-manager->block-this-site.  
Worse yet, when you do the menu thing you get a message saying that the site is 
blocked whereas in fact it is not.

The fix is very simple and zero risk.  Here's the patch.

*** nsCookie.cpp        2000/08/22 00:39:09     1.86
--- nsCookie.cpp        2000/09/05 12:57:20
***************
*** 2542,2550 ****
    char * imageURLCString = imageURL.ToNewCString();
    char *host = cookie_ParseURL(imageURLCString, GET_HOST_PART);
    Recycle(imageURLCString);
!   if (PL_strlen(host) != 0) {
!     permission_Add(host, permission, type, PR_TRUE);
!   }
  }

  MODULE_PRIVATE time_t
--- 2542,2550 ----
    char * imageURLCString = imageURL.ToNewCString();
    char *host = cookie_ParseURL(imageURLCString, GET_HOST_PART);
    Recycle(imageURLCString);
!   char * hostname = nsnull;
!   StrAllocCopy(hostname, host);
!   permission_Add(hostname, permission, type, PR_TRUE);
  }

  MODULE_PRIVATE time_t

Comment 3

[Daniel Veditz [:dveditz]]

I'm concerned about leaks here: either permission_Add() eventually would have
gotten rid of host and therefore now gets rid of hostname (leaking host), or
host didn't need freeing and now you're leaking the allocated hostname. Also
looks like permission_Add() likes to Recycle() things, while StrAllocCopy()
looks like it needs PR_strfree() instead to match the PR_strdup(). (yes those
are normally equivalent, but there have been lots of experiments with the string
APIs because they account for >30% of our allocated memory).

This is not a PR3 blocker in any case.

Comment 4

[Stephen P. Morse]

Not only did my previous patch leak, but so did another call to permission_Add 
which was already in the code.  Here's my corrected patch which doesn't leak and 
fixes the leak that already existed.

Index: nsCookie.cpp
===================================================================
RCS file: /cvsroot/mozilla/extensions/cookie/nsCookie.cpp,v
retrieving revision 1.86
diff -c -r1.86 nsCookie.cpp
*** nsCookie.cpp	2000/08/22 00:39:09	1.86
--- nsCookie.cpp	2000/09/05 21:36:18
***************
*** 868,873 ****
--- 868,874 ----
        hostname++;
      }
      StrAllocCopy(hostname2, hostname);
+     Recycle(hostname);
      permission_Add(hostname2, permission, type, PR_TRUE);
    }
    if (rememberChecked != permission_GetRememberChecked(type)) {
***************
*** 2529,2537 ****
    char * imageURLCString = imageURL.ToNewCString();
    char *host = cookie_ParseURL(imageURLCString, GET_HOST_PART);
    Recycle(imageURLCString);
!   if (PL_strlen(host) != 0) {
!     permission_Add(host, PR_FALSE, IMAGEPERMISSION, PR_TRUE);
!   }
  }
  
  PUBLIC void
--- 2530,2539 ----
    char * imageURLCString = imageURL.ToNewCString();
    char *host = cookie_ParseURL(imageURLCString, GET_HOST_PART);
    Recycle(imageURLCString);
!   char * hostname = nsnull;
!   StrAllocCopy(hostname, host);
!   Recycle(host);
!   permission_Add(hostname, PR_FALSE, IMAGEPERMISSION, PR_TRUE);
  }
  
  PUBLIC void
***************
*** 2542,2550 ****
    char * imageURLCString = imageURL.ToNewCString();
    char *host = cookie_ParseURL(imageURLCString, GET_HOST_PART);
    Recycle(imageURLCString);
!   if (PL_strlen(host) != 0) {
!     permission_Add(host, permission, type, PR_TRUE);
!   }
  }
  
  MODULE_PRIVATE time_t 
--- 2544,2553 ----
    char * imageURLCString = imageURL.ToNewCString();
    char *host = cookie_ParseURL(imageURLCString, GET_HOST_PART);
    Recycle(imageURLCString);
!   char * hostname = nsnull;
!   StrAllocCopy(hostname, host);
!   Recycle(host);
!   permission_Add(hostname, permission, type, PR_TRUE);
  }
  
  MODULE_PRIVATE time_t 

Comment 5

[Blake Ross]

taking per conversation with morse

Comment 6

[Blake Ross]

fixed.

Comment 7

[Diego]

Recent crash inside cookies code.
linux 2000090821 works fine.
linux 2000090908 crashes.

I'm reporting here as it seems the only check-in between the two nightlies.

Steps to reproduce the crash:
1) go to http://www.zdnet.com/
2) accept the cookie

Program received signal SIGSEGV, Segmentation fault.
chunk_free (ar_ptr=0x40700010, p=0x41e43299) at malloc.c:3049
3049    malloc.c: File o directory inesistente.
#0  chunk_free (ar_ptr=0x40700010, p=0x41e43299) at malloc.c:3049
#1  0x4029bfba in __libc_free (mem=0x41e432a1) at malloc.c:3023
#2  0x40183eb0 in PR_Free () from /root/package/./libnspr4.so
#3  0x400c4d60 in nsMemoryImpl::Free () from /root/package/./libxpcom.so
#4  0x400c5050 in nsMemory::Free () from /root/package/./libxpcom.so
#5  0x41149f6b in NSGetModule () from /root/package/components/libcookie.so
#6  0x4114b1da in NSGetModule () from /root/package/components/libcookie.so
#7  0x4114b6cd in NSGetModule () from /root/package/components/libcookie.so
#8  0x4114b5d5 in NSGetModule () from /root/package/components/libcookie.so
#9  0x411483f2 in NSGetModule () from /root/package/components/libcookie.so
#10 0x4114ea93 in NSGetModule () from /root/package/components/libcookie.so
#11 0x400cfcd1 in XPTC_InvokeByIndex () from /root/package/./libxpcom.so
#12 0x400c2ad1 in nsProxyObject::Post () from /root/package/./libxpcom.so
#13 0x400c29fa in nsProxyObject::Post () from /root/package/./libxpcom.so
#14 0x400c4288 in nsProxyEventObject::CallMethod ()
   from /root/package/./libxpcom.so
#15 0x400cfe74 in XPTC_InvokeByIndex () from /root/package/./libxpcom.so
#16 0x400cff02 in nsXPTCStubBase::Stub4 () from /root/package/./libxpcom.so
#17 0x405b815f in NSGetModule () from /root/package/components/libnecko.so
#18 0x405b90ae in NSGetModule () from /root/package/components/libnecko.so
#19 0x405be744 in NSGetModule () from /root/package/components/libnecko.so
#20 0x405bd37d in NSGetModule () from /root/package/components/libnecko.so
#21 0x4057cba4 in NSGetModule () from /root/package/components/libnecko.so
#22 0x4057c450 in NSGetModule () from /root/package/components/libnecko.so
#23 0x400bebc3 in PL_HandleEvent () from /root/package/./libxpcom.so
#24 0x400bead6 in PL_ProcessPendingEvents () from /root/package/./libxpcom.so
#25 0x400bf85d in nsEventQueueImpl::ProcessPendingEvents ()
   from /root/package/./libxpcom.so
#26 0x40817fdf in NSGetModule () from /root/package/components/libwidget_gtk.so
#27 0x40817d9d in NSGetModule () from /root/package/components/libwidget_gtk.so
#28 0x409aeaca in g_io_unix_dispatch () from /usr/lib/libglib-1.2.so.0
#29 0x409b0186 in g_main_dispatch () from /usr/lib/libglib-1.2.so.0
#30 0x409b0751 in g_main_iterate () from /usr/lib/libglib-1.2.so.0
#31 0x409b08f1 in g_main_run () from /usr/lib/libglib-1.2.so.0
#32 0x408d85b9 in gtk_main () from /usr/lib/libgtk-1.2.so.0
#33 0x408184cc in NSGetModule () from /root/package/components/libwidget_gtk.so
#34 0x4045c18a in inflate_mask ()
   from /root/package/components/libnsappshell.so
#35 0x804d987 in JS_PushArguments ()
#36 0x804dde6 in JS_PushArguments ()
#37 0x4025a9cb in __libc_start_main (main=0x804dcdc <JS_PushArguments+11364>, 
    argc=2, argv=0xbffff644, init=0x804abf8 <_init>, fini=0x80533dc <_fini>, 
    rtld_fini=0x4000ae60 <_dl_fini>, stack_end=0xbffff63c)
    at ../sysdeps/generic/libc-start.c:92

Comment 8

[Diego]

I realized that the steps to reproduce it are a bit more complex.
1) remove all zdnet cookies, set accept only cookies from the same site, check
warn me before accepting a cookie.
2) Save, quit mozilla, restart mozilla http://www.zdnet.com/ with the URL on the
command line.
3) At the first "Accept cookie" request, check the remember this decision and
answer what you like (yes or no is indifferent, mozilla crashes anyway).

However this bug has just been filed as bug 52026: "crash on cookie with
"Remember this decision"" so sorry for the SPAM.

Comment 9

[Henrik Gemal]

This bug is not really fixed. When at the "about:" page and I select "do not 
allow cookies from this site", I'm getting an empty Site value in the Cookie 
Manager. Here I'd expect the "about:" to appear...

Comment 10

[sairuh (rarely reading bugmail)]

handing over to tever, the cookie man.

Comment 11

[Blake Ross]

reassigning to morse

Comment 12

[Marcia Knous [:marcia]]

Netscape Nav triage team: this is not a Netscape beta stopper.

Comment 13

[Boris Zbarsky [:bzbarsky]]

*** Bug 69515 has been marked as a duplicate of this bug. ***

Comment 14

[Wesha]

Somebody please verify, this one looks like a dup of bug 64068 for me.

Comment 15

[Stephen P. Morse]

This bug was fixed per comment 6.  In comment 9 gemal reported a different 
problem and that should have been opened in a separate report.  So I'm reclosing 
this as fixed.

gemal, if you still think there is a problem with "about:" giving a blank host 
name in the cookie list, then report a separate bug.  However, this is 
consistent with all other entries, since none of the cookie hosts contain the 
protocol.

Comment 16

[Tom Everingham]

verified

Comment 17

[Henrik Gemal]

bug 161636 has been filed about the blank entry