Added initial version of NTFS UsnJrnl parser #316

Added initial version of NTFS UsnJrnl parser #316

[onager, 2015-12-04 11:11:45]

https://codereview.appspot.com/276290043/diff/1/plaso/engine/worker.py
File plaso/engine/worker.py (right):

https://codereview.appspot.com/276290043/diff/1/plaso/engine/worker.py#newcod...
plaso/engine/worker.py:488: logging.debug(u'[ProcessFileEntry] parsing file:
{0:s}'.format(
Can you change this log please? We're no longer "parsing a file" at this point.

https://codereview.appspot.com/276290043/diff/1/plaso/engine/worker.py#newcod...
plaso/engine/worker.py:500: # requires to read directly from the volume.
-requires to read +needs to read

https://codereview.appspot.com/276290043/diff/1/plaso/engine/worker.py#newcod...
plaso/engine/worker.py:556: 
-Parse +Process

https://codereview.appspot.com/276290043/diff/1/plaso/events/file_system_even...
File plaso/events/file_system_events.py (right):

https://codereview.appspot.com/276290043/diff/1/plaso/events/file_system_even...
plaso/events/file_system_events.py:57: reference of the parent, set to None if
not
-set to +or

Also in other docstrings.

https://codereview.appspot.com/276290043/diff/1/plaso/formatters/file_system.py
File plaso/formatters/file_system.py (right):

https://codereview.appspot.com/276290043/diff/1/plaso/formatters/file_system....
plaso/formatters/file_system.py:206: file_reference =
event_values.get(u'file_reference', 0)
Why 0 as the default for these values, instead of indicating that they're not
present?

https://codereview.appspot.com/276290043/diff/1/plaso/lib/binary.py
File plaso/lib/binary.py (right):

https://codereview.appspot.com/276290043/diff/1/plaso/lib/binary.py#newcode135
plaso/lib/binary.py:135: def Utf16StreamCopyToString(byte_stream,
byte_stream_size=None):
UTF16, per the style guide.

https://codereview.appspot.com/276290043/diff/1/plaso/parsers/ntfs.py
File plaso/parsers/ntfs.py (right):

https://codereview.appspot.com/276290043/diff/1/plaso/parsers/ntfs.py#newcode276
plaso/parsers/ntfs.py:276: u'{0:s}').format(exception))
 + invalid characters replace (or equiv)

[Joachim Metz, 2015-12-05 07:28:19]

https://codereview.appspot.com/276290043/diff/1/plaso/engine/worker.py
File plaso/engine/worker.py (right):

https://codereview.appspot.com/276290043/diff/1/plaso/engine/worker.py#newcod...
plaso/engine/worker.py:488: logging.debug(u'[ProcessFileEntry] parsing file:
{0:s}'.format(
On 2015/12/04 11:11:44, onager wrote:
> Can you change this log please? We're no longer "parsing a file" at this
point.

Done.

https://codereview.appspot.com/276290043/diff/1/plaso/engine/worker.py#newcod...
plaso/engine/worker.py:500: # requires to read directly from the volume.
On 2015/12/04 11:11:44, onager wrote:
> -requires to read +needs to read

Done.

https://codereview.appspot.com/276290043/diff/1/plaso/engine/worker.py#newcod...
plaso/engine/worker.py:556: 
On 2015/12/04 11:11:44, onager wrote:
> -Parse +Process

Done.

https://codereview.appspot.com/276290043/diff/1/plaso/events/file_system_even...
File plaso/events/file_system_events.py (right):

https://codereview.appspot.com/276290043/diff/1/plaso/events/file_system_even...
plaso/events/file_system_events.py:57: reference of the parent, set to None if
not
On 2015/12/04 11:11:44, onager wrote:
> -set to +or
> 
> Also in other docstrings.

Done.

https://codereview.appspot.com/276290043/diff/1/plaso/formatters/file_system.py
File plaso/formatters/file_system.py (right):

https://codereview.appspot.com/276290043/diff/1/plaso/formatters/file_system....
plaso/formatters/file_system.py:206: file_reference =
event_values.get(u'file_reference', 0)
On 2015/12/04 11:11:45, onager wrote:
> Why 0 as the default for these values, instead of indicating that they're not
> present?

Done.

https://codereview.appspot.com/276290043/diff/1/plaso/lib/binary.py
File plaso/lib/binary.py (right):

https://codereview.appspot.com/276290043/diff/1/plaso/lib/binary.py#newcode135
plaso/lib/binary.py:135: def Utf16StreamCopyToString(byte_stream,
byte_stream_size=None):
On 2015/12/04 11:11:45, onager wrote:
> UTF16, per the style guide.

Done.

https://codereview.appspot.com/276290043/diff/1/plaso/parsers/ntfs.py
File plaso/parsers/ntfs.py (right):

https://codereview.appspot.com/276290043/diff/1/plaso/parsers/ntfs.py#newcode276
plaso/parsers/ntfs.py:276: u'{0:s}').format(exception))
On 2015/12/04 11:11:45, onager wrote:
>  + invalid characters replace (or equiv)

