Bug 694301 - ABRT2: report_Logger configurator accepts arbitrary text in Append field
Summary: ABRT2: report_Logger configurator accepts arbitrary text in Append field
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: abrt
Version: 15
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Denys Vlasenko
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2011-04-06 21:38 UTC by Steve Tyler
Modified: 2011-10-11 09:40 UTC (History)
7 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2011-10-11 09:40:37 UTC
Type: ---


Attachments (Terms of Use)

Description Steve Tyler 2011-04-06 21:38:14 UTC
Description of problem:
The Append field of the report_Logger configuration is a text input box. This box accepts arbitrary text instead of "yes" or "no".

Version-Release number of selected component (if applicable):
abrt-2.0.0-4.fc15.x86_64

How reproducible:
Always.

Steps to Reproduce:
1. Select Edit:Event configuration:report_Logger.
2. Click "Configure Event".
3. Type "foobar" into the Append field.
4. Click "Close".
  
Actual results:
The GUI accepts this string without complaint.
The user cannot know whether appending is enabled or not.

Expected results:
The user is presented with an Append checkbox instead of a text input box.

Additional info:
I attempted to crash the GUI by pasting a huge text string into the Append field. It did not crash, and the string was truncated to 65535 characters.

Comment 1 Steve Tyler 2011-04-06 21:59:04 UTC
abrt-action-print also accepts an arbitrary string for the "-a" (append) option:

$ /usr/bin/abrt-action-print -d ccpp-2011-03-29-13:47:38-2362/ -o x1.log -a foobar
The report was stored to x1.log

$ /usr/bin/abrt-action-print --help
Usage: abrt-action-print [-v] -d DIR [-o FILE] [-a yes/no]

Prints problem information to standard output or FILE

    -v, --verbose         Be verbose
    -d DIR                Dump directory
    -o FILE               Output file
    -a yes/no             Append to, or overwrite FILE


abrt-plugin-logger-2.0.0-4.fc15.x86_64

Comment 2 Jiri Moskovcak 2011-04-08 10:53:25 UTC
That's because the report_Logger event is missing the xml description which defines the option types. Without the xml is not possible to determine the option type, so ABRT just creates this simple window which is accepting an arbitrary text...

Comment 3 Denys Vlasenko 2011-04-08 14:24:15 UTC
We want to keep Logger as an example of a .xml-less reporter (one which only has a .conf file but not .xml file in /etc/abrt/events).

I propose to not fix this.

Comment 4 Steve Tyler 2011-04-08 14:35:17 UTC
(In reply to comment #3)
> We want to keep Logger as an example of a .xml-less reporter (one which only
> has a .conf file but not .xml file in /etc/abrt/events).
> 
> I propose to not fix this.

An "example" reporter sounds like a good idea.
But does it have to be identical to a "deployed" reporter?
They could be derived from the same source, perhaps.

Comment 5 Steve Tyler 2011-04-08 14:44:11 UTC
abrt-devel appears to already have an example reporter ...

$ rpm -ql abrt-devel
/usr/include/abrt
/usr/include/abrt/abrt_exception.h
/usr/include/abrt/abrt_types.h
/usr/include/abrt/abrt_xmlrpc.h
/usr/include/abrt/abrtlib.h
/usr/include/abrt/action.h
/usr/include/abrt/analyzer.h
/usr/include/abrt/comm_layer_inner.h
/usr/include/abrt/crash_types.h
/usr/include/abrt/database.h
/usr/include/abrt/dbus_common.h
/usr/include/abrt/debug_dump.h
/usr/include/abrt/observer.h
/usr/include/abrt/plugin.h
/usr/include/abrt/reporter.h
/usr/include/abrt/xfuncs.h
/usr/lib64/libABRTUtils.so
/usr/lib64/libABRTdUtils.so
/usr/lib64/libbtparser.so
/usr/lib64/pkgconfig/abrt.pc
/usr/share/doc/abrt-devel-1.1.17
/usr/share/doc/abrt-devel-1.1.17/abrt-plugin
/usr/share/doc/abrt-devel-1.1.17/abrt-plugin/abrt-plugin
/usr/share/doc/abrt-devel-1.1.17/abrt-plugin/abrt-plugin/HelloWorld.conf
/usr/share/doc/abrt-devel-1.1.17/abrt-plugin/abrt-plugin/Makefile
/usr/share/doc/abrt-devel-1.1.17/abrt-plugin/abrt-plugin/abrt-reporter-hello-world.cpp
/usr/share/doc/abrt-devel-1.1.17/abrt-plugin/abrt-plugin/abrt-reporter-hello-world.h
/usr/share/doc/abrt-devel-1.1.17/howto-write-reporter

Comment 6 Denys Vlasenko 2011-05-17 16:28:22 UTC
(In reply to comment #4)
> (In reply to comment #3)
> > We want to keep Logger as an example of a .xml-less reporter (one which only
> > has a .conf file but not .xml file in /etc/abrt/events).
> > 
> > I propose to not fix this.
> 
> An "example" reporter sounds like a good idea.
> But does it have to be identical to a "deployed" reporter?
> They could be derived from the same source, perhaps.

If the reporter will not be installed, it will receive much less testing.

Considering that Logger is not only an example (that it may be useful for real world cases), I am trying to kill two birds with one stone here:
(1) have a reporter which saves to a file
(2) have a .xml-less reporter, to make sure this feature hasn't bit rotted.

Here the solution may be to make boolean params look nicer even in non-xml reporters.

Comment 7 Steve Tyler 2011-05-17 20:34:05 UTC
(In reply to comment #6)
> (In reply to comment #4)
> > (In reply to comment #3)
> > > We want to keep Logger as an example of a .xml-less reporter (one which only
> > > has a .conf file but not .xml file in /etc/abrt/events).
> > > 
> > > I propose to not fix this.
> > 
> > An "example" reporter sounds like a good idea.
> > But does it have to be identical to a "deployed" reporter?
> > They could be derived from the same source, perhaps.
> 
> If the reporter will not be installed, it will receive much less testing.
> 
> Considering that Logger is not only an example (that it may be useful for real
> world cases), I am trying to kill two birds with one stone here:
> (1) have a reporter which saves to a file
> (2) have a .xml-less reporter, to make sure this feature hasn't bit rotted.
> 
> Here the solution may be to make boolean params look nicer even in non-xml
> reporters.

How would you do that?

All I can think of is:
"Append?"
"y*":  yes
"n*":  no
other: report error

That's more of a CLI implementation than a GUI implementation, but it would fix this bug ...

Comment 8 Denys Vlasenko 2011-10-11 09:40:37 UTC
We have /etc/libreport/events/report_Logger.xml now, which has this:

<option type="bool" name="Logger_Append">

Thus, GUI shows a checkbox, not a text field now. Tested.


Note You need to log in before you can comment on or make changes to this bug.