[ABRT PATCH] abrt-server: update for future rasdaemon needs + bugfixes

Denys Vlasenko dvlasenk at redhat.com
Tue Jun 18 13:28:44 UTC 2013


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 at 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);
         /*
-- 
1.8.1.4



More information about the Crash-catcher mailing list