Not sure what you mean here?

[Joachim Metz, 2015-12-05 07:31:16]

Code updated.

[onager, 2015-12-05 08:21:43]

https://codereview.appspot.com/276290043/diff/1/plaso/parsers/ntfs.py
File plaso/parsers/ntfs.py (right):

https://codereview.appspot.com/276290043/diff/1/plaso/parsers/ntfs.py#newcode276
plaso/parsers/ntfs.py:276: u'{0:s}').format(exception))
On 2015/12/05 07:28:19, Joachim Metz wrote:
> On 2015/12/04 11:11:45, onager wrote:
> >  + invalid characters replace (or equiv)
> 
> Not sure what you mean here?

I was finishing this comment just as I was heading out for lunch, and didn't
finish it properly. I meant that the error message should indicate that the
invalid byte sequences were removed from the record name (and the event was
still created).

[Joachim Metz, 2015-12-06 17:32:57]

https://codereview.appspot.com/276290043/diff/1/plaso/parsers/ntfs.py
File plaso/parsers/ntfs.py (right):

https://codereview.appspot.com/276290043/diff/1/plaso/parsers/ntfs.py#newcode276
plaso/parsers/ntfs.py:276: u'{0:s}').format(exception))
On 2015/12/05 08:21:43, onager wrote:
> On 2015/12/05 07:28:19, Joachim Metz wrote:
> > On 2015/12/04 11:11:45, onager wrote:
> > >  + invalid characters replace (or equiv)
> > 
> > Not sure what you mean here?
> 
> I was finishing this comment just as I was heading out for lunch, and didn't
> finish it properly. I meant that the error message should indicate that the
> invalid byte sequences were removed from the record name (and the event was
> still created).

Done.

[Joachim Metz, 2015-12-06 17:34:24]

Code updated.

[onager, 2015-12-08 20:25:02]

One little tidy up, but LGTM

https://codereview.appspot.com/276290043/diff/40001/plaso/engine/worker.py
File plaso/engine/worker.py (right):

https://codereview.appspot.com/276290043/diff/40001/plaso/engine/worker.py#ne...
plaso/engine/worker.py:494: is_metadata_file = self._IsMetadataFile(file_entry)
This can be moved to the condition now, no need for a separate variable.

[Joachim Metz, 2015-12-08 21:37:48]

Code updated.

[Joachim Metz, 2015-12-08 22:06:18]

https://codereview.appspot.com/276290043/diff/40001/plaso/engine/worker.py
File plaso/engine/worker.py (right):

https://codereview.appspot.com/276290043/diff/40001/plaso/engine/worker.py#ne...
plaso/engine/worker.py:494: is_metadata_file = self._IsMetadataFile(file_entry)
On 2015/12/08 20:25:01, onager wrote:
> This can be moved to the condition now, no need for a separate variable.

Acknowledged.