From 98836f8b7ee058c32d54562903676c4d16f83aa4 Mon Sep 17 00:00:00 2001 From: LemonBoy Date: Sat, 21 Oct 2017 10:21:03 +0200 Subject: [PATCH 01/19] Parse the K/V form in CAP LS This is a prerequisite for the IRC v3.2 compliance. --- src/irc/core/irc-cap.c | 43 ++++++++++++++++++++++++++++++++++---- src/irc/core/irc-servers.c | 2 +- src/irc/core/irc-servers.h | 2 +- src/perl/irc/Irc.xs | 15 +++++++++---- 4 files changed, 52 insertions(+), 10 deletions(-) diff --git a/src/irc/core/irc-cap.c b/src/irc/core/irc-cap.c index 5464e493..eac6f239 100644 --- a/src/irc/core/irc-cap.c +++ b/src/irc/core/irc-cap.c @@ -45,7 +45,7 @@ int cap_toggle (IRC_SERVER_REC *server, char *cap, int enable) if (enable && !gslist_find_string(server->cap_active, cap)) { /* Make sure the required cap is supported by the server */ - if (!gslist_find_string(server->cap_supported, cap)) + if (!g_hash_table_lookup_extended(server->cap_supported, cap, NULL, NULL)) return FALSE; irc_send_cmdv(server, "CAP REQ %s", cap); @@ -96,9 +96,44 @@ static void event_cap (IRC_SERVER_REC *server, char *args, char *nick, char *add caps_length = g_strv_length(caps); if (!g_strcmp0(evt, "LS")) { + if (server->cap_supported) { + g_hash_table_destroy(server->cap_supported); + } + /* Start with a fresh table */ + server->cap_supported = g_hash_table_new_full(g_str_hash, + g_str_equal, + g_free, g_free); + /* Create a list of the supported caps */ - for (i = 0; i < caps_length; i++) - server->cap_supported = g_slist_prepend(server->cap_supported, g_strdup(caps[i])); + for (i = 0; i < caps_length; i++) { + const char *name = caps[i]; + const char *eq = strchr(name, '='); + int fresh = TRUE; + + if (!eq) { + fresh = g_hash_table_insert(server->cap_supported, + g_strdup(name), + NULL); + } + /* Some values are in a KEY=VALUE form, parse them */ + else if (eq[1] != '\0') { + char *key = g_strndup(name, (int)(eq - name)); + char *val = g_strdup(eq + 1); + fresh = g_hash_table_insert(server->cap_supported, + key, val); + } + /* If the string ends after the '=' consider the value + * as invalid */ + else { + g_warning("Invalid CAP key/value pair"); + } + + /* The specification doesn't say anything about + * duplicated values, let's just warn the user */ + if (fresh == FALSE) { + g_warning("Duplicate value"); + } + } /* Request the required caps, if any */ if (server->cap_queue == NULL) { @@ -111,7 +146,7 @@ static void event_cap (IRC_SERVER_REC *server, char *args, char *nick, char *add /* Check whether the cap is supported by the server */ for (tmp = server->cap_queue; tmp != NULL; tmp = tmp->next) { - if (gslist_find_string(server->cap_supported, tmp->data)) { + if (g_hash_table_lookup_extended(server->cap_supported, tmp->data, NULL, NULL)) { if (avail_caps > 0) g_string_append_c(cmd, ' '); g_string_append(cmd, tmp->data); diff --git a/src/irc/core/irc-servers.c b/src/irc/core/irc-servers.c index 4eaab712..7fd83045 100644 --- a/src/irc/core/irc-servers.c +++ b/src/irc/core/irc-servers.c @@ -443,7 +443,7 @@ static void sig_disconnected(IRC_SERVER_REC *server) gslist_free_full(server->cap_active, (GDestroyNotify) g_free); server->cap_active = NULL; - gslist_free_full(server->cap_supported, (GDestroyNotify) g_free); + g_hash_table_destroy(server->cap_supported); server->cap_supported = NULL; gslist_free_full(server->cap_queue, (GDestroyNotify) g_free); diff --git a/src/irc/core/irc-servers.h b/src/irc/core/irc-servers.h index 09f3f81d..6c6002c8 100644 --- a/src/irc/core/irc-servers.h +++ b/src/irc/core/irc-servers.h @@ -75,7 +75,7 @@ struct _IRC_SERVER_REC { int max_whois_in_cmd; /* max. number of nicks in one /WHOIS command */ int max_msgs_in_cmd; /* max. number of targets in one /MSG */ - GSList *cap_supported; /* A list of caps supported by the server */ + GHashTable *cap_supported; /* A list of caps supported by the server */ GSList *cap_active; /* A list of caps active for this session */ GSList *cap_queue; /* A list of caps to request on connection */ diff --git a/src/perl/irc/Irc.xs b/src/perl/irc/Irc.xs index 41690010..bb2d27bb 100644 --- a/src/perl/irc/Irc.xs +++ b/src/perl/irc/Irc.xs @@ -12,7 +12,10 @@ static void perl_irc_connect_fill_hash(HV *hv, IRC_SERVER_CONNECT_REC *conn) static void perl_irc_server_fill_hash(HV *hv, IRC_SERVER_REC *server) { AV *av; + HV *hv_; GSList *tmp; + GHashTableIter iter; + gpointer key_, val_; perl_irc_connect_fill_hash(hv, server->connrec); perl_server_fill_hash(hv, (SERVER_REC *) server); @@ -34,10 +37,14 @@ static void perl_irc_server_fill_hash(HV *hv, IRC_SERVER_REC *server) (void) hv_store(hv, "cap_complete", 12, newSViv(server->cap_complete), 0); (void) hv_store(hv, "sasl_success", 12, newSViv(server->sasl_success), 0); - av = newAV(); - for (tmp = server->cap_supported; tmp != NULL; tmp = tmp->next) - av_push(av, new_pv(tmp->data)); - (void) hv_store(hv, "cap_supported", 13, newRV_noinc((SV*)av), 0); + hv_ = newHV(); + g_hash_table_iter_init(&iter, server->cap_supported); + while (g_hash_table_iter_next(&iter, &key_, &val_)) { + char *key = (char *)key_; + char *val = (char *)val_; + hv_store(hv_, key, strlen(key), new_pv(val), 0); + } + (void) hv_store(hv, "cap_supported", 13, newRV_noinc((SV*)hv_), 0); av = newAV(); for (tmp = server->cap_active; tmp != NULL; tmp = tmp->next) From d21706e1cc78284d5e7b2d69ebe4873e459d0e9b Mon Sep 17 00:00:00 2001 From: LemonBoy Date: Sat, 21 Oct 2017 10:35:49 +0200 Subject: [PATCH 02/19] Factor out the parsing function This is also needed for CAP NEW and CAP DEL. --- src/irc/core/irc-cap.c | 56 ++++++++++++++++++++++++++---------------- 1 file changed, 35 insertions(+), 21 deletions(-) diff --git a/src/irc/core/irc-cap.c b/src/irc/core/irc-cap.c index eac6f239..9ac232b5 100644 --- a/src/irc/core/irc-cap.c +++ b/src/irc/core/irc-cap.c @@ -79,6 +79,33 @@ static void cap_emit_signal (IRC_SERVER_REC *server, char *cmd, char *args) g_free(signal_name); } +static gboolean parse_cap_name(char *name, char **key, char **val) +{ + g_return_val_if_fail(name != NULL, FALSE); + g_return_val_if_fail(name[0] != '\0', FALSE); + + const char *eq = strchr(name, '='); + /* KEY only value */ + if (!eq) { + *key = g_strdup(name); + *val = NULL; + return TRUE; + } + /* Some values are in a KEY=VALUE form, parse them */ + else if (eq[1] != '\0') { + *key = g_strndup(name, (int)(eq - name)); + *val = g_strdup(eq + 1); + return TRUE; + } + /* If the string ends after the '=' consider the value + * as invalid */ + else { + *key = NULL; + *val = NULL; + return FALSE; + } +} + static void event_cap (IRC_SERVER_REC *server, char *args, char *nick, char *address) { GSList *tmp; @@ -106,33 +133,20 @@ static void event_cap (IRC_SERVER_REC *server, char *args, char *nick, char *add /* Create a list of the supported caps */ for (i = 0; i < caps_length; i++) { - const char *name = caps[i]; - const char *eq = strchr(name, '='); - int fresh = TRUE; + char *key, *val; - if (!eq) { - fresh = g_hash_table_insert(server->cap_supported, - g_strdup(name), - NULL); + if (parse_cap_name(caps[i], &key, &val)) { + if (!g_hash_table_insert(server->cap_supported, + key, val)) { + /* The specification doesn't say anything about + * duplicated values, let's just warn the user */ + g_warning("Duplicate value"); + } } - /* Some values are in a KEY=VALUE form, parse them */ - else if (eq[1] != '\0') { - char *key = g_strndup(name, (int)(eq - name)); - char *val = g_strdup(eq + 1); - fresh = g_hash_table_insert(server->cap_supported, - key, val); - } - /* If the string ends after the '=' consider the value - * as invalid */ else { g_warning("Invalid CAP key/value pair"); } - /* The specification doesn't say anything about - * duplicated values, let's just warn the user */ - if (fresh == FALSE) { - g_warning("Duplicate value"); - } } /* Request the required caps, if any */ From 57827ca743e95a6b6ad830fab03f46efaf1d9069 Mon Sep 17 00:00:00 2001 From: LemonBoy Date: Sat, 21 Oct 2017 10:44:53 +0200 Subject: [PATCH 03/19] Don't free the hash table if there's none Glib doesn't like that and shows a harmless warning. --- src/irc/core/irc-servers.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/irc/core/irc-servers.c b/src/irc/core/irc-servers.c index 7fd83045..3b4e1a52 100644 --- a/src/irc/core/irc-servers.c +++ b/src/irc/core/irc-servers.c @@ -443,8 +443,10 @@ static void sig_disconnected(IRC_SERVER_REC *server) gslist_free_full(server->cap_active, (GDestroyNotify) g_free); server->cap_active = NULL; - g_hash_table_destroy(server->cap_supported); - server->cap_supported = NULL; + if (server->cap_supported) { + g_hash_table_destroy(server->cap_supported); + server->cap_supported = NULL; + } gslist_free_full(server->cap_queue, (GDestroyNotify) g_free); server->cap_queue = NULL; From 8c87766132b7dbb75d8a49548ff7c97037ea983b Mon Sep 17 00:00:00 2001 From: LemonBoy Date: Sat, 21 Oct 2017 11:23:44 +0200 Subject: [PATCH 04/19] Parse multiline responses to CAP LS The parsing logic isn't too elegant because of the optional parameter used for signaling if a response has a continuation one. --- src/irc/core/irc-cap.c | 89 +++++++++++++++++++++++++++--------------- 1 file changed, 57 insertions(+), 32 deletions(-) diff --git a/src/irc/core/irc-cap.c b/src/irc/core/irc-cap.c index 9ac232b5..602a7b06 100644 --- a/src/irc/core/irc-cap.c +++ b/src/irc/core/irc-cap.c @@ -110,13 +110,31 @@ static void event_cap (IRC_SERVER_REC *server, char *args, char *nick, char *add { GSList *tmp; GString *cmd; - char *params, *evt, *list, **caps; - int i, caps_length, disable, avail_caps; + char *params, *evt, *list, *star, **caps; + int i, caps_length, disable, avail_caps, multiline; - params = event_get_params(args, 3, NULL, &evt, &list); + params = event_get_params(args, 4, NULL, &evt, &star, &list); if (params == NULL) return; + /* Multiline responses have an additional parameter and we have to do + * this stupid dance to parse them */ + if (evt[0] == 'L' && !strcmp(star, "*")) { + multiline = TRUE; + } + /* This branch covers the '*' parameter isn't present, adjust the + * parameter pointer to compensate for this */ + else if (list[0] == '\0') { + multiline = FALSE; + list = star; + } + /* Malformed request, terminate the negotiation */ + else { + cap_finish_negotiation(server); + g_warn_if_reached(); + return; + } + /* Strip the trailing whitespaces before splitting the string, some servers send responses with * superfluous whitespaces that g_strsplit the interprets as tokens */ caps = g_strsplit(g_strchomp(list), " ", -1); @@ -149,37 +167,41 @@ static void event_cap (IRC_SERVER_REC *server, char *args, char *nick, char *add } - /* Request the required caps, if any */ - if (server->cap_queue == NULL) { - cap_finish_negotiation(server); - } - else { - cmd = g_string_new("CAP REQ :"); - - avail_caps = 0; - - /* Check whether the cap is supported by the server */ - for (tmp = server->cap_queue; tmp != NULL; tmp = tmp->next) { - if (g_hash_table_lookup_extended(server->cap_supported, tmp->data, NULL, NULL)) { - if (avail_caps > 0) - g_string_append_c(cmd, ' '); - g_string_append(cmd, tmp->data); - - avail_caps++; - } - } - - /* Clear the queue here */ - gslist_free_full(server->cap_queue, (GDestroyNotify) g_free); - server->cap_queue = NULL; - - /* If the server doesn't support any cap we requested close the negotiation here */ - if (avail_caps > 0) - irc_send_cmd_now(server, cmd->str); - else + /* A multiline response is always terminated by a normal one, + * wait until we receive that one to require any CAP */ + if (multiline == FALSE) { + /* No CAP has been requested */ + if (server->cap_queue == NULL) { cap_finish_negotiation(server); + } + else { + cmd = g_string_new("CAP REQ :"); - g_string_free(cmd, TRUE); + avail_caps = 0; + + /* Check whether the cap is supported by the server */ + for (tmp = server->cap_queue; tmp != NULL; tmp = tmp->next) { + if (g_hash_table_lookup_extended(server->cap_supported, tmp->data, NULL, NULL)) { + if (avail_caps > 0) + g_string_append_c(cmd, ' '); + g_string_append(cmd, tmp->data); + + avail_caps++; + } + } + + /* Clear the queue here */ + gslist_free_full(server->cap_queue, (GDestroyNotify) g_free); + server->cap_queue = NULL; + + /* If the server doesn't support any cap we requested close the negotiation here */ + if (avail_caps > 0) + irc_send_cmd_now(server, cmd->str); + else + cap_finish_negotiation(server); + + g_string_free(cmd, TRUE); + } } } else if (!g_strcmp0(evt, "ACK")) { @@ -214,6 +236,9 @@ static void event_cap (IRC_SERVER_REC *server, char *args, char *nick, char *add for (i = 0; i < caps_length; i++) cap_emit_signal(server, "nak", caps[i]); } + else { + g_warning("Unhandled CAP subcommand %s", evt); + } g_strfreev(caps); g_free(params); From f4d811ddf51ce03e90e0417a6c25baeb9aa48353 Mon Sep 17 00:00:00 2001 From: LemonBoy Date: Sat, 21 Oct 2017 17:11:43 +0200 Subject: [PATCH 05/19] Handle CAP {ADD,DEL} from cap-notify This is the last piece of the puzzle. --- src/irc/core/irc-cap.c | 36 ++++++++++++++++++++++++++++++++++-- 1 file changed, 34 insertions(+), 2 deletions(-) diff --git a/src/irc/core/irc-cap.c b/src/irc/core/irc-cap.c index 602a7b06..2378d640 100644 --- a/src/irc/core/irc-cap.c +++ b/src/irc/core/irc-cap.c @@ -135,6 +135,8 @@ static void event_cap (IRC_SERVER_REC *server, char *args, char *nick, char *add return; } + g_warning("%s -> %s", evt, list); + /* Strip the trailing whitespaces before splitting the string, some servers send responses with * superfluous whitespaces that g_strsplit the interprets as tokens */ caps = g_strsplit(g_strchomp(list), " ", -1); @@ -158,11 +160,11 @@ static void event_cap (IRC_SERVER_REC *server, char *args, char *nick, char *add key, val)) { /* The specification doesn't say anything about * duplicated values, let's just warn the user */ - g_warning("Duplicate value"); + g_warning("Duplicate value %s", key); } } else { - g_warning("Invalid CAP key/value pair"); + g_warning("Invalid CAP %s key/value pair", evt); } } @@ -236,6 +238,36 @@ static void event_cap (IRC_SERVER_REC *server, char *args, char *nick, char *add for (i = 0; i < caps_length; i++) cap_emit_signal(server, "nak", caps[i]); } + else if (!strcmp(evt, "NEW")) { + for (i = 0; i < caps_length; i++) { + char *key, *val; + + if (parse_cap_name(caps[i], &key, &val)) { + g_hash_table_insert(server->cap_supported, + key, val); + } + else { + g_warning("Invalid CAP %s key/value pair", evt); + } + cap_emit_signal(server, "new", key); + } + } + else if (!strcmp(evt, "DEL")) { + for (i = 0; i < caps_length; i++) { + char *key, *val; + + if (parse_cap_name(caps[i], &key, &val)) { + g_hash_table_remove(server->cap_supported, key); + } + else { + g_warning("Invalid CAP %s key/value pair", evt); + } + cap_emit_signal(server, "delete", key); + /* The server removed this CAP, remove it from the list + * of the active ones if we had requested it */ + server->cap_active = gslist_remove_string(server->cap_active, key); + } + } else { g_warning("Unhandled CAP subcommand %s", evt); } From cfc8c9f8e2d982ee3ebff7afa1d1bdeb04003029 Mon Sep 17 00:00:00 2001 From: LemonBoy Date: Sat, 21 Oct 2017 17:38:06 +0200 Subject: [PATCH 06/19] Properly dispose the GSList chains We forgot to free the link and the data, oops. --- src/core/misc.c | 13 +++++++++++++ src/core/misc.h | 1 + src/irc/core/irc-cap.c | 8 +++----- 3 files changed, 17 insertions(+), 5 deletions(-) diff --git a/src/core/misc.c b/src/core/misc.c index 4e9f4bbe..27741220 100644 --- a/src/core/misc.c +++ b/src/core/misc.c @@ -218,6 +218,19 @@ GSList *gslist_remove_string (GSList *list, const char *str) return list; } +GSList *gslist_delete_string (GSList *list, const char *str, GDestroyNotify free_func) +{ + GSList *l; + + l = g_slist_find_custom(list, str, (GCompareFunc) g_strcmp0); + if (l != NULL) { + free_func(l->data); + return g_slist_delete_link(list, l); + } + + return list; +} + /* `list' contains pointer to structure with a char* to string. */ char *gslistptr_to_string(GSList *list, int offset, const char *delimiter) { diff --git a/src/core/misc.h b/src/core/misc.h index 375744db..689cf5c2 100644 --- a/src/core/misc.h +++ b/src/core/misc.h @@ -22,6 +22,7 @@ GSList *gslist_find_icase_string(GSList *list, const char *key); GList *glist_find_string(GList *list, const char *key); GList *glist_find_icase_string(GList *list, const char *key); GSList *gslist_remove_string (GSList *list, const char *str); +GSList *gslist_delete_string (GSList *list, const char *str, GDestroyNotify free_func); void gslist_free_full (GSList *list, GDestroyNotify free_func); diff --git a/src/irc/core/irc-cap.c b/src/irc/core/irc-cap.c index 2378d640..a5ae07aa 100644 --- a/src/irc/core/irc-cap.c +++ b/src/irc/core/irc-cap.c @@ -36,7 +36,7 @@ int cap_toggle (IRC_SERVER_REC *server, char *cap, int enable) return TRUE; } else if (!enable && gslist_find_string(server->cap_queue, cap)) { - server->cap_queue = gslist_remove_string(server->cap_queue, cap); + server->cap_queue = gslist_delete_string(server->cap_queue, cap, g_free); return TRUE; } @@ -135,8 +135,6 @@ static void event_cap (IRC_SERVER_REC *server, char *args, char *nick, char *add return; } - g_warning("%s -> %s", evt, list); - /* Strip the trailing whitespaces before splitting the string, some servers send responses with * superfluous whitespaces that g_strsplit the interprets as tokens */ caps = g_strsplit(g_strchomp(list), " ", -1); @@ -214,7 +212,7 @@ static void event_cap (IRC_SERVER_REC *server, char *args, char *nick, char *add disable = (*caps[i] == '-'); if (disable) - server->cap_active = gslist_remove_string(server->cap_active, caps[i] + 1); + server->cap_active = gslist_delete_string(server->cap_active, caps[i] + 1, g_free); else server->cap_active = g_slist_prepend(server->cap_active, g_strdup(caps[i])); @@ -265,7 +263,7 @@ static void event_cap (IRC_SERVER_REC *server, char *args, char *nick, char *add cap_emit_signal(server, "delete", key); /* The server removed this CAP, remove it from the list * of the active ones if we had requested it */ - server->cap_active = gslist_remove_string(server->cap_active, key); + server->cap_active = gslist_delete_string(server->cap_active, key, g_free); } } else { From 432368bdc6b941c1ff6319748b357bd5bae1bb6e Mon Sep 17 00:00:00 2001 From: LemonBoy Date: Sat, 21 Oct 2017 17:40:45 +0200 Subject: [PATCH 07/19] Use strcmp instead of g_strcmp0 There's no need to use the latter. --- src/irc/core/irc-cap.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/irc/core/irc-cap.c b/src/irc/core/irc-cap.c index a5ae07aa..f448ef83 100644 --- a/src/irc/core/irc-cap.c +++ b/src/irc/core/irc-cap.c @@ -140,7 +140,7 @@ static void event_cap (IRC_SERVER_REC *server, char *args, char *nick, char *add caps = g_strsplit(g_strchomp(list), " ", -1); caps_length = g_strv_length(caps); - if (!g_strcmp0(evt, "LS")) { + if (!strcmp(evt, "LS")) { if (server->cap_supported) { g_hash_table_destroy(server->cap_supported); } @@ -204,7 +204,7 @@ static void event_cap (IRC_SERVER_REC *server, char *args, char *nick, char *add } } } - else if (!g_strcmp0(evt, "ACK")) { + else if (!strcmp(evt, "ACK")) { int got_sasl = FALSE; /* Emit a signal for every ack'd cap */ @@ -216,7 +216,7 @@ static void event_cap (IRC_SERVER_REC *server, char *args, char *nick, char *add else server->cap_active = g_slist_prepend(server->cap_active, g_strdup(caps[i])); - if (!g_strcmp0(caps[i], "sasl")) + if (!strcmp(caps[i], "sasl")) got_sasl = TRUE; cap_emit_signal(server, "ack", caps[i]); @@ -228,7 +228,7 @@ static void event_cap (IRC_SERVER_REC *server, char *args, char *nick, char *add if (got_sasl == FALSE) cap_finish_negotiation(server); } - else if (!g_strcmp0(evt, "NAK")) { + else if (!strcmp(evt, "NAK")) { g_warning("The server answered with a NAK to our CAP request, this should not happen"); /* A NAK'd request means that a required cap can't be enabled or disabled, don't update the From f683e81880ac4408693582df3ec11d640684c78d Mon Sep 17 00:00:00 2001 From: LemonBoy Date: Mon, 23 Oct 2017 12:42:37 +0200 Subject: [PATCH 08/19] Prevent a NULL pointer deference Always create the cap_supported table when a CAP event is received. --- src/irc/core/irc-cap.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/irc/core/irc-cap.c b/src/irc/core/irc-cap.c index f448ef83..46276243 100644 --- a/src/irc/core/irc-cap.c +++ b/src/irc/core/irc-cap.c @@ -135,19 +135,21 @@ static void event_cap (IRC_SERVER_REC *server, char *args, char *nick, char *add return; } + /* The table is created only when needed */ + if (server->cap_supported == NULL) { + server->cap_supported = g_hash_table_new_full(g_str_hash, + g_str_equal, + g_free, g_free); + } + /* Strip the trailing whitespaces before splitting the string, some servers send responses with * superfluous whitespaces that g_strsplit the interprets as tokens */ caps = g_strsplit(g_strchomp(list), " ", -1); caps_length = g_strv_length(caps); if (!strcmp(evt, "LS")) { - if (server->cap_supported) { - g_hash_table_destroy(server->cap_supported); - } - /* Start with a fresh table */ - server->cap_supported = g_hash_table_new_full(g_str_hash, - g_str_equal, - g_free, g_free); + /* Throw away everything and start from scratch */ + g_hash_table_remove_all(server->cap_supported); /* Create a list of the supported caps */ for (i = 0; i < caps_length; i++) { From 74409aa85071390a3969fffa21c08a6736efe314 Mon Sep 17 00:00:00 2001 From: LemonBoy Date: Mon, 23 Oct 2017 20:08:19 +0200 Subject: [PATCH 09/19] Miscellaneous fixes Stylistic stuff, please ignore. --- src/irc/core/irc-cap.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/irc/core/irc-cap.c b/src/irc/core/irc-cap.c index 46276243..9af55130 100644 --- a/src/irc/core/irc-cap.c +++ b/src/irc/core/irc-cap.c @@ -86,14 +86,14 @@ static gboolean parse_cap_name(char *name, char **key, char **val) const char *eq = strchr(name, '='); /* KEY only value */ - if (!eq) { + if (eq == NULL) { *key = g_strdup(name); *val = NULL; return TRUE; } /* Some values are in a KEY=VALUE form, parse them */ else if (eq[1] != '\0') { - *key = g_strndup(name, (int)(eq - name)); + *key = g_strndup(name, (gsize)(eq - name)); *val = g_strdup(eq + 1); return TRUE; } From cd107deb463934f7a8611cdc8d17b861a93d97f9 Mon Sep 17 00:00:00 2001 From: LemonBoy Date: Mon, 23 Oct 2017 21:19:51 +0200 Subject: [PATCH 10/19] Prevent a memory leak When a CAP DEL is received the key/val pair is not stored in the hashtable at all so just free them when we're done. --- src/irc/core/irc-cap.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/irc/core/irc-cap.c b/src/irc/core/irc-cap.c index 9af55130..8be9d33f 100644 --- a/src/irc/core/irc-cap.c +++ b/src/irc/core/irc-cap.c @@ -266,6 +266,10 @@ static void event_cap (IRC_SERVER_REC *server, char *args, char *nick, char *add /* The server removed this CAP, remove it from the list * of the active ones if we had requested it */ server->cap_active = gslist_delete_string(server->cap_active, key, g_free); + /* We don't transfer the ownership of those two + * variables this time, just free them when we're done. */ + g_free(key); + g_free(val); } } else { From 9160ddaffd45b03c523320ef8f71bb3a8a9fb87a Mon Sep 17 00:00:00 2001 From: LemonBoy Date: Mon, 23 Oct 2017 21:23:59 +0200 Subject: [PATCH 11/19] Keep processing the CAPs on error If an invalid CAP is found we keep going by parsing the next one. --- src/irc/core/irc-cap.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/irc/core/irc-cap.c b/src/irc/core/irc-cap.c index 8be9d33f..d93dfac4 100644 --- a/src/irc/core/irc-cap.c +++ b/src/irc/core/irc-cap.c @@ -165,8 +165,8 @@ static void event_cap (IRC_SERVER_REC *server, char *args, char *nick, char *add } else { g_warning("Invalid CAP %s key/value pair", evt); + continue; } - } /* A multiline response is always terminated by a normal one, @@ -248,6 +248,7 @@ static void event_cap (IRC_SERVER_REC *server, char *args, char *nick, char *add } else { g_warning("Invalid CAP %s key/value pair", evt); + continue; } cap_emit_signal(server, "new", key); } @@ -261,6 +262,7 @@ static void event_cap (IRC_SERVER_REC *server, char *args, char *nick, char *add } else { g_warning("Invalid CAP %s key/value pair", evt); + continue; } cap_emit_signal(server, "delete", key); /* The server removed this CAP, remove it from the list From c00132ac4cf78702b4f72fc6a0085432bcef2fac Mon Sep 17 00:00:00 2001 From: LemonBoy Date: Tue, 24 Oct 2017 15:47:06 +0200 Subject: [PATCH 12/19] Simplify the code Early exit, simpler code. --- src/irc/core/irc-cap.c | 31 +++++++++++++------------------ 1 file changed, 13 insertions(+), 18 deletions(-) diff --git a/src/irc/core/irc-cap.c b/src/irc/core/irc-cap.c index d93dfac4..5d033bbf 100644 --- a/src/irc/core/irc-cap.c +++ b/src/irc/core/irc-cap.c @@ -155,18 +155,16 @@ static void event_cap (IRC_SERVER_REC *server, char *args, char *nick, char *add for (i = 0; i < caps_length; i++) { char *key, *val; - if (parse_cap_name(caps[i], &key, &val)) { - if (!g_hash_table_insert(server->cap_supported, - key, val)) { - /* The specification doesn't say anything about - * duplicated values, let's just warn the user */ - g_warning("Duplicate value %s", key); - } - } - else { + if (!parse_cap_name(caps[i], &key, &val)) { g_warning("Invalid CAP %s key/value pair", evt); continue; } + + if (!g_hash_table_insert(server->cap_supported, key, val)) { + /* The specification doesn't say anything about + * duplicated values, let's just warn the user */ + g_warning("Duplicate value %s", key); + } } /* A multiline response is always terminated by a normal one, @@ -242,14 +240,12 @@ static void event_cap (IRC_SERVER_REC *server, char *args, char *nick, char *add for (i = 0; i < caps_length; i++) { char *key, *val; - if (parse_cap_name(caps[i], &key, &val)) { - g_hash_table_insert(server->cap_supported, - key, val); - } - else { + if (!parse_cap_name(caps[i], &key, &val)) { g_warning("Invalid CAP %s key/value pair", evt); continue; } + + g_hash_table_insert(server->cap_supported, key, val); cap_emit_signal(server, "new", key); } } @@ -257,13 +253,12 @@ static void event_cap (IRC_SERVER_REC *server, char *args, char *nick, char *add for (i = 0; i < caps_length; i++) { char *key, *val; - if (parse_cap_name(caps[i], &key, &val)) { - g_hash_table_remove(server->cap_supported, key); - } - else { + if (!parse_cap_name(caps[i], &key, &val)) { g_warning("Invalid CAP %s key/value pair", evt); continue; } + + g_hash_table_remove(server->cap_supported, key); cap_emit_signal(server, "delete", key); /* The server removed this CAP, remove it from the list * of the active ones if we had requested it */ From 6c45ab0493e42bca08016ad70a54bb523282b7b8 Mon Sep 17 00:00:00 2001 From: LemonBoy Date: Sun, 29 Oct 2017 15:59:51 +0100 Subject: [PATCH 13/19] Command names may be in lower-case Do not take the string case into account when comparing the command name. --- src/irc/core/irc-cap.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/irc/core/irc-cap.c b/src/irc/core/irc-cap.c index 5d033bbf..1a76fc8e 100644 --- a/src/irc/core/irc-cap.c +++ b/src/irc/core/irc-cap.c @@ -147,7 +147,7 @@ static void event_cap (IRC_SERVER_REC *server, char *args, char *nick, char *add caps = g_strsplit(g_strchomp(list), " ", -1); caps_length = g_strv_length(caps); - if (!strcmp(evt, "LS")) { + if (!g_ascii_strcasecmp(evt, "LS")) { /* Throw away everything and start from scratch */ g_hash_table_remove_all(server->cap_supported); @@ -204,7 +204,7 @@ static void event_cap (IRC_SERVER_REC *server, char *args, char *nick, char *add } } } - else if (!strcmp(evt, "ACK")) { + else if (!g_ascii_strcasecmp(evt, "ACK")) { int got_sasl = FALSE; /* Emit a signal for every ack'd cap */ @@ -228,7 +228,7 @@ static void event_cap (IRC_SERVER_REC *server, char *args, char *nick, char *add if (got_sasl == FALSE) cap_finish_negotiation(server); } - else if (!strcmp(evt, "NAK")) { + else if (!g_ascii_strcasecmp(evt, "NAK")) { g_warning("The server answered with a NAK to our CAP request, this should not happen"); /* A NAK'd request means that a required cap can't be enabled or disabled, don't update the @@ -236,7 +236,7 @@ static void event_cap (IRC_SERVER_REC *server, char *args, char *nick, char *add for (i = 0; i < caps_length; i++) cap_emit_signal(server, "nak", caps[i]); } - else if (!strcmp(evt, "NEW")) { + else if (!g_ascii_strcasecmp(evt, "NEW")) { for (i = 0; i < caps_length; i++) { char *key, *val; @@ -249,7 +249,7 @@ static void event_cap (IRC_SERVER_REC *server, char *args, char *nick, char *add cap_emit_signal(server, "new", key); } } - else if (!strcmp(evt, "DEL")) { + else if (!g_ascii_strcasecmp(evt, "DEL")) { for (i = 0; i < caps_length; i++) { char *key, *val; From f3a535564880d73b75263b685794a7165e56f9bd Mon Sep 17 00:00:00 2001 From: LemonBoy Date: Sun, 29 Oct 2017 16:03:28 +0100 Subject: [PATCH 14/19] Match LS instead of checking the first letter only --- src/irc/core/irc-cap.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/irc/core/irc-cap.c b/src/irc/core/irc-cap.c index 1a76fc8e..a9d79c24 100644 --- a/src/irc/core/irc-cap.c +++ b/src/irc/core/irc-cap.c @@ -119,7 +119,7 @@ static void event_cap (IRC_SERVER_REC *server, char *args, char *nick, char *add /* Multiline responses have an additional parameter and we have to do * this stupid dance to parse them */ - if (evt[0] == 'L' && !strcmp(star, "*")) { + if (!g_ascii_strcasecmp(evt, "LS") && !strcmp(star, "*")) { multiline = TRUE; } /* This branch covers the '*' parameter isn't present, adjust the From 4b9fcbc15ae3561c34944e30e24fd1d54346bb99 Mon Sep 17 00:00:00 2001 From: LemonBoy Date: Sun, 29 Oct 2017 16:06:36 +0100 Subject: [PATCH 15/19] Nicer error message when a duplicate CAP in LS --- src/irc/core/irc-cap.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/irc/core/irc-cap.c b/src/irc/core/irc-cap.c index a9d79c24..880a15b6 100644 --- a/src/irc/core/irc-cap.c +++ b/src/irc/core/irc-cap.c @@ -163,7 +163,7 @@ static void event_cap (IRC_SERVER_REC *server, char *args, char *nick, char *add if (!g_hash_table_insert(server->cap_supported, key, val)) { /* The specification doesn't say anything about * duplicated values, let's just warn the user */ - g_warning("Duplicate value %s", key); + g_warning("The server sent the %s capability twice", key); } } From fed791ed9100552edbe643d8b3d0e0bc271e88d4 Mon Sep 17 00:00:00 2001 From: LemonBoy Date: Mon, 30 Oct 2017 17:41:16 +0100 Subject: [PATCH 16/19] Fix a problem with multiline responses Do not clear the whole table every time a response is received. --- src/irc/core/irc-cap.c | 8 ++++++-- src/irc/core/irc-servers.h | 1 + 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/irc/core/irc-cap.c b/src/irc/core/irc-cap.c index 880a15b6..90bffd80 100644 --- a/src/irc/core/irc-cap.c +++ b/src/irc/core/irc-cap.c @@ -148,8 +148,12 @@ static void event_cap (IRC_SERVER_REC *server, char *args, char *nick, char *add caps_length = g_strv_length(caps); if (!g_ascii_strcasecmp(evt, "LS")) { - /* Throw away everything and start from scratch */ - g_hash_table_remove_all(server->cap_supported); + if (!server->cap_in_multiline) { + /* Throw away everything and start from scratch */ + g_hash_table_remove_all(server->cap_supported); + } + + server->cap_in_multiline = multiline; /* Create a list of the supported caps */ for (i = 0; i < caps_length; i++) { diff --git a/src/irc/core/irc-servers.h b/src/irc/core/irc-servers.h index 6c6002c8..1374e846 100644 --- a/src/irc/core/irc-servers.h +++ b/src/irc/core/irc-servers.h @@ -68,6 +68,7 @@ struct _IRC_SERVER_REC { unsigned int motd_got:1; /* We've received MOTD */ unsigned int isupport_sent:1; /* Server has sent us an isupport reply */ unsigned int cap_complete:1; /* We've done the initial CAP negotiation */ + unsigned int cap_in_multiline:1; /* We're waiting for the multiline response to end */ unsigned int sasl_success:1; /* Did we authenticate successfully ? */ int max_kicks_in_cmd; /* max. number of people to kick with one /KICK command */ From 474ee8ee70c9ff9c69487746a9740355f38a01b2 Mon Sep 17 00:00:00 2001 From: LemonBoy Date: Sun, 7 Jan 2018 12:44:28 +0100 Subject: [PATCH 17/19] Address some minor stylish nits --- src/irc/core/irc-cap.c | 10 +++++----- src/irc/core/irc-servers.c | 4 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/irc/core/irc-cap.c b/src/irc/core/irc-cap.c index 90bffd80..95a051f0 100644 --- a/src/irc/core/irc-cap.c +++ b/src/irc/core/irc-cap.c @@ -81,25 +81,25 @@ static void cap_emit_signal (IRC_SERVER_REC *server, char *cmd, char *args) static gboolean parse_cap_name(char *name, char **key, char **val) { + const char *eq; + g_return_val_if_fail(name != NULL, FALSE); g_return_val_if_fail(name[0] != '\0', FALSE); - const char *eq = strchr(name, '='); + eq = strchr(name, '='); /* KEY only value */ if (eq == NULL) { *key = g_strdup(name); *val = NULL; return TRUE; - } /* Some values are in a KEY=VALUE form, parse them */ - else if (eq[1] != '\0') { + } else if (eq[1] != '\0') { *key = g_strndup(name, (gsize)(eq - name)); *val = g_strdup(eq + 1); return TRUE; - } /* If the string ends after the '=' consider the value * as invalid */ - else { + } else { *key = NULL; *val = NULL; return FALSE; diff --git a/src/irc/core/irc-servers.c b/src/irc/core/irc-servers.c index 3b4e1a52..e154d17f 100644 --- a/src/irc/core/irc-servers.c +++ b/src/irc/core/irc-servers.c @@ -444,8 +444,8 @@ static void sig_disconnected(IRC_SERVER_REC *server) server->cap_active = NULL; if (server->cap_supported) { - g_hash_table_destroy(server->cap_supported); - server->cap_supported = NULL; + g_hash_table_destroy(server->cap_supported); + server->cap_supported = NULL; } gslist_free_full(server->cap_queue, (GDestroyNotify) g_free); From b0b40be82e046785562fdb18dca3b1afb1b08a2b Mon Sep 17 00:00:00 2001 From: LemonBoy Date: Wed, 24 Jan 2018 10:47:40 +0100 Subject: [PATCH 18/19] Deprecate gslist_remove_string It is not used anymore and it leaks memory. --- src/core/misc.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/misc.h b/src/core/misc.h index 689cf5c2..a46a1432 100644 --- a/src/core/misc.h +++ b/src/core/misc.h @@ -21,7 +21,7 @@ GSList *gslist_find_string(GSList *list, const char *key); GSList *gslist_find_icase_string(GSList *list, const char *key); GList *glist_find_string(GList *list, const char *key); GList *glist_find_icase_string(GList *list, const char *key); -GSList *gslist_remove_string (GSList *list, const char *str); +GSList *gslist_remove_string (GSList *list, const char *str) G_GNUC_DEPRECATED; GSList *gslist_delete_string (GSList *list, const char *str, GDestroyNotify free_func); void gslist_free_full (GSList *list, GDestroyNotify free_func); From 260733475c2b701c35024a3b4fbe900751b45341 Mon Sep 17 00:00:00 2001 From: LemonBoy Date: Wed, 24 Jan 2018 10:55:20 +0100 Subject: [PATCH 19/19] Accept CAPs with an empty value (KEY=) --- src/irc/core/irc-cap.c | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/src/irc/core/irc-cap.c b/src/irc/core/irc-cap.c index 95a051f0..dd720841 100644 --- a/src/irc/core/irc-cap.c +++ b/src/irc/core/irc-cap.c @@ -91,19 +91,13 @@ static gboolean parse_cap_name(char *name, char **key, char **val) if (eq == NULL) { *key = g_strdup(name); *val = NULL; - return TRUE; /* Some values are in a KEY=VALUE form, parse them */ - } else if (eq[1] != '\0') { + } else { *key = g_strndup(name, (gsize)(eq - name)); *val = g_strdup(eq + 1); - return TRUE; - /* If the string ends after the '=' consider the value - * as invalid */ - } else { - *key = NULL; - *val = NULL; - return FALSE; } + + return TRUE; } static void event_cap (IRC_SERVER_REC *server, char *args, char *nick, char *address)