Clean up of Windows preprocessor plugins #155.

Clean up of Windows preprocessor plugins #155.

[Joachim Metz, 2015-07-05 09:18:09]

Not finished yet, but have a look for discussion/direction sake.

[Joachim Metz, 2015-07-06 06:58:32]

Code updated.

[Joachim Metz, 2015-07-06 08:05:18]

Code updated.

[Joachim Metz, 2015-07-06 08:09:17]

Ready for review let me know if you need some addition context and input on
where this refactor is heading.

[Joachim Metz, 2015-07-07 08:29:18]

Code updated.

[onager, 2015-07-23 14:47:18]

Some initial comments

https://codereview.appspot.com/250920043/diff/60001/plaso/engine/engine.py
File plaso/engine/engine.py (right):

https://codereview.appspot.com/250920043/diff/60001/plaso/engine/engine.py#ne...
plaso/engine/engine.py:54: the mount point path specification (instance of
path.PathSpec).
How is the mount point path spec different to the source path spec?

https://codereview.appspot.com/250920043/diff/60001/plaso/frontend/image_expo...
File plaso/frontend/image_export.py (right):

https://codereview.appspot.com/250920043/diff/60001/plaso/frontend/image_expo...
plaso/frontend/image_export.py:715: file_system: the file system object
(instance of vfs.FileSystem).
The file system to be preprocessed.

