Issue 310600043: [plaso] Add systemd journal parser (Closed)
|
Created:
7 years, 6 months ago by romaing Modified:
7 years ago CC:
kiddi, log2timeline-dev_googlegroups.com Visibility:
Public. |
Description[plaso] Add systemd journal parser
Patch Set 1 #
Total comments: 64
Patch Set 2 : Update accordingly #Patch Set 3 : Rebased to master #
Total comments: 34
Patch Set 4 : Implement comment fixes #Patch Set 5 : move to python's lzma library, also properly test compressed messages #Patch Set 6 : Skip systemd_journal tests if python-lzma is not installed #Patch Set 7 : Fix skip test string message #MessagesTotal messages: 20
Initial nits https://codereview.appspot.com/310600043/diff/1/plaso/formatters/__init__.py File plaso/formatters/__init__.py (right): https://codereview.appspot.com/310600043/diff/1/plaso/formatters/__init__.py#... plaso/formatters/__init__.py:68: from plaso.formatters import ssh please move ssh into alphabetical order https://codereview.appspot.com/310600043/diff/1/plaso/formatters/systemd_jour... File plaso/formatters/systemd_journal.py (right): https://codereview.appspot.com/310600043/diff/1/plaso/formatters/systemd_jour... plaso/formatters/systemd_journal.py:6: + white line https://codereview.appspot.com/310600043/diff/1/plaso/formatters/systemd_jour... plaso/formatters/systemd_journal.py:22: u'{_HOSTNAME} ', _HOSTNAME => hostname lower case and use public attribute container names. https://codereview.appspot.com/310600043/diff/1/plaso/parsers/systemd_journal.py File plaso/parsers/systemd_journal.py (right): https://codereview.appspot.com/310600043/diff/1/plaso/parsers/systemd_journal... plaso/parsers/systemd_journal.py:6: from dfvfs.compression.xz_decompressor import XZDecompressor + white line after third party imports also the XZ decompressor works optionally at the moment let's sync about this before commit. https://codereview.appspot.com/310600043/diff/1/plaso/parsers/systemd_journal... plaso/parsers/systemd_journal.py:11: + white line https://codereview.appspot.com/310600043/diff/1/plaso/parsers/systemd_journal... plaso/parsers/systemd_journal.py:101: construct.Struct(u'boot_id', To reduce whitespace I opt: construct.Struct( u'boot_id', ... https://codereview.appspot.com/310600043/diff/1/plaso/parsers/systemd_journal... plaso/parsers/systemd_journal.py:145: - white line https://codereview.appspot.com/310600043/diff/1/plaso/parsers/systemd_journal... plaso/parsers/systemd_journal.py:146: def __init__(self): + doc string https://codereview.appspot.com/310600043/diff/1/plaso/parsers/systemd_journal... plaso/parsers/systemd_journal.py:150: self._max_journal_file_offset = 0 I opt to put protected "self._z" attributes before public "self.z" https://codereview.appspot.com/310600043/diff/1/plaso/parsers/systemd_journal... plaso/parsers/systemd_journal.py:159: tuple(_OBJECT_HEADER,int): the parsed object header and size of the tuple[_OBJECT_HEADER, int]: https://codereview.appspot.com/310600043/diff/1/plaso/parsers/systemd_journal... plaso/parsers/systemd_journal.py:165: - white line https://codereview.appspot.com/310600043/diff/1/plaso/parsers/systemd_journal... plaso/parsers/systemd_journal.py:182: tuple(str,str): the key and value of this item. tuple(str,str) => tuple[str, str]: and -the https://codereview.appspot.com/310600043/diff/1/plaso/parsers/systemd_journal... plaso/parsers/systemd_journal.py:185: SystemdJournalParseException: When an unexpected object type is parsed. Why not raise the generic parser error ? https://codereview.appspot.com/310600043/diff/1/plaso/parsers/systemd_journal... plaso/parsers/systemd_journal.py:187: - white line https://codereview.appspot.com/310600043/diff/1/plaso/parsers/systemd_journal... plaso/parsers/systemd_journal.py:189: _ = self.journal_file.read(self._DATA_OBJECT_SIZE) just remove "_ = " https://codereview.appspot.com/310600043/diff/1/plaso/parsers/systemd_journal... plaso/parsers/systemd_journal.py:191: if object_header.type == u'DATA': I opt to use a less nested approach if object_header.type != u'DATA': raise() ... https://codereview.appspot.com/310600043/diff/1/plaso/parsers/systemd_journal... plaso/parsers/systemd_journal.py:195: event_data = xzd.Decompress(event_data)[0] event_data, _ = xzd.Decompress(event_data) https://codereview.appspot.com/310600043/diff/1/plaso/parsers/systemd_journal... plaso/parsers/systemd_journal.py:209: parser_mediator: A parser mediator object (instance of ParserMediator). parser_mediator (ParserMediator): parser mediator. https://codereview.appspot.com/310600043/diff/1/plaso/parsers/systemd_journal... plaso/parsers/systemd_journal.py:215: - white line https://codereview.appspot.com/310600043/diff/1/plaso/parsers/systemd_journal... plaso/parsers/systemd_journal.py:222: if object_header.type == u'ENTRY': if object_header.type != u'ENTRY': raise() ... https://codereview.appspot.com/310600043/diff/1/plaso/parsers/systemd_journal... plaso/parsers/systemd_journal.py:238: if u'SYSLOG_IDENTIFIER' in fields: use x = field.get(u'SYSLOG_IDENTIFIER', None) to have a single look up https://codereview.appspot.com/310600043/diff/1/plaso/parsers/systemd_journal... plaso/parsers/systemd_journal.py:263: if object_header.type == u'ENTRY_ARRAY': if object_header.type != u'ENTRY_ARRAY': raise() ... https://codereview.appspot.com/310600043/diff/1/tests/parsers/systemd_journal.py File tests/parsers/systemd_journal.py (right): https://codereview.appspot.com/310600043/diff/1/tests/parsers/systemd_journal... tests/parsers/systemd_journal.py:11: + white line https://codereview.appspot.com/310600043/diff/1/tests/parsers/systemd_journal... tests/parsers/systemd_journal.py:13: """ Tests for the Systemd Journal parser.""" """ Tests => """Tests https://codereview.appspot.com/310600043/diff/1/tests/parsers/systemd_journal... tests/parsers/systemd_journal.py:18: journal = self._ParseFile([u'systemd', u'journal', u'system.journal'], break after [ to prevent code clutter on the right journal = self._ParseFile([ ... (repeat this below as well) https://codereview.appspot.com/310600043/diff/1/tests/parsers/systemd_journal... tests/parsers/systemd_journal.py:31: expected_msg = (u'test-VirtualBox systemd-journald[577] Runtime journal ' please don't use abbreviations expected_msg => expected_message https://codereview.appspot.com/310600043/diff/1/tests/parsers/systemd_journal... tests/parsers/systemd_journal.py:45: self.assertEqual( these 2 lines will fit on 1 line https://codereview.appspot.com/310600043/diff/1/tests/parsers/systemd_journal... tests/parsers/systemd_journal.py:48: event_object = journal.events[0] event_object => event
Sign in to reply to this message.
https://codereview.appspot.com/310600043/diff/1/plaso/formatters/systemd_jour... File plaso/formatters/systemd_journal.py (right): https://codereview.appspot.com/310600043/diff/1/plaso/formatters/systemd_jour... plaso/formatters/systemd_journal.py:18: # for the system, and hopefully more unique that the _HOSTNAME field. -that +than https://codereview.appspot.com/310600043/diff/1/plaso/parsers/systemd_journal.py File plaso/parsers/systemd_journal.py (right): https://codereview.appspot.com/310600043/diff/1/plaso/parsers/systemd_journal... plaso/parsers/systemd_journal.py:41: # A file can be in any of these state and still be corrupted, for example, by states https://codereview.appspot.com/310600043/diff/1/plaso/parsers/systemd_journal... plaso/parsers/systemd_journal.py:276: 'Expected an object of type ENTRY_ARRAY, but got {0:s}'.format( +u https://codereview.appspot.com/310600043/diff/1/plaso/parsers/systemd_journal... plaso/parsers/systemd_journal.py:311: break Why the different behaviour between the two error states? Is there comment here correct? It looks like parsing continues...
Sign in to reply to this message.
Code updated.
Sign in to reply to this message.
https://codereview.appspot.com/310600043/diff/1/plaso/formatters/__init__.py File plaso/formatters/__init__.py (right): https://codereview.appspot.com/310600043/diff/1/plaso/formatters/__init__.py#... plaso/formatters/__init__.py:68: from plaso.formatters import ssh On 2016/10/26 19:08:33, Joachim Metz wrote: > please move ssh into alphabetical order Done. https://codereview.appspot.com/310600043/diff/1/plaso/formatters/systemd_jour... File plaso/formatters/systemd_journal.py (right): https://codereview.appspot.com/310600043/diff/1/plaso/formatters/systemd_jour... plaso/formatters/systemd_journal.py:6: On 2016/10/26 19:08:33, Joachim Metz wrote: > + white line Done. https://codereview.appspot.com/310600043/diff/1/plaso/formatters/systemd_jour... plaso/formatters/systemd_journal.py:18: # for the system, and hopefully more unique that the _HOSTNAME field. On 2016/11/01 14:53:31, onager wrote: > -that +than Done. https://codereview.appspot.com/310600043/diff/1/plaso/formatters/systemd_jour... plaso/formatters/systemd_journal.py:22: u'{_HOSTNAME} ', On 2016/10/26 19:08:33, Joachim Metz wrote: > _HOSTNAME => hostname > > lower case and use public attribute container names. _HOSTNAME is the name of the "key" in the journal event, in the dict that is passed to text_events.TextEvent's init (and then becomes an attribute). https://codereview.appspot.com/310600043/diff/1/plaso/parsers/systemd_journal.py File plaso/parsers/systemd_journal.py (right): https://codereview.appspot.com/310600043/diff/1/plaso/parsers/systemd_journal... plaso/parsers/systemd_journal.py:6: from dfvfs.compression.xz_decompressor import XZDecompressor On 2016/10/26 19:08:33, Joachim Metz wrote: > + white line after third party imports > > also the XZ decompressor works optionally at the moment let's sync about this > before commit. Acknowledged. https://codereview.appspot.com/310600043/diff/1/plaso/parsers/systemd_journal... plaso/parsers/systemd_journal.py:6: from dfvfs.compression.xz_decompressor import XZDecompressor On 2016/10/26 19:08:33, Joachim Metz wrote: > + white line after third party imports > > also the XZ decompressor works optionally at the moment let's sync about this > before commit. Acknowledged. https://codereview.appspot.com/310600043/diff/1/plaso/parsers/systemd_journal... plaso/parsers/systemd_journal.py:11: On 2016/10/26 19:08:33, Joachim Metz wrote: > + white line Done. https://codereview.appspot.com/310600043/diff/1/plaso/parsers/systemd_journal... plaso/parsers/systemd_journal.py:41: # A file can be in any of these state and still be corrupted, for example, by On 2016/11/01 14:53:31, onager wrote: > states Done. https://codereview.appspot.com/310600043/diff/1/plaso/parsers/systemd_journal... plaso/parsers/systemd_journal.py:101: construct.Struct(u'boot_id', On 2016/10/26 19:08:33, Joachim Metz wrote: > To reduce whitespace I opt: > > construct.Struct( > u'boot_id', > ... Done. https://codereview.appspot.com/310600043/diff/1/plaso/parsers/systemd_journal... plaso/parsers/systemd_journal.py:145: On 2016/10/26 19:08:34, Joachim Metz wrote: > - white line Done. https://codereview.appspot.com/310600043/diff/1/plaso/parsers/systemd_journal... plaso/parsers/systemd_journal.py:146: def __init__(self): On 2016/10/26 19:08:33, Joachim Metz wrote: > + doc string Done. https://codereview.appspot.com/310600043/diff/1/plaso/parsers/systemd_journal... plaso/parsers/systemd_journal.py:150: self._max_journal_file_offset = 0 On 2016/10/26 19:08:33, Joachim Metz wrote: > I opt to put protected "self._z" attributes before public "self.z" Done. https://codereview.appspot.com/310600043/diff/1/plaso/parsers/systemd_journal... plaso/parsers/systemd_journal.py:159: tuple(_OBJECT_HEADER,int): the parsed object header and size of the On 2016/10/26 19:08:34, Joachim Metz wrote: > tuple[_OBJECT_HEADER, int]: Done. https://codereview.appspot.com/310600043/diff/1/plaso/parsers/systemd_journal... plaso/parsers/systemd_journal.py:165: On 2016/10/26 19:08:33, Joachim Metz wrote: > - white line Done. https://codereview.appspot.com/310600043/diff/1/plaso/parsers/systemd_journal... plaso/parsers/systemd_journal.py:182: tuple(str,str): the key and value of this item. On 2016/10/26 19:08:33, Joachim Metz wrote: > tuple(str,str) => tuple[str, str]: > and -the Done. https://codereview.appspot.com/310600043/diff/1/plaso/parsers/systemd_journal... plaso/parsers/systemd_journal.py:185: SystemdJournalParseException: When an unexpected object type is parsed. On 2016/10/26 19:08:33, Joachim Metz wrote: > Why not raise the generic parser error ? The only reason I can think of is to use different names in case we implement some event carving-like feature when parsing broken journal file, https://codereview.appspot.com/310600043/diff/1/plaso/parsers/systemd_journal... plaso/parsers/systemd_journal.py:187: On 2016/10/26 19:08:33, Joachim Metz wrote: > - white line Done. https://codereview.appspot.com/310600043/diff/1/plaso/parsers/systemd_journal... plaso/parsers/systemd_journal.py:189: _ = self.journal_file.read(self._DATA_OBJECT_SIZE) On 2016/10/26 19:08:33, Joachim Metz wrote: > just remove "_ = " Done. https://codereview.appspot.com/310600043/diff/1/plaso/parsers/systemd_journal... plaso/parsers/systemd_journal.py:191: if object_header.type == u'DATA': On 2016/10/26 19:08:33, Joachim Metz wrote: > I opt to use a less nested approach > > if object_header.type != u'DATA': > raise() > > ... Done. https://codereview.appspot.com/310600043/diff/1/plaso/parsers/systemd_journal... plaso/parsers/systemd_journal.py:195: event_data = xzd.Decompress(event_data)[0] On 2016/10/26 19:08:33, Joachim Metz wrote: > event_data, _ = xzd.Decompress(event_data) Done. https://codereview.appspot.com/310600043/diff/1/plaso/parsers/systemd_journal... plaso/parsers/systemd_journal.py:209: parser_mediator: A parser mediator object (instance of ParserMediator). On 2016/10/26 19:08:33, Joachim Metz wrote: > parser_mediator (ParserMediator): parser mediator. Done. https://codereview.appspot.com/310600043/diff/1/plaso/parsers/systemd_journal... plaso/parsers/systemd_journal.py:215: On 2016/10/26 19:08:33, Joachim Metz wrote: > - white line Done. https://codereview.appspot.com/310600043/diff/1/plaso/parsers/systemd_journal... plaso/parsers/systemd_journal.py:222: if object_header.type == u'ENTRY': On 2016/10/26 19:08:33, Joachim Metz wrote: > if object_header.type != u'ENTRY': > raise() > > ... Done. https://codereview.appspot.com/310600043/diff/1/plaso/parsers/systemd_journal... plaso/parsers/systemd_journal.py:238: if u'SYSLOG_IDENTIFIER' in fields: On 2016/10/26 19:08:33, Joachim Metz wrote: > use x = field.get(u'SYSLOG_IDENTIFIER', None) to have a single look up Done. https://codereview.appspot.com/310600043/diff/1/plaso/parsers/systemd_journal... plaso/parsers/systemd_journal.py:263: if object_header.type == u'ENTRY_ARRAY': On 2016/10/26 19:08:33, Joachim Metz wrote: > if object_header.type != u'ENTRY_ARRAY': > raise() > > ... Done. https://codereview.appspot.com/310600043/diff/1/plaso/parsers/systemd_journal... plaso/parsers/systemd_journal.py:276: 'Expected an object of type ENTRY_ARRAY, but got {0:s}'.format( On 2016/11/01 14:53:31, onager wrote: > +u Done. https://codereview.appspot.com/310600043/diff/1/plaso/parsers/systemd_journal... plaso/parsers/systemd_journal.py:311: break On 2016/11/01 14:53:31, onager wrote: > Why the different behaviour between the two error states? Is there comment here > correct? It looks like parsing continues... The file we are parsing is either structurally broken, or we get unexpected data (like 0x0 offsets to event items) later, after having properly parsed some events. In the latter case, I stop looping over events('entries') and parsing stops https://codereview.appspot.com/310600043/diff/1/tests/parsers/systemd_journal.py File tests/parsers/systemd_journal.py (right): https://codereview.appspot.com/310600043/diff/1/tests/parsers/systemd_journal... tests/parsers/systemd_journal.py:11: On 2016/10/26 19:08:34, Joachim Metz wrote: > + white line Done. https://codereview.appspot.com/310600043/diff/1/tests/parsers/systemd_journal... tests/parsers/systemd_journal.py:13: """ Tests for the Systemd Journal parser.""" On 2016/10/26 19:08:34, Joachim Metz wrote: > """ Tests => """Tests Done. https://codereview.appspot.com/310600043/diff/1/tests/parsers/systemd_journal... tests/parsers/systemd_journal.py:31: expected_msg = (u'test-VirtualBox systemd-journald[577] Runtime journal ' On 2016/10/26 19:08:34, Joachim Metz wrote: > please don't use abbreviations > > expected_msg => expected_message grep -r "expected_msg" tests/ | wc -l 273 I was told coding was copy-pasting! Done. https://codereview.appspot.com/310600043/diff/1/tests/parsers/systemd_journal... tests/parsers/systemd_journal.py:45: self.assertEqual( On 2016/10/26 19:08:34, Joachim Metz wrote: > these 2 lines will fit on 1 line Done. https://codereview.appspot.com/310600043/diff/1/tests/parsers/systemd_journal... tests/parsers/systemd_journal.py:48: event_object = journal.events[0] On 2016/10/26 19:08:34, Joachim Metz wrote: > event_object => event Done.
Sign in to reply to this message.
Code updated.
Sign in to reply to this message.
https://codereview.appspot.com/310600043/diff/40001/plaso/parsers/systemd_jou... File plaso/parsers/systemd_journal.py (right): https://codereview.appspot.com/310600043/diff/40001/plaso/parsers/systemd_jou... plaso/parsers/systemd_journal.py:33: """Parses Systemd Journal files.""" If you define public class members please add them to the docstring. + Attributes: ... https://codereview.appspot.com/310600043/diff/40001/plaso/parsers/systemd_jou... plaso/parsers/systemd_journal.py:68: _ULInt64 = construct.Struct(u'int', construct.ULInt64(u'int')) Why this, and not use construct.ULInt64(u'int') directly? https://codereview.appspot.com/310600043/diff/40001/plaso/parsers/systemd_jou... plaso/parsers/systemd_journal.py:152: self.journal_file = None Should these be public? https://codereview.appspot.com/310600043/diff/40001/plaso/parsers/systemd_jou... plaso/parsers/systemd_journal.py:162: tuple[_OBJECT_HEADER,int]: the parsed object header and size of the _OBJECT_HEADER => construct.Struct https://codereview.appspot.com/310600043/diff/40001/plaso/parsers/systemd_jou... plaso/parsers/systemd_journal.py:166: SystemdJournalParseException: When an unexpected object type is parsed. where is this raised, I think the try: ... except: raise SystemdJournalParseException() is missing https://codereview.appspot.com/310600043/diff/40001/plaso/parsers/systemd_jou... plaso/parsers/systemd_journal.py:166: SystemdJournalParseException: When an unexpected object type is parsed. please user lower case for style consistency When => when https://codereview.appspot.com/310600043/diff/40001/plaso/parsers/systemd_jou... plaso/parsers/systemd_journal.py:166: SystemdJournalParseException: When an unexpected object type is parsed. Instead of SystemdJournalParseException why not use ParseError ? https://codereview.appspot.com/310600043/diff/40001/plaso/parsers/systemd_jou... plaso/parsers/systemd_journal.py:199: xzd = XZDecompressor() please do not use abbreviations xzd => xz_decompressor https://codereview.appspot.com/310600043/diff/40001/plaso/parsers/systemd_jou... plaso/parsers/systemd_journal.py:201: return event_data.decode(u'utf-8').split(u'=', 1) separate the split and decode across multiple lines what if the decode fails? https://codereview.appspot.com/310600043/diff/40001/plaso/parsers/systemd_jou... plaso/parsers/systemd_journal.py:225: fields = dict() dict() => {} https://codereview.appspot.com/310600043/diff/40001/plaso/parsers/systemd_jou... plaso/parsers/systemd_journal.py:233: # Already a number of microseconds since 1970-01-01, in UTC What is this comment trying to tell me? https://codereview.appspot.com/310600043/diff/40001/plaso/parsers/systemd_jou... plaso/parsers/systemd_journal.py:259: # First call What is this comment trying to tell me? https://codereview.appspot.com/310600043/diff/40001/plaso/parsers/systemd_jou... plaso/parsers/systemd_journal.py:268: next_array_offset = self._ULInt64.parse(self.journal_file.read(8)).int please split different function call across multiple lines, easier to debug also what if the read or parse fails? https://codereview.appspot.com/310600043/diff/40001/plaso/parsers/systemd_jou... plaso/parsers/systemd_journal.py:274: if next_array_offset == 0: rewrite as (or something equivalent): if next_array_offset != 0: entry_offsets.extend(self._ParseEntries(offset=next_array_offset)) return entry_offsets https://codereview.appspot.com/310600043/diff/40001/plaso/parsers/systemd_jou... plaso/parsers/systemd_journal.py:297: data_hash_table_end_offset = A + B field_hash_table_end_offset = C + D .. = max(data_hash_table_end_offset, field_hash_table_end_offset) https://codereview.appspot.com/310600043/diff/40001/plaso/parsers/systemd_jou... plaso/parsers/systemd_journal.py:309: # The journal seems broken from here, skip this entry and abort parsing. create an extraction error ? https://codereview.appspot.com/310600043/diff/40001/tests/parsers/systemd_jou... File tests/parsers/systemd_journal.py (right): https://codereview.appspot.com/310600043/diff/40001/tests/parsers/systemd_jou... tests/parsers/systemd_journal.py:35: expected_message_short = (u'test-VirtualBox systemd-journald[577] Runtime ' I would suggest a line break after ( so the text is less right-centric
Sign in to reply to this message.
https://codereview.appspot.com/310600043/diff/40001/plaso/parsers/systemd_jou... File plaso/parsers/systemd_journal.py (right): https://codereview.appspot.com/310600043/diff/40001/plaso/parsers/systemd_jou... plaso/parsers/systemd_journal.py:33: """Parses Systemd Journal files.""" On 2016/12/22 15:45:03, Joachim Metz wrote: > If you define public class members please add them to the docstring. > > + Attributes: > ... Attributes have been made private https://codereview.appspot.com/310600043/diff/40001/plaso/parsers/systemd_jou... plaso/parsers/systemd_journal.py:68: _ULInt64 = construct.Struct(u'int', construct.ULInt64(u'int')) On 2016/12/22 15:45:03, Joachim Metz wrote: > Why this, and not use construct.ULInt64(u'int') directly? That's a very interesting question. Fixed Done. https://codereview.appspot.com/310600043/diff/40001/plaso/parsers/systemd_jou... plaso/parsers/systemd_journal.py:152: self.journal_file = None On 2016/12/22 15:45:03, Joachim Metz wrote: > Should these be public? Probably not. Fixed https://codereview.appspot.com/310600043/diff/40001/plaso/parsers/systemd_jou... plaso/parsers/systemd_journal.py:162: tuple[_OBJECT_HEADER,int]: the parsed object header and size of the On 2016/12/22 15:45:03, Joachim Metz wrote: > _OBJECT_HEADER => construct.Struct Done. https://codereview.appspot.com/310600043/diff/40001/plaso/parsers/systemd_jou... plaso/parsers/systemd_journal.py:166: SystemdJournalParseException: When an unexpected object type is parsed. On 2016/12/22 15:45:03, Joachim Metz wrote: > please user lower case for style consistency > > When => when Done. https://codereview.appspot.com/310600043/diff/40001/plaso/parsers/systemd_jou... plaso/parsers/systemd_journal.py:166: SystemdJournalParseException: When an unexpected object type is parsed. On 2016/12/22 15:45:03, Joachim Metz wrote: > where is this raised, I think the try: ... except: raise > SystemdJournalParseException() is missing Deleted. https://codereview.appspot.com/310600043/diff/40001/plaso/parsers/systemd_jou... plaso/parsers/systemd_journal.py:166: SystemdJournalParseException: When an unexpected object type is parsed. On 2016/12/22 15:45:03, Joachim Metz wrote: > Instead of SystemdJournalParseException why not use ParseError ? Done. Had to change the test a bit https://codereview.appspot.com/310600043/diff/40001/plaso/parsers/systemd_jou... plaso/parsers/systemd_journal.py:199: xzd = XZDecompressor() On 2016/12/22 15:45:03, Joachim Metz wrote: > please do not use abbreviations > > xzd => xz_decompressor Done. https://codereview.appspot.com/310600043/diff/40001/plaso/parsers/systemd_jou... plaso/parsers/systemd_journal.py:201: return event_data.decode(u'utf-8').split(u'=', 1) On 2016/12/22 15:45:03, Joachim Metz wrote: > separate the split and decode across multiple lines > > what if the decode fails? Done. https://codereview.appspot.com/310600043/diff/40001/plaso/parsers/systemd_jou... plaso/parsers/systemd_journal.py:225: fields = dict() On 2016/12/22 15:45:03, Joachim Metz wrote: > dict() => {} Done. https://codereview.appspot.com/310600043/diff/40001/plaso/parsers/systemd_jou... plaso/parsers/systemd_journal.py:233: # Already a number of microseconds since 1970-01-01, in UTC On 2016/12/22 15:45:03, Joachim Metz wrote: > What is this comment trying to tell me? Trying to make it more clear https://codereview.appspot.com/310600043/diff/40001/plaso/parsers/systemd_jou... plaso/parsers/systemd_journal.py:259: # First call On 2016/12/22 15:45:03, Joachim Metz wrote: > What is this comment trying to tell me? Removed https://codereview.appspot.com/310600043/diff/40001/plaso/parsers/systemd_jou... plaso/parsers/systemd_journal.py:268: next_array_offset = self._ULInt64.parse(self.journal_file.read(8)).int On 2016/12/22 15:45:03, Joachim Metz wrote: > please split different function call across multiple lines, easier to debug > > also what if the read or parse fails? Done. https://codereview.appspot.com/310600043/diff/40001/plaso/parsers/systemd_jou... plaso/parsers/systemd_journal.py:274: if next_array_offset == 0: On 2016/12/22 15:45:03, Joachim Metz wrote: > rewrite as (or something equivalent): > > if next_array_offset != 0: > entry_offsets.extend(self._ParseEntries(offset=next_array_offset)) > > return entry_offsets Done. https://codereview.appspot.com/310600043/diff/40001/plaso/parsers/systemd_jou... plaso/parsers/systemd_journal.py:297: On 2016/12/22 15:45:03, Joachim Metz wrote: > data_hash_table_end_offset = A + B > field_hash_table_end_offset = C + D > > .. = max(data_hash_table_end_offset, field_hash_table_end_offset) Done. https://codereview.appspot.com/310600043/diff/40001/plaso/parsers/systemd_jou... plaso/parsers/systemd_journal.py:309: # The journal seems broken from here, skip this entry and abort parsing. On 2016/12/22 15:45:03, Joachim Metz wrote: > create an extraction error ? I'm getting rid of SystemdJournalParseException, and rely on ParseError to break out of parsing. Had to change the test to parse a drity file a bit https://codereview.appspot.com/310600043/diff/40001/tests/parsers/systemd_jou... File tests/parsers/systemd_journal.py (right): https://codereview.appspot.com/310600043/diff/40001/tests/parsers/systemd_jou... tests/parsers/systemd_journal.py:35: expected_message_short = (u'test-VirtualBox systemd-journald[577] Runtime ' On 2016/12/22 15:45:03, Joachim Metz wrote: > I would suggest a line break after ( > so the text is less right-centric Done.
Sign in to reply to this message.
Code updated.
Sign in to reply to this message.
On 2016/12/23 14:01:45, romaing wrote: > Code updated. romaing@, jbmetz@ - how are your discussions on the dependency going?
Sign in to reply to this message.
let's sync on this today
Sign in to reply to this message.
Code updated.
Sign in to reply to this message.
On 2017/01/26 14:45:10, onager wrote: > On 2016/12/23 14:01:45, romaing wrote: > > Code updated. > > romaing@, jbmetz@ - how are your discussions on the dependency going? I've moved to using the python's stdlib lzma module. Also added a new test_data file that actually has compressed events.
Sign in to reply to this message.
LGTM
Sign in to reply to this message.
Code updated.
Sign in to reply to this message.
Code updated.
Sign in to reply to this message.
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 systemd-journal
Sign in to reply to this message.
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 lzma
Sign in to reply to this message.
|
