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).
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.
Signed-off-by: Denys Vlasenko dvlasenk@redhat.com --- doc/abrt-server.txt | 20 +++++++++---------- src/daemon/abrt-server.c | 52 ++++++++++++++++++++++++++++++++++-------------- 2 files changed, 46 insertions(+), 26 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);
Make python hook use POST ionstead of PUT, send lowercased item names (we might drop automatic lowercasing in the server later), send "type" instead of "analyzer", and not send "basename" pseudo-item (server will use type instead).
Signed-off-by: Denys Vlasenko dvlasenk@redhat.com --- src/hooks/abrt_exception_handler.py.in | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-)
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))
Signed-off-by: Denys Vlasenko dvlasenk@redhat.com --- src/plugins/abrt-dump-xorg.c | 1 + 1 file changed, 1 insertion(+)
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); /*
Pushed all. Thanks!
I've added the bugzilla ID (812539) to all of the patches.
The concerns about reporting from anaconda were pointless. We provide anaconda's reporting configuration and we don't use analyzer there.
On Thu, 2013-06-20 at 16:37 +0200, Denys Vlasenko wrote:
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).
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.
Signed-off-by: Denys Vlasenko dvlasenk@redhat.com
doc/abrt-server.txt | 20 +++++++++---------- src/daemon/abrt-server.c | 52 ++++++++++++++++++++++++++++++++++-------------- 2 files changed, 46 insertions(+), 26 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
) { if (!is_correct_filename(value))|| strcmp(key, FILENAME_TYPE) == 0
@@ -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);
crash-catcher@lists.fedorahosted.org