https://codereview.appspot.com/250920043/diff/60001/plaso/frontend/image_expo...
plaso/frontend/image_export.py:716: mount_point: the mount point path
specification (instance of
The mount point path specification that refers to the location of the file
system.

https://codereview.appspot.com/250920043/diff/60001/plaso/frontend/preg.py
File plaso/frontend/preg.py (right):

https://codereview.appspot.com/250920043/diff/60001/plaso/frontend/preg.py#ne...
plaso/frontend/preg.py:278: source_path_spec: The source path specification
(instance of
Per my comment on the other instance of this method - how is the source path
spec different the to the mount point path spec?

https://codereview.appspot.com/250920043/diff/60001/plaso/preprocessors/manag...
File plaso/preprocessors/manager.py (right):

https://codereview.appspot.com/250920043/diff/60001/plaso/preprocessors/manag...
plaso/preprocessors/manager.py:24: their classanme as the key.
class name

https://codereview.appspot.com/250920043/diff/60001/plaso/preprocessors/manag...
plaso/preprocessors/manager.py:30: A preprocess plugin objects that matches the
platform and weight.
-s

https://codereview.appspot.com/250920043/diff/60001/plaso/preprocessors/manag...
plaso/preprocessors/manager.py:123: file_system: the file system object
(instance of vfs.FileSystem).
the file system to run the plugins against

https://codereview.appspot.com/250920043/diff/60001/plaso/preprocessors/manag...
plaso/preprocessors/manager.py:124: mount_point: the mount point path
specification (instance of
Again, a little more specificity around this path spec is would be handy

https://codereview.appspot.com/250920043/diff/60001/plaso/preprocessors/manag...
plaso/preprocessors/manager.py:150: # Run the Registry plugins seperately so we
do not have to open
separately

https://codereview.appspot.com/250920043/diff/60001/plaso/preprocessors/windo...
File plaso/preprocessors/windows.py (right):

https://codereview.appspot.com/250920043/diff/60001/plaso/preprocessors/windo...
plaso/preprocessors/windows.py:60: """Set the attribute of the object store to
the value from GetValue.
? what object store? GetValue?

https://codereview.appspot.com/250920043/diff/60001/plaso/winreg/winpyregf.py
File plaso/winreg/winpyregf.py (right):

https://codereview.appspot.com/250920043/diff/60001/plaso/winreg/winpyregf.py...
plaso/winreg/winpyregf.py:255: A Registry key path may contain paths that are
attributes, based on
-paths +elements?

[Joachim Metz, 2015-07-24 03:55:32]

https://codereview.appspot.com/250920043/diff/60001/plaso/engine/engine.py
File plaso/engine/engine.py (right):

https://codereview.appspot.com/250920043/diff/60001/plaso/engine/engine.py#ne...
plaso/engine/engine.py:54: the mount point path specification (instance of
path.PathSpec).
On 2015/07/23 14:47:17, onager wrote:
> How is the mount point path spec different to the source path spec?

Done.

https://codereview.appspot.com/250920043/diff/60001/plaso/frontend/image_expo...
File plaso/frontend/image_export.py (right):

https://codereview.appspot.com/250920043/diff/60001/plaso/frontend/image_expo...
plaso/frontend/image_export.py:715: file_system: the file system object
(instance of vfs.FileSystem).
On 2015/07/23 14:47:17, onager wrote:
> The file system to be preprocessed.

Done.

https://codereview.appspot.com/250920043/diff/60001/plaso/frontend/image_expo...
plaso/frontend/image_export.py:716: mount_point: the mount point path
specification (instance of
On 2015/07/23 14:47:17, onager wrote:
> The mount point path specification that refers to the location of the file
> system.

Done.

https://codereview.appspot.com/250920043/diff/60001/plaso/frontend/preg.py
File plaso/frontend/preg.py (right):

https://codereview.appspot.com/250920043/diff/60001/plaso/frontend/preg.py#ne...
plaso/frontend/preg.py:278: source_path_spec: The source path specification
(instance of
On 2015/07/23 14:47:17, onager wrote:
> Per my comment on the other instance of this method - how is the source path
> spec different the to the mount point path spec?

Done.

https://codereview.appspot.com/250920043/diff/60001/plaso/preprocessors/manag...
File plaso/preprocessors/manager.py (right):

https://codereview.appspot.com/250920043/diff/60001/plaso/preprocessors/manag...
plaso/preprocessors/manager.py:24: their classanme as the key.
On 2015/07/23 14:47:17, onager wrote:
> class name

Done.

https://codereview.appspot.com/250920043/diff/60001/plaso/preprocessors/manag...
plaso/preprocessors/manager.py:30: A preprocess plugin objects that matches the
platform and weight.
On 2015/07/23 14:47:17, onager wrote:
> -s

Done.

https://codereview.appspot.com/250920043/diff/60001/plaso/preprocessors/manag...
plaso/preprocessors/manager.py:123: file_system: the file system object
(instance of vfs.FileSystem).
On 2015/07/23 14:47:17, onager wrote:
> the file system to run the plugins against

Done.

https://codereview.appspot.com/250920043/diff/60001/plaso/preprocessors/manag...
plaso/preprocessors/manager.py:124: mount_point: the mount point path
specification (instance of
On 2015/07/23 14:47:17, onager wrote:
> Again, a little more specificity around this path spec is would be handy

Done.

https://codereview.appspot.com/250920043/diff/60001/plaso/preprocessors/manag...
plaso/preprocessors/manager.py:150: # Run the Registry plugins seperately so we
do not have to open
On 2015/07/23 14:47:17, onager wrote:
> separately

Done.

https://codereview.appspot.com/250920043/diff/60001/plaso/preprocessors/windo...
File plaso/preprocessors/windows.py (right):

https://codereview.appspot.com/250920043/diff/60001/plaso/preprocessors/windo...
plaso/preprocessors/windows.py:60: """Set the attribute of the object store to
the value from GetValue.
Done, left overs from earlier time

https://codereview.appspot.com/250920043/diff/60001/plaso/winreg/winpyregf.py
File plaso/winreg/winpyregf.py (right):

https://codereview.appspot.com/250920043/diff/60001/plaso/winreg/winpyregf.py...
plaso/winreg/winpyregf.py:255: A Registry key path may contain paths that are
attributes, based on
On 2015/07/23 14:47:17, onager wrote:
> -paths +elements?

Done.

[Joachim Metz, 2015-07-24 03:57:40]

Code updated.

[onager, 2015-07-24 08:13:08]

https://codereview.appspot.com/250920043/diff/80001/plaso/engine/engine.py
File plaso/engine/engine.py (right):

https://codereview.appspot.com/250920043/diff/80001/plaso/engine/engine.py#ne...
plaso/engine/engine.py:46: a volume on storage media device or image. It is
needed by the dfVFS
+a

https://codereview.appspot.com/250920043/diff/80001/plaso/engine/engine.py#ne...
plaso/engine/engine.py:48: the base location of the file system.
OK, so this the location of the root of the file system relative to... what,
specifically? Other file systems in the VFS?

https://codereview.appspot.com/250920043/diff/80001/plaso/engine/engine.py#ne...
plaso/engine/engine.py:59: the point path specification (instance of
path.PathSpec).
+mount

[Joachim Metz, 2015-07-24 09:59:17]

https://codereview.appspot.com/250920043/diff/80001/plaso/engine/engine.py
File plaso/engine/engine.py (right):

https://codereview.appspot.com/250920043/diff/80001/plaso/engine/engine.py#ne...
plaso/engine/engine.py:48: the base location of the file system.
the base is the virtual root of the mounted file system this will depend if the
source type is a directory or a storage media image/device

e.g. source type directory: /case/mount/

We want to search for /Windows/System32 the base would be /case/mount/ and the
search would match /case/mount/Windows/System32

Similar for a storage media image/device where the base is the volume (parent
path spec).

[Joachim Metz, 2015-07-25 04:00:42]

https://codereview.appspot.com/250920043/diff/80001/plaso/engine/engine.py
File plaso/engine/engine.py (right):

https://codereview.appspot.com/250920043/diff/80001/plaso/engine/engine.py#ne...
plaso/engine/engine.py:46: a volume on storage media device or image. It is
needed by the dfVFS
On 2015/07/24 08:13:08, onager wrote:
> +a

Done.

https://codereview.appspot.com/250920043/diff/80001/plaso/engine/engine.py#ne...
plaso/engine/engine.py:59: the point path specification (instance of
path.PathSpec).
On 2015/07/24 08:13:08, onager wrote:
> +mount

Done.

[Joachim Metz, 2015-07-28 15:12:23]

Code updated.

[onager, 2015-07-30 09:11:15]

On 2015/07/28 15:12:23, Joachim Metz wrote:
> Code updated.

Minor comments, otherwise LGTM

[onager, 2015-07-30 12:24:07]

Now with the comments!

https://codereview.appspot.com/250920043/diff/100001/plaso/engine/engine.py
File plaso/engine/engine.py (right):

https://codereview.appspot.com/250920043/diff/100001/plaso/engine/engine.py#n...
plaso/engine/engine.py:47: file system searcher (instance of FileSystemSearcher)
to indicate
Move this comment to the returns section - it's weird here, before the mount
point path spec is mentioned.

https://codereview.appspot.com/250920043/diff/100001/plaso/frontend/preg.py
File plaso/frontend/preg.py (right):

https://codereview.appspot.com/250920043/diff/100001/plaso/frontend/preg.py#n...
plaso/frontend/preg.py:757: """Calculate the Registry cache.
Calculate? I suspect this should be "Create"

https://codereview.appspot.com/250920043/diff/100001/plaso/preprocessors/mana...
File plaso/preprocessors/manager.py (right):

https://codereview.appspot.com/250920043/diff/100001/plaso/preprocessors/mana...
plaso/preprocessors/manager.py:134: PreProcessFail: If the preprocessing failed.
Does it? I see this exception being caught and then logged.

https://codereview.appspot.com/250920043/diff/100001/tests/winreg/winregistry.py
File tests/winreg/winregistry.py (right):

https://codereview.appspot.com/250920043/diff/100001/tests/winreg/winregistry...
tests/winreg/winregistry.py:15: def testInitialze(self):
Initialize

[Joachim Metz, 2015-07-30 12:37:54]

https://codereview.appspot.com/250920043/diff/100001/plaso/engine/engine.py
File plaso/engine/engine.py (right):

https://codereview.appspot.com/250920043/diff/100001/plaso/engine/engine.py#n...
plaso/engine/engine.py:47: file system searcher (instance of FileSystemSearcher)
to indicate
On 2015/07/30 12:24:07, onager wrote:
> Move this comment to the returns section - it's weird here, before the mount
> point path spec is mentioned.

Done.

https://codereview.appspot.com/250920043/diff/100001/plaso/frontend/preg.py
File plaso/frontend/preg.py (right):

https://codereview.appspot.com/250920043/diff/100001/plaso/frontend/preg.py#n...
plaso/frontend/preg.py:757: """Calculate the Registry cache.
On 2015/07/30 12:24:07, onager wrote:
> Calculate? I suspect this should be "Create"

Good question, changed to Create

https://codereview.appspot.com/250920043/diff/100001/plaso/preprocessors/mana...
File plaso/preprocessors/manager.py (right):

https://codereview.appspot.com/250920043/diff/100001/plaso/preprocessors/mana...
plaso/preprocessors/manager.py:134: PreProcessFail: If the preprocessing failed.
Good catch, removed raises

https://codereview.appspot.com/250920043/diff/100001/tests/winreg/winregistry.py
File tests/winreg/winregistry.py (right):

https://codereview.appspot.com/250920043/diff/100001/tests/winreg/winregistry...
tests/winreg/winregistry.py:15: def testInitialze(self):
On 2015/07/30 12:24:07, onager wrote:
> Initialize

Done.

[Joachim Metz, 2015-07-30 12:41:41]

Code updated.