FILENAME_BACKTRACE and FILENAME_EXECUTABLE make no sense for rasdaemon's ECC memory errors reporting - we shouldn't require those. Also, we should allow requests with only "type=..." but without "analyzer=..." (a step for analyzer -> type transition). While at it, helped abrt-dump-xorg to save "type" item too.
Bug fix: PUT is the _wrong type of request for creation of new entities_! Added POST as an alias.
Updated abrt-server.txt to show POST and lowercased item names. Similarly updated python crash reporter module.
Signed-off-by: Denys Vlasenko dvlasenk@redhat.com --- doc/abrt-server.txt | 20 ++++++------- src/daemon/abrt-server.c | 52 ++++++++++++++++++++++++---------- src/hooks/abrt_exception_handler.py.in | 13 ++++----- src/plugins/abrt-dump-xorg.c | 1 + 4 files changed, 53 insertions(+), 33 deletions(-)
diff --git a/doc/abrt-server.txt b/doc/abrt-server.txt index 8e9958b..bf9b40d 100644 --- a/doc/abrt-server.txt +++ b/doc/abrt-server.txt @@ -38,22 +38,20 @@ connect to UNIX domain socket /var/run/abrt.socket Providing data (writting data to the socket):
------------------------------------------------- --> "PUT / HTTP/1.1\r\n" +-> "POST / HTTP/1.1\r\n" -> "\r\n" --> "PID=number\0" +-> "type=string\0" + string, maximum length 100 bytes +-> "reason=string\0" + string, maximum length 512 bytes +-> "pid=number\0" number, 0 - PID_MAX (/proc/sys/kernel/pid_max) --> "EXECUTABLE=string\0" +-> "executable=string\0" string, maximum length ~MAX_PATH --> "BACKTRACE=string\0" +-> "backtrace=string\0" string, maximum length 1 MB --> "ANALYZER=string\0" - string, maximum length 100 bytes --> "BASENAME=string\0" - string maximum length 100 bytes, no slashes --> "REASON=string\0" - string, maximum length 512 bytes -> (close writing half of the socket) -<- "HTTP/1.1 200 \r\n" +<- "HTTP/1.1 201 \r\n" <- "\r\n" -------------------------------------------------
diff --git a/src/daemon/abrt-server.c b/src/daemon/abrt-server.c index 98d6db0..14c2f69 100644 --- a/src/daemon/abrt-server.c +++ b/src/daemon/abrt-server.c @@ -89,7 +89,7 @@ static int create_problem_dir(GHashTable *problem_info, unsigned pid)
gchar *dir_basename = g_hash_table_lookup(problem_info, "basename"); if (!dir_basename) - dir_basename = g_hash_table_lookup(problem_info, FILENAME_ANALYZER); + dir_basename = g_hash_table_lookup(problem_info, FILENAME_TYPE);
char *path = xasprintf("%s/%s-%s-%u.new", g_settings_dump_location, @@ -232,7 +232,7 @@ static gboolean key_value_ok(gchar *key, gchar *value) /* check value of 'basename', it has to be valid non-hidden directory * name */ if (strcmp(key, "basename") == 0 - || strcmp(key, FILENAME_ANALYZER) == 0 + || strcmp(key, FILENAME_TYPE) == 0 ) { if (!is_correct_filename(value)) @@ -268,7 +268,11 @@ static void process_message(GHashTable *problem_info, char *message) else { g_hash_table_insert(problem_info, key, xstrdup(value)); - key = NULL; /* prevent freeing later */ + /* Compat, delete when FILENAME_ANALYZER is replaced by FILENAME_TYPE: */ + if (strcmp(key, FILENAME_TYPE) == 0) + g_hash_table_insert(problem_info, xstrdup(FILENAME_ANALYZER), xstrdup(value)); + /* Prevent freeing key later: */ + key = NULL; } } else @@ -289,11 +293,13 @@ static void die_if_data_is_missing(GHashTable *problem_info) { gboolean missing_data = FALSE; gchar **pstring; - static const gchar *const needed[] = {FILENAME_ANALYZER, - FILENAME_BACKTRACE, - FILENAME_EXECUTABLE, - FILENAME_REASON, - NULL}; + static const gchar *const needed[] = { + FILENAME_TYPE, + FILENAME_REASON, + /* FILENAME_BACKTRACE, - ECC errors have no such elements */ + /* FILENAME_EXECUTABLE, */ + NULL + };
for (pstring = (gchar**) needed; *pstring; pstring++) { @@ -408,8 +414,14 @@ static int perform_http_xact(void) return delete_path(messagebuf_data); }
- if (strncmp(messagebuf_data, "PUT ", strlen("PUT ")) != 0) - { + /* We erroneously used "PUT /" to create new problems. + * POST is the correct request in this case: + * "PUT /" implies creation or replace of resource named "/"! + * Delete PUT in 2014. + */ + if (strncmp(messagebuf_data, "PUT ", strlen("PUT ")) != 0 + && strncmp(messagebuf_data, "POST ", strlen("POST ")) != 0 + ) { return 400; /* Bad Request */; }
@@ -464,11 +476,21 @@ static int perform_http_xact(void) die_if_data_is_missing(problem_info);
char *executable = g_hash_table_lookup(problem_info, FILENAME_EXECUTABLE); - char *last_file = concat_path_file(g_settings_dump_location, "last-via-server"); - int repeating_crash = check_recent_crash_file(last_file, executable); - free(last_file); - if (repeating_crash) /* Only pretend that we saved it */ - goto out; /* ret is 0: "success" */ + if (executable) + { + char *last_file = concat_path_file(g_settings_dump_location, "last-via-server"); + int repeating_crash = check_recent_crash_file(last_file, executable); + free(last_file); + if (repeating_crash) /* Only pretend that we saved it */ + goto out; /* ret is 0: "success" */ + } + +#if 0 +//TODO: + /* At least it should generate local problem identifier UUID */ + problem_data_add_basics(problem_info); +//...the problem being that problem_info here is not a problem_data_t! +#endif
ret = create_problem_dir(problem_info, pid);
diff --git a/src/hooks/abrt_exception_handler.py.in b/src/hooks/abrt_exception_handler.py.in index 710ebdc..774a586 100644 --- a/src/hooks/abrt_exception_handler.py.in +++ b/src/hooks/abrt_exception_handler.py.in @@ -53,16 +53,15 @@ def write_dump(tb_text, tb): s.settimeout(5) try: s.connect(@VAR_RUN@ + "/abrt/abrt.socket") - s.sendall("PUT / HTTP/1.1\r\n\r\n") - s.sendall("PID=%s\0" % os.getpid()) - s.sendall("EXECUTABLE=%s\0" % executable) - s.sendall("ANALYZER=Python\0") - s.sendall("BASENAME=pyhook\0") + s.sendall("POST / HTTP/1.1\r\n\r\n") + s.sendall("type=Python\0") + s.sendall("pid=%s\0" % os.getpid()) + s.sendall("executable=%s\0" % executable) # This handler puts a short(er) crash descr in 1st line of the backtrace. # Example: # CCMainWindow.py:1:<module>:ZeroDivisionError: integer division or modulo by zero - s.sendall("REASON=%s\0" % tb_text.splitlines()[0]) - s.sendall("BACKTRACE=%s\0" % tb_text) + s.sendall("reason=%s\0" % tb_text.splitlines()[0]) + s.sendall("backtrace=%s\0" % tb_text)
if dso_list: s.sendall("dso_list=%s\0" % "\n".join(dso_list)) diff --git a/src/plugins/abrt-dump-xorg.c b/src/plugins/abrt-dump-xorg.c index 5e84f81..e3d6b0a 100644 --- a/src/plugins/abrt-dump-xorg.c +++ b/src/plugins/abrt-dump-xorg.c @@ -100,6 +100,7 @@ static void save_bt_to_dump_dir(const char *bt, const char *exe, const char *rea dd_create_basic_files(dd, /*uid:*/ my_euid, NULL); dd_save_text(dd, FILENAME_ABRT_VERSION, VERSION); dd_save_text(dd, FILENAME_ANALYZER, "xorg"); + dd_save_text(dd, FILENAME_TYPE, "xorg"); dd_save_text(dd, FILENAME_REASON, reason); dd_save_text(dd, FILENAME_BACKTRACE, bt); /*
crash-catcher@lists.fedorahosted.org