Closed GoogleCodeExporter closed 8 years ago
To add, this works in
LUSCA_HEAD-r13894
however fails in the CVS, and every other head version newer to download.
Thanks
Original comment by unsecret...@gmail.com
on 15 Oct 2009 at 2:59
I need to know which function(s) were calling the string functions to lead to
that
particular crash. Doesn't the gdb backtrace give any further info?
Original comment by adrian.c...@gmail.com
on 15 Oct 2009 at 10:10
Full backtrace:
(gdb) bt
#0 0xf7f9d430 in __kernel_vsyscall ()
#1 0xf7dd2640 in raise () from /lib/i686/cmov/libc.so.6
#2 0xf7dd4018 in abort () from /lib/i686/cmov/libc.so.6
#3 0x080e9cc3 in xassert (msg=0x8130333 "s->buf", file=0x8130304 "String.c",
line=140) at debug.c:270
#4 0x080eb85d in stringDupToCOffset (s=0xff9a6734, offset=0) at String.c:140
#5 0x080eb910 in stringDupToC (s=0xff9a6734) at String.c:151
#6 0x0804b6d4 in accessLogCustom (al=0x91808d4, log=0x8ffeca8) at
access_log.c:547
#7 0x0804d643 in accessLogLog (al=0x91808d4, checklist=0x9030e18) at
access_log.c:1191
#8 0x0806c6c3 in httpRequestFree (data=0x9180850) at client_side.c:1250
#9 0x0806cb91 in connStateFree (fd=19, data=0x917f590) at client_side.c:1314
#10 0x080f79b1 in commCallCloseHandlers (fd=19) at comm.c:637
#11 0x080f7c42 in comm_close (fd=19) at comm.c:814
#12 0x08073333 in clientWriteComplete (fd=19, bufnotused=0x0, size=1050,
errflag=0,
data=0x9180850) at client_side.c:3363
#13 0x08072d3f in clientWriteBodyComplete (fd=19,
buf=0x9032e4c "<!DOCTYPE html PUBLIC \"-//W3C//DTD HTML 4.01//EN\"
\"http://www.w3.org/TR/html4/strict.dtd\">\n<html><head>\n<meta
http-equiv=\"Content-Type\" content=\"text/html;
charset=iso-8859-1\">\n<title>ERROR:
The reque"..., size=1050, errflag=0, data=0x9180850)
at client_side.c:3240
#14 0x080f624c in CommWriteStateCallbackAndFree (fd=19, code=0) at comm.c:130
#15 0x080f940e in commHandleWrite (fd=19, data=0x0) at comm.c:1319
#16 0x080fc11e in comm_call_handlers (fd=19, read_event=0, write_event=4) at
comm_generic.c:300
#17 0x080fc95f in do_comm_select (msec=0) at comm_epoll.c:233
#18 0x080fc434 in comm_select (msec=0) at comm_generic.c:384
#19 0x080fb771 in iapp_runonce (msec=60000) at mainloop.c:68
#20 0x0809ff05 in main (argc=2, argv=0xff9b6f44) at main.c:933
The bug appears to happen when access logging is enabled to log referers and
there is
no referer header sent (i.e. a direct request to a site).
One option is to patch libmem/String.c to allow stringDupToCOffset to handle
empty
(NULL) strings. A patch for libmem/String.c is attached.
Andy
Original comment by a...@andymillar.co.uk
on 23 Oct 2009 at 4:46
Oops. That patch reverts my changes /o\
Actual patch attached!
Andy
Original comment by a...@andymillar.co.uk
on 23 Oct 2009 at 5:12
Heh, there's a good reason why strdupC*() doesn't allow NULL strings. :)
I'll fix the referer log code itself. :) Thanks!
Original comment by adrian.c...@gmail.com
on 24 Oct 2009 at 12:06
Ok :-)
Here's another one - this time we patch accessLogCustom in src/access_log.c to
handle
being returned StringNull.
Andy
Original comment by a...@andymillar.co.uk
on 24 Oct 2009 at 3:04
Attachments:
So the question is - what -should- it log? A blank string? Or a (NULL) string?
Or
something else?
Answer -that- and I'll document this and fix it. :)
Original comment by adrian.c...@gmail.com
on 25 Oct 2009 at 6:04
if it were apache the method is to replace the null with a single
-
Cheers
Kev
Original comment by unsecret...@gmail.com
on 25 Oct 2009 at 10:27
By the looks of it, your string quoting routines correctly (imo) log a NULL
string as
"-".
Andy
Original comment by a...@andymillar.co.uk
on 25 Oct 2009 at 4:11
You're right. there is (if out && *out) { stuff; } else { append "-" }
Ok. With that in hand, I'll go and fix it.
Original comment by adrian.c...@gmail.com
on 26 Oct 2009 at 5:07
ok. I've decided that the best thing to do at the moment is modify
stringDupToCOffset() and stringDupToCSubstr().
I've reviewed the code and I really shouldn't have assumed I'd be being passed
non-NULL strings.
Please try this patch and let me know how it goes.
Index: String.c
===================================================================
--- String.c (revision 14342)
+++ String.c (working copy)
@@ -130,14 +130,23 @@
}
/*
- * This routine REQUIRES the string to be something and not NULL
+ * This routine SHOULD REQUIRE the string to be something and not NULL
+ * but plenty of code unfortunately doesn't check whether the string
+ * was empty in the first place.
+ *
+ * New code MUST NOT call this on an unset string.
+ *
* This copies -from- offset to the end of the string.
*/
char *
stringDupToCOffset(const String *s, int offset)
{
char *d;
- assert(s->buf);
+
+ /* This is horribly temporary [ahc] */
+ if (s->buf == NULL)
+ return NULL;
+
assert(offset <= s->len);
d = xmalloc(s->len + 1 - offset);
memcpy(d, s->buf + offset, s->len - offset);
@@ -156,7 +165,11 @@
{
char *d;
int l = XMIN(len, s->len);
- assert(s->buf);
+
+ /* This is horribly temporary [ahc] */
+ if (s->buf == NULL)
+ return NULL;
+
assert(len <= s->len);
d = xmalloc(l + 1);
memcpy(d, s->buf, l + 1);
Original comment by adrian.c...@gmail.com
on 26 Oct 2009 at 9:33
Workaround committed in r14343. Please give that a whirl!
Original comment by adrian.c...@gmail.com
on 27 Oct 2009 at 4:42
Hi there
r14343 checked out, and compiled / installed , have now been running for over 60
minutes without an assertion failed (it was occurring after some 10 seconds of
startup!).
Good work guys, this now appears to be fixed.
Cheers
Kev
Original comment by unsecret...@gmail.com
on 27 Oct 2009 at 10:23
just an update, my lusca install has been stable / working well since this
patch was
added.
Cheers
Original comment by unsecret...@gmail.com
on 30 Oct 2009 at 4:37
Fixed.
Original comment by adrian.c...@gmail.com
on 31 Oct 2009 at 12:47
Original issue reported on code.google.com by
unsecret...@gmail.com
on 10 Oct 2009 at 4:54