sm00th / bitlbee-discord

Bitlbee plugin for Discord (http://discordapp.com)
GNU General Public License v2.0
291 stars 27 forks source link

Added fixes for crashing upon somebody removing you from their friends. #171

Closed ghost closed 5 years ago

ghost commented 5 years ago

Before these fixes, RELATIONSHIP_REMOVE would cause a segfault. My understanding of this, is that RELATIONSHIP_REMOVE's json looks like: {"t":"RELATIONSHIP_REMOVE","s":97,"op":0,"d":{"type":1,"id":"<removed>"}} and this is the primary cause of this problem.

In the original code, json_value *uinfo = json_o_get(rinfo, "user"); would lead to a null pointer being fed into discord_canonize_name as char *name = discord_canonize_name(json_o_str(uinfo, "username")); has no handling for being fed a null pointer. This then causes a segmentation fault, as follows:

Program received signal SIGSEGV, Segmentation fault.
0x000055555558cd4b in str_reject_chars (string=0x0, reject=0x7ffff7fc90c6 "@+ ", replacement=95 '_') at misc.c:741
741     while (*c) {
#0  0x000055555558cd4b in str_reject_chars (string=0x0, reject=0x7ffff7fc90c6 "@+ ", replacement=95 '_') at misc.c:741
#1  0x00007ffff7fc1bea in ?? () from /usr/local/lib/bitlbee/discord.so
#2  0x00007ffff7fc41b3 in discord_parse_message () from /usr/local/lib/bitlbee/discord.so
#3  0x00007ffff7fc6826 in ?? () from /usr/local/lib/bitlbee/discord.so
#4  0x00005555555862e7 in gaim_io_invoke (source=0x555555657a70, condition=G_IO_IN, data=0x55555566a0e0) at events_glib.c:86
#5  0x00007ffff793e1d6 in g_main_context_dispatch () from /usr/lib/libglib-2.0.so.0
#6  0x00007ffff793e5b1 in ?? () from /usr/lib/libglib-2.0.so.0
#7  0x00007ffff793e8e2 in g_main_loop_run () from /usr/lib/libglib-2.0.so.0
#8  0x0000555555586257 in b_main_run () at events_glib.c:59
#9  0x0000555555583100 in main (argc=4, argv=0x7fffffffe588) at unix.c:196
(gdb)

Instead of adding in specific null pointer handling, I reworked it to get the ID from the request and to use get_user to get the user from that, instead. This is probably a messy fix and a better fix would be to break up discord_handle_relationship into more than one function and handle each seperately.

dequis commented 5 years ago

Oh welp this is my fault in #164 I guess

Instead of adding in specific null pointer handling, I reworked it to get the ID from the request and to use get_user to get the user from that, instead. This is probably a messy fix and a better fix would be to break up discord_handle_relationship into more than one function and handle each seperately.

Why though? Handling nulls in discord_canonize_name seems easier and cleaner than thi actually nevermind, i guess the old code silently failed to process this event, the name is needed for the imcb_buddy_status call.

Handling nulls in discord_canonize_name would still be a good idea.

ghost commented 5 years ago

I did think about that but I figure if a null pointer is being passed to discord_canonize_name, it'd be hiding actual errors unless a real warning was given in debug output...?

dequis commented 5 years ago

Yeah just g_return_val_if_fail(string, NULL);, that takes care of the warning.

ghost commented 5 years ago

I think that's all of them?

sm00th commented 5 years ago

I think that's all of them?

Yep, looks good now. Thank you again, merged.