[plaso] Changes to make syslog parser more error tolerant #1142

[plaso] Changes to make syslog parser more error tolerant #1142

[Joachim Metz, 2017-01-22 11:33:48]

Code updated.

[onager, 2017-01-24 14:48:29]

https://codereview.appspot.com/320020043/diff/20001/plaso/parsers/text_parser.py
File plaso/parsers/text_parser.py (right):

https://codereview.appspot.com/320020043/diff/20001/plaso/parsers/text_parser...
plaso/parsers/text_parser.py:742: u'unable parse record with error:
{0:s}'.format(exception))
Please include the key in the error message to improve debugging.

https://codereview.appspot.com/320020043/diff/20001/plaso/parsers/text_parser...
plaso/parsers/text_parser.py:760: @abc.abstractmethod
These are already defined as abstract in the parent class. If you need to
redefine them (linter?) then please use the docstrings from the parent class,
not from syslog.

[onager, 2017-01-24 16:23:46]

https://codereview.appspot.com/320020043/diff/20001/tests/parsers/syslog.py
File tests/parsers/syslog.py (left):

https://codereview.appspot.com/320020043/diff/20001/tests/parsers/syslog.py#o...
tests/parsers/syslog.py:22: self.plugins = [plugin for _, plugin in
list(self._parser.GetPlugins())]
Why are you leaving all the plugins enabled for this test? It means that a
broken plugin is likely to cause this test to fail too, making debugging
unnecessarily complex.

[Joachim Metz, 2017-01-24 21:48:29]

https://codereview.appspot.com/320020043/diff/20001/tests/parsers/syslog.py
File tests/parsers/syslog.py (left):

https://codereview.appspot.com/320020043/diff/20001/tests/parsers/syslog.py#o...
tests/parsers/syslog.py:22: self.plugins = [plugin for _, plugin in
list(self._parser.GetPlugins())]
what about having:
syslog.SyslogParser(disable_plugins=True)

[Joachim Metz, 2017-01-25 06:37:58]

https://codereview.appspot.com/320020043/diff/20001/plaso/parsers/text_parser.py
File plaso/parsers/text_parser.py (right):

https://codereview.appspot.com/320020043/diff/20001/plaso/parsers/text_parser...
plaso/parsers/text_parser.py:742: u'unable parse record with error:
{0:s}'.format(exception))
On 2017/01/24 14:48:29, onager wrote:
> Please include the key in the error message to improve debugging.

Done.

https://codereview.appspot.com/320020043/diff/20001/plaso/parsers/text_parser...
plaso/parsers/text_parser.py:760: @abc.abstractmethod
On 2017/01/24 14:48:29, onager wrote:
> These are already defined as abstract in the parent class. If you need to
> redefine them (linter?) then please use the docstrings from the parent class,
> not from syslog.

Acknowledged.

https://codereview.appspot.com/320020043/diff/20001/tests/parsers/syslog.py
File tests/parsers/syslog.py (left):

https://codereview.appspot.com/320020043/diff/20001/tests/parsers/syslog.py#o...
tests/parsers/syslog.py:22: self.plugins = [plugin for _, plugin in
list(self._parser.GetPlugins())]
Also since syslog_parser.EnablePlugins() is not called in the is this
setup/teardown needed?

[Joachim Metz, 2017-01-25 06:41:26]

Code updated.

[onager, 2017-01-26 14:11:57]

One small nit in the error message, LGTM

https://codereview.appspot.com/320020043/diff/40001/plaso/parsers/text_parser.py
File plaso/parsers/text_parser.py (right):

https://codereview.appspot.com/320020043/diff/40001/plaso/parsers/text_parser...
plaso/parsers/text_parser.py:742: u'unable parse record: {0:s} with error:
{1:s}'.format(
parse record with key: {0:s}

The record is the text in the file, the key is the identifier for the code
trying to parse the record.

[Joachim Metz, 2017-01-27 04:46:28]

https://codereview.appspot.com/320020043/diff/40001/plaso/parsers/text_parser.py
File plaso/parsers/text_parser.py (right):

https://codereview.appspot.com/320020043/diff/40001/plaso/parsers/text_parser...
plaso/parsers/text_parser.py:742: u'unable parse record: {0:s} with error:
{1:s}'.format(
IMO emphasizing this in the error message is not needed.

[Joachim Metz, 2017-01-27 04:49:53]

Changes have been merged with master branch. To close the review and clean up
the feature branch you can run: python ./utils/review.py close syslog