is there any other plugin which using this?
Signed-off-by: Nikola Pajkovsky npajkovs@redhat.com --- src/cli/report.c | 11 ++++------- src/include/report/event_config.h | 1 + src/lib/event_xml_parser.c | 7 +++++++ src/plugins/report_Bugzilla.xml.in | 1 + 4 files changed, 13 insertions(+), 7 deletions(-)
diff --git a/src/cli/report.c b/src/cli/report.c index 9374a3f..7866574 100644 --- a/src/cli/report.c +++ b/src/cli/report.c @@ -744,7 +744,7 @@ int report(const char *dump_dir_name, int flags) else { const char *rating_str = get_crash_item_content_or_NULL(crash_data, FILENAME_RATING); - unsigned rating = rating_str ? xatou(rating_str) : 4; + unsigned rating = rating_str ? xatou(rating_str) : 0;
/* For every reporter, ask if user really wants to report using it. */ for (GList *li = report_events; li; li = li->next) @@ -769,17 +769,14 @@ int report(const char *dump_dir_name, int flags) continue; }
- /* TODO: npajkovs; not implemented yet */ - //const char *rating_required = get_map_string_item_or_NULL(single_plugin_settings, "RatingRequired"); - //if (rating_required - // && string_to_bool(rating_required) == true - if (rating < 3) + if (config->rating_required && rating < 3) { puts(_("Reporting disabled because the backtrace is unusable"));
const char *package = get_crash_item_content_or_NULL(crash_data, FILENAME_PACKAGE); if (package && package[0]) - printf(_("Please try to install debuginfo manually using the command: "debuginfo-install %s" and try again\n"), package); + printf(_("Please try to install debuginfo manually using the" + " command: "debuginfo-install %s" and try again\n"), package);
plugins++; errors++; diff --git a/src/include/report/event_config.h b/src/include/report/event_config.h index 8dcfb9f..52b548e 100644 --- a/src/include/report/event_config.h +++ b/src/include/report/event_config.h @@ -68,6 +68,7 @@ typedef struct //char *title; //window title - not used right now, maybe the "name" is enough? //char *action;//action description to show in gui like: Upload report to the Red Hat bugzilla" char *description; // "Report to ...."/"Save to file" + int rating_required; GList *options; } event_config_t;
diff --git a/src/lib/event_xml_parser.c b/src/lib/event_xml_parser.c index 8bb8902..aea2f76 100644 --- a/src/lib/event_xml_parser.c +++ b/src/lib/event_xml_parser.c @@ -23,6 +23,7 @@ #define LABEL_ELEMENT "label" #define DESCRIPTION_ELEMENT "description" #define ALLOW_EMPTY_ELEMENT "allow-empty" +#define RATING_REQUIRED_ELEMENT "rating-required" #define OPTION_ELEMENT "option" //#define ACTION_ELEMENT "action" #define NAME_ELEMENT "name" @@ -294,6 +295,12 @@ static void text(GMarkupParseContext *context, } return; } + if (strcmp(inner_element, RATING_REQUIRED_ELEMENT) == 0) + { + VERB2 log("rating-required:'%s'", text_copy); + ui->rating_required = string_to_bool(text_copy); + return; + } } free(text_copy); } diff --git a/src/plugins/report_Bugzilla.xml.in b/src/plugins/report_Bugzilla.xml.in index 6c87ad9..f5d04d2 100644 --- a/src/plugins/report_Bugzilla.xml.in +++ b/src/plugins/report_Bugzilla.xml.in @@ -2,6 +2,7 @@ <event> <_name>Bugzilla</_name> <_description>Report to Bugzilla bug tracker</_description> + <rating-required>yes</rating-required> <options> <option type="text" name="Bugzilla_BugzillaURL"> <_label>Bugzilla URL</_label>
I propose a little different solution:
Implement option <minimal-rating>3</minimal-rating>
and then instead of hardcoding the values you can:
if (rating < config->minimal_rating) error_msg(_("Rating is too low, blablablabla...."));
On 04/06/2011 04:59 PM, Nikola Pajkovsky wrote:
is there any other plugin which using this?
Signed-off-by: Nikola Pajkovskynpajkovs@redhat.com
src/cli/report.c | 11 ++++------- src/include/report/event_config.h | 1 + src/lib/event_xml_parser.c | 7 +++++++ src/plugins/report_Bugzilla.xml.in | 1 + 4 files changed, 13 insertions(+), 7 deletions(-)
diff --git a/src/cli/report.c b/src/cli/report.c index 9374a3f..7866574 100644 --- a/src/cli/report.c +++ b/src/cli/report.c @@ -744,7 +744,7 @@ int report(const char *dump_dir_name, int flags) else { const char *rating_str = get_crash_item_content_or_NULL(crash_data, FILENAME_RATING);
unsigned rating = rating_str ? xatou(rating_str) : 4;
unsigned rating = rating_str ? xatou(rating_str) : 0; /* For every reporter, ask if user really wants to report using it. */ for (GList *li = report_events; li; li = li->next)
@@ -769,17 +769,14 @@ int report(const char *dump_dir_name, int flags) continue; }
/* TODO: npajkovs; not implemented yet */
//const char *rating_required = get_map_string_item_or_NULL(single_plugin_settings, "RatingRequired");
//if (rating_required
//&& string_to_bool(rating_required) == true
if (rating< 3)
if (config->rating_required&& rating< 3) { puts(_("Reporting disabled because the backtrace is unusable")); const char *package = get_crash_item_content_or_NULL(crash_data, FILENAME_PACKAGE); if (package&& package[0])
printf(_("Please try to install debuginfo manually using the command: \"debuginfo-install %s\" and try again\n"), package);
printf(_("Please try to install debuginfo manually using the"
" command: \"debuginfo-install %s\" and try again\n"), package); plugins++; errors++;
diff --git a/src/include/report/event_config.h b/src/include/report/event_config.h index 8dcfb9f..52b548e 100644 --- a/src/include/report/event_config.h +++ b/src/include/report/event_config.h @@ -68,6 +68,7 @@ typedef struct //char *title; //window title - not used right now, maybe the "name" is enough? //char *action;//action description to show in gui like: Upload report to the Red Hat bugzilla" char *description; // "Report to ...."/"Save to file"
- int rating_required; GList *options; } event_config_t;
diff --git a/src/lib/event_xml_parser.c b/src/lib/event_xml_parser.c index 8bb8902..aea2f76 100644 --- a/src/lib/event_xml_parser.c +++ b/src/lib/event_xml_parser.c @@ -23,6 +23,7 @@ #define LABEL_ELEMENT "label" #define DESCRIPTION_ELEMENT "description" #define ALLOW_EMPTY_ELEMENT "allow-empty" +#define RATING_REQUIRED_ELEMENT "rating-required" #define OPTION_ELEMENT "option" //#define ACTION_ELEMENT "action" #define NAME_ELEMENT "name" @@ -294,6 +295,12 @@ static void text(GMarkupParseContext *context, } return; }
if (strcmp(inner_element, RATING_REQUIRED_ELEMENT) == 0)
{
VERB2 log("rating-required:'%s'", text_copy);
ui->rating_required = string_to_bool(text_copy);
return;
}} } free(text_copy);
diff --git a/src/plugins/report_Bugzilla.xml.in b/src/plugins/report_Bugzilla.xml.in index 6c87ad9..f5d04d2 100644 --- a/src/plugins/report_Bugzilla.xml.in +++ b/src/plugins/report_Bugzilla.xml.in @@ -2,6 +2,7 @@
<event> <_name>Bugzilla</_name> <_description>Report to Bugzilla bug tracker</_description> +<rating-required>yes</rating-required> <options> <option type="text" name="Bugzilla_BugzillaURL"> <_label>Bugzilla URL</_label>
Jiri Moskovcak jmoskovc@redhat.com writes:
I propose a little different solution:
Implement option <minimal-rating>3</minimal-rating>
and then instead of hardcoding the values you can:
if (rating < config->minimal_rating) error_msg(_("Rating is too low, blablablabla...."));
Make sense. I will modify patch and resend a new proposal.
On 04/06/2011 04:59 PM, Nikola Pajkovsky wrote:
is there any other plugin which using this?
Signed-off-by: Nikola Pajkovskynpajkovs@redhat.com
src/cli/report.c | 11 ++++------- src/include/report/event_config.h | 1 + src/lib/event_xml_parser.c | 7 +++++++ src/plugins/report_Bugzilla.xml.in | 1 + 4 files changed, 13 insertions(+), 7 deletions(-)
diff --git a/src/cli/report.c b/src/cli/report.c index 9374a3f..7866574 100644 --- a/src/cli/report.c +++ b/src/cli/report.c @@ -744,7 +744,7 @@ int report(const char *dump_dir_name, int flags) else { const char *rating_str = get_crash_item_content_or_NULL(crash_data, FILENAME_RATING);
unsigned rating = rating_str ? xatou(rating_str) : 4;
unsigned rating = rating_str ? xatou(rating_str) : 0; /* For every reporter, ask if user really wants to report using it. */ for (GList *li = report_events; li; li = li->next)
@@ -769,17 +769,14 @@ int report(const char *dump_dir_name, int flags) continue; }
/* TODO: npajkovs; not implemented yet */
//const char *rating_required = get_map_string_item_or_NULL(single_plugin_settings, "RatingRequired");
//if (rating_required
//&& string_to_bool(rating_required) == true
if (rating< 3)
if (config->rating_required&& rating< 3) { puts(_("Reporting disabled because the backtrace is unusable")); const char *package = get_crash_item_content_or_NULL(crash_data, FILENAME_PACKAGE); if (package&& package[0])
printf(_("Please try to install debuginfo manually using the command: \"debuginfo-install %s\" and try again\n"), package);
printf(_("Please try to install debuginfo manually using the"
" command: \"debuginfo-install %s\" and try again\n"), package); plugins++; errors++;
diff --git a/src/include/report/event_config.h b/src/include/report/event_config.h index 8dcfb9f..52b548e 100644 --- a/src/include/report/event_config.h +++ b/src/include/report/event_config.h @@ -68,6 +68,7 @@ typedef struct //char *title; //window title - not used right now, maybe the "name" is enough? //char *action;//action description to show in gui like: Upload report to the Red Hat bugzilla" char *description; // "Report to ...."/"Save to file"
- int rating_required; GList *options; } event_config_t;
diff --git a/src/lib/event_xml_parser.c b/src/lib/event_xml_parser.c index 8bb8902..aea2f76 100644 --- a/src/lib/event_xml_parser.c +++ b/src/lib/event_xml_parser.c @@ -23,6 +23,7 @@ #define LABEL_ELEMENT "label" #define DESCRIPTION_ELEMENT "description" #define ALLOW_EMPTY_ELEMENT "allow-empty" +#define RATING_REQUIRED_ELEMENT "rating-required" #define OPTION_ELEMENT "option" //#define ACTION_ELEMENT "action" #define NAME_ELEMENT "name" @@ -294,6 +295,12 @@ static void text(GMarkupParseContext *context, } return; }
if (strcmp(inner_element, RATING_REQUIRED_ELEMENT) == 0)
{
VERB2 log("rating-required:'%s'", text_copy);
ui->rating_required = string_to_bool(text_copy);
return;
}} } free(text_copy);
diff --git a/src/plugins/report_Bugzilla.xml.in b/src/plugins/report_Bugzilla.xml.in index 6c87ad9..f5d04d2 100644 --- a/src/plugins/report_Bugzilla.xml.in +++ b/src/plugins/report_Bugzilla.xml.in @@ -2,6 +2,7 @@
<event> <_name>Bugzilla</_name> <_description>Report to Bugzilla bug tracker</_description> +<rating-required>yes</rating-required> <options> <option type="text" name="Bugzilla_BugzillaURL"> <_label>Bugzilla URL</_label>
Jiri Moskovcak jmoskovc@redhat.com writes:
I propose a little different solution:
Implement option <minimal-rating>3</minimal-rating>
and then instead of hardcoding the values you can:
if (rating < config->minimal_rating) error_msg(_("Rating is too low, blablablabla...."));
When I was on toilet, I realize, that doesn't make sense at all. See the scenario. Someone wants to report bug, but the backtrace is useless. "The bad guy" change <minimal-rating> to 0 and now "the bad guy" can successfully send totally useless backtrace to bugzilla or whatever. And we definitely don't want this.
On 04/06/2011 04:59 PM, Nikola Pajkovsky wrote:
is there any other plugin which using this?
Signed-off-by: Nikola Pajkovskynpajkovs@redhat.com
src/cli/report.c | 11 ++++------- src/include/report/event_config.h | 1 + src/lib/event_xml_parser.c | 7 +++++++ src/plugins/report_Bugzilla.xml.in | 1 + 4 files changed, 13 insertions(+), 7 deletions(-)
diff --git a/src/cli/report.c b/src/cli/report.c index 9374a3f..7866574 100644 --- a/src/cli/report.c +++ b/src/cli/report.c @@ -744,7 +744,7 @@ int report(const char *dump_dir_name, int flags) else { const char *rating_str = get_crash_item_content_or_NULL(crash_data, FILENAME_RATING);
unsigned rating = rating_str ? xatou(rating_str) : 4;
unsigned rating = rating_str ? xatou(rating_str) : 0; /* For every reporter, ask if user really wants to report using it. */ for (GList *li = report_events; li; li = li->next)
@@ -769,17 +769,14 @@ int report(const char *dump_dir_name, int flags) continue; }
/* TODO: npajkovs; not implemented yet */
//const char *rating_required = get_map_string_item_or_NULL(single_plugin_settings, "RatingRequired");
//if (rating_required
//&& string_to_bool(rating_required) == true
if (rating< 3)
if (config->rating_required&& rating< 3) { puts(_("Reporting disabled because the backtrace is unusable")); const char *package = get_crash_item_content_or_NULL(crash_data, FILENAME_PACKAGE); if (package&& package[0])
printf(_("Please try to install debuginfo manually using the command: \"debuginfo-install %s\" and try again\n"), package);
printf(_("Please try to install debuginfo manually using the"
" command: \"debuginfo-install %s\" and try again\n"), package); plugins++; errors++;
diff --git a/src/include/report/event_config.h b/src/include/report/event_config.h index 8dcfb9f..52b548e 100644 --- a/src/include/report/event_config.h +++ b/src/include/report/event_config.h @@ -68,6 +68,7 @@ typedef struct //char *title; //window title - not used right now, maybe the "name" is enough? //char *action;//action description to show in gui like: Upload report to the Red Hat bugzilla" char *description; // "Report to ...."/"Save to file"
- int rating_required; GList *options; } event_config_t;
diff --git a/src/lib/event_xml_parser.c b/src/lib/event_xml_parser.c index 8bb8902..aea2f76 100644 --- a/src/lib/event_xml_parser.c +++ b/src/lib/event_xml_parser.c @@ -23,6 +23,7 @@ #define LABEL_ELEMENT "label" #define DESCRIPTION_ELEMENT "description" #define ALLOW_EMPTY_ELEMENT "allow-empty" +#define RATING_REQUIRED_ELEMENT "rating-required" #define OPTION_ELEMENT "option" //#define ACTION_ELEMENT "action" #define NAME_ELEMENT "name" @@ -294,6 +295,12 @@ static void text(GMarkupParseContext *context, } return; }
if (strcmp(inner_element, RATING_REQUIRED_ELEMENT) == 0)
{
VERB2 log("rating-required:'%s'", text_copy);
ui->rating_required = string_to_bool(text_copy);
return;
}} } free(text_copy);
diff --git a/src/plugins/report_Bugzilla.xml.in b/src/plugins/report_Bugzilla.xml.in index 6c87ad9..f5d04d2 100644 --- a/src/plugins/report_Bugzilla.xml.in +++ b/src/plugins/report_Bugzilla.xml.in @@ -2,6 +2,7 @@
<event> <_name>Bugzilla</_name> <_description>Report to Bugzilla bug tracker</_description> +<rating-required>yes</rating-required> <options> <option type="text" name="Bugzilla_BugzillaURL"> <_label>Bugzilla URL</_label>
On Thu, 2011-04-07 at 09:34 +0200, Nikola Pajkovsky wrote:
Jiri Moskovcak jmoskovc@redhat.com writes:
I propose a little different solution:
Implement option <minimal-rating>3</minimal-rating>
and then instead of hardcoding the values you can:
if (rating < config->minimal_rating) error_msg(_("Rating is too low, blablablabla...."));
When I was on toilet, I realize, that doesn't make sense at all. See the scenario. Someone wants to report bug, but the backtrace is useless. "The bad guy" change <minimal-rating> to 0 and now "the bad guy" can successfully send totally useless backtrace to bugzilla or whatever. And we definitely don't want this.
The bad guy can always tweak "rating" element in dump dir.
The really bad guy can write a script which talks to Bugzilla and creates quadrazilloin of random bugs.
IOW: you can't defend against significantly bad guys. Therefore, we are not trying to do that. We are only trying to defend against lazy reporters which would submit useless (bad rating) backtraces - if it's easy to do, but which wouldn't bother actively trying to subvert (hack) the tool to achieve this.
On 04/07/2011 02:01 PM, Denys Vlasenko wrote:
On Thu, 2011-04-07 at 09:34 +0200, Nikola Pajkovsky wrote:
Jiri Moskovcakjmoskovc@redhat.com writes:
I propose a little different solution:
Implement option <minimal-rating>3</minimal-rating>
and then instead of hardcoding the values you can:
if (rating< config->minimal_rating) error_msg(_("Rating is too low, blablablabla...."));
When I was on toilet, I realize, that doesn't make sense at all. See the scenario. Someone wants to report bug, but the backtrace is useless. "The bad guy" change<minimal-rating> to 0 and now "the bad guy" can successfully send totally useless backtrace to bugzilla or whatever. And we definitely don't want this.
The bad guy can always tweak "rating" element in dump dir.
The really bad guy can write a script which talks to Bugzilla and creates quadrazilloin of random bugs.
- or just download the abrt source and remove one line with the condition which checks the backtrace and build his own packages ;)
IOW: you can't defend against significantly bad guys. Therefore, we are not trying to do that. We are only trying to defend against lazy reporters which would submit useless (bad rating) backtraces - if it's easy to do, but which wouldn't bother actively trying to subvert (hack) the tool to achieve this.
Jiri Moskovcak jmoskovc@redhat.com writes:
On 04/07/2011 02:01 PM, Denys Vlasenko wrote:
On Thu, 2011-04-07 at 09:34 +0200, Nikola Pajkovsky wrote:
Jiri Moskovcakjmoskovc@redhat.com writes:
I propose a little different solution:
Implement option <minimal-rating>3</minimal-rating>
and then instead of hardcoding the values you can:
if (rating< config->minimal_rating) error_msg(_("Rating is too low, blablablabla...."));
When I was on toilet, I realize, that doesn't make sense at all. See the scenario. Someone wants to report bug, but the backtrace is useless. "The bad guy" change<minimal-rating> to 0 and now "the bad guy" can successfully send totally useless backtrace to bugzilla or whatever. And we definitely don't want this.
The bad guy can always tweak "rating" element in dump dir.
The really bad guy can write a script which talks to Bugzilla and creates quadrazilloin of random bugs.
- or just download the abrt source and remove one line with the
condition which checks the backtrace and build his own packages ;)
yea I know ;)
IOW: you can't defend against significantly bad guys. Therefore, we are not trying to do that. We are only trying to defend against lazy reporters which would submit useless (bad rating) backtraces - if it's easy to do, but which wouldn't bother actively trying to subvert (hack) the tool to achieve this.
I'm a little thickheaded today. What do you want to see in abrt? Hardcoded trashhold with rating-required or minimal-rating?
On Thu, 2011-04-07 at 14:38 +0200, Nikola Pajkovsky wrote:
Jiri Moskovcak jmoskovc@redhat.com writes:
On 04/07/2011 02:01 PM, Denys Vlasenko wrote:
On Thu, 2011-04-07 at 09:34 +0200, Nikola Pajkovsky wrote:
Jiri Moskovcakjmoskovc@redhat.com writes:
I propose a little different solution:
Implement option <minimal-rating>3</minimal-rating>
and then instead of hardcoding the values you can:
if (rating< config->minimal_rating) error_msg(_("Rating is too low, blablablabla...."));
When I was on toilet, I realize, that doesn't make sense at all. See the scenario. Someone wants to report bug, but the backtrace is useless. "The bad guy" change<minimal-rating> to 0 and now "the bad guy" can successfully send totally useless backtrace to bugzilla or whatever. And we definitely don't want this.
The bad guy can always tweak "rating" element in dump dir.
The really bad guy can write a script which talks to Bugzilla and creates quadrazilloin of random bugs.
- or just download the abrt source and remove one line with the
condition which checks the backtrace and build his own packages ;)
yea I know ;)
IOW: you can't defend against significantly bad guys. Therefore, we are not trying to do that. We are only trying to defend against lazy reporters which would submit useless (bad rating) backtraces - if it's easy to do, but which wouldn't bother actively trying to subvert (hack) the tool to achieve this.
I'm a little thickheaded today. What do you want to see in abrt? Hardcoded trashhold with rating-required or minimal-rating?
Jiri proposed minimal-rating thing. I don't have a strong opinion on this detail.
crash-catcher@lists.fedorahosted.org