Adding coldfusion parser

Adding coldfusion parser

[ash, 2014-06-21 14:18:45]

.

[ash, 2014-06-21 14:21:15]

.

[ash, 2014-06-21 14:42:36]

.

[kiddi, 2014-06-21 17:01:43]

First round of comments, and I'm not seeing the test file in the codereview....
did you add it to your client???

https://codereview.appspot.com/110090043/diff/2/plaso/formatters/coldfusion.py
File plaso/formatters/coldfusion.py (right):

https://codereview.appspot.com/110090043/diff/2/plaso/formatters/coldfusion.p...
plaso/formatters/coldfusion.py:1: #!/usr/bin/python # MAKE THE UPLOADER WORK
remove this

https://codereview.appspot.com/110090043/diff/2/plaso/formatters/coldfusion.p...
plaso/formatters/coldfusion.py:4: # Copyright 2013 The Plaso Project Authors.
new file in 2014

https://codereview.appspot.com/110090043/diff/2/plaso/formatters/coldfusion.p...
plaso/formatters/coldfusion.py:18: """Formatter for Windows firewall log
files."""
is it?

https://codereview.appspot.com/110090043/diff/2/plaso/formatters/coldfusion.p...
plaso/formatters/coldfusion.py:33: ]
close the bracket on the line above (same with the short pieces list)

https://codereview.appspot.com/110090043/diff/2/plaso/parsers/coldfusion.py
File plaso/parsers/coldfusion.py (right):

https://codereview.appspot.com/110090043/diff/2/plaso/parsers/coldfusion.py#n...
plaso/parsers/coldfusion.py:1: #!/usr/bin/python # MAKE THE UPLOADER WORK
remove this

https://codereview.appspot.com/110090043/diff/2/plaso/parsers/coldfusion.py#n...
plaso/parsers/coldfusion.py:4: # Copyright 2013 The Plaso Project Authors.
new one in 2014

https://codereview.appspot.com/110090043/diff/2/plaso/parsers/coldfusion.py#n...
plaso/parsers/coldfusion.py:29: 
two empty lines before __author and same before class

https://codereview.appspot.com/110090043/diff/2/plaso/parsers/coldfusion.py#n...
plaso/parsers/coldfusion.py:50: HEADER =
pyparsing.Literal('"Severity","ThreadID","Date","Time",' \
indentation here

pyparsing.Literal(
    '"Severity",......
    '"Time......)

https://codereview.appspot.com/110090043/diff/2/plaso/parsers/coldfusion.py#n...
plaso/parsers/coldfusion.py:76: DATA_TYPE = 'coldfusion:log_entry'
data type should be defined in the event object, not the parser

https://codereview.appspot.com/110090043/diff/2/plaso/parsers/coldfusion.py#n...
plaso/parsers/coldfusion.py:85: super(ColdfusionParser, self).__init__(pre_obj,
config)
you are not changing anything in the __init__ function, no need to define it

https://codereview.appspot.com/110090043/diff/2/plaso/parsers/coldfusion.py#n...
plaso/parsers/coldfusion.py:105: log_dict = structure.asDict()
any reason you want the dict instead of just using the structure object?

https://codereview.appspot.com/110090043/diff/2/plaso/parsers/coldfusion.py#n...
plaso/parsers/coldfusion.py:117: zone = pytz.utc
utc is the default zone, no need to specifically define that if the timestamp is
UTC.... and then you don'pt need to import pytz

https://codereview.appspot.com/110090043/diff/2/plaso/parsers/coldfusion.py#n...
plaso/parsers/coldfusion.py:126: timestamp,
eventdata.EventTimestamp.WRITTEN_TIME, self.DATA_TYPE)
create a new event object class,

ColdfusionEvent or something like that, which inherits from TimestampEvent, that
class defines the DATA_TYPE

https://codereview.appspot.com/110090043/diff/2/plaso/parsers/coldfusion.py#n...
plaso/parsers/coldfusion.py:128: for key, value in structure.asDict().items():
so you already have log_dict, which is structure.asDict, no need to call that
twice.....

https://codereview.appspot.com/110090043/diff/2/plaso/parsers/coldfusion.py#n...
plaso/parsers/coldfusion.py:131: if key in ('message'):
or if key == 'message' ?

https://codereview.appspot.com/110090043/diff/2/plaso/parsers/coldfusion.py#n...
plaso/parsers/coldfusion.py:132: value = str(value).replace('"', '')
str() ??? what if it is unicode????

you can use the utils functions

from plaso.lib import utils

https://code.google.com/p/plaso/source/browse/plaso/lib/utils.py#127

utils.GetUnicodeString(value) (uses similar but uses decode to get an unicode
string back)

https://codereview.appspot.com/110090043/diff/2/plaso/parsers/coldfusion.py#n...
plaso/parsers/coldfusion.py:134: setattr(event_object, key, ''.join(value))
u''.join

https://codereview.appspot.com/110090043/diff/2/plaso/parsers/coldfusion.py#n...
plaso/parsers/coldfusion.py:137: save_value = int(value)
use base, int(value, 10)

https://codereview.appspot.com/110090043/diff/2/plaso/parsers/coldfusion_test.py
File plaso/parsers/coldfusion_test.py (right):

https://codereview.appspot.com/110090043/diff/2/plaso/parsers/coldfusion_test...
plaso/parsers/coldfusion_test.py:4: # Copyright 2013 The Plaso Project Authors.
new file in 2014

https://codereview.appspot.com/110090043/diff/2/plaso/parsers/coldfusion_test...
plaso/parsers/coldfusion_test.py:18: """Tests for the Windows firewall log
parser."""
windows firewall? really?

https://codereview.appspot.com/110090043/diff/2/plaso/parsers/coldfusion_test...
plaso/parsers/coldfusion_test.py:28: 
two empty lines before the class

https://codereview.appspot.com/110090043/diff/2/plaso/parsers/coldfusion_test...
plaso/parsers/coldfusion_test.py:30: """Tests for the Windows firewall log
parser."""
firewall?

https://codereview.appspot.com/110090043/diff/2/plaso/parsers/coldfusion_test...
plaso/parsers/coldfusion_test.py:53: 
only one empty lien here

https://codereview.appspot.com/110090043/diff/2/plaso/parsers/coldfusion_test...
plaso/parsers/coldfusion_test.py:59: u'Information: Application: ,')
why is the ": ," string there? should that be the case?

https://codereview.appspot.com/110090043/diff/2/plaso/parsers/coldfusion_test...
plaso/parsers/coldfusion_test.py:68: test_message = r'Error attempting to
resolve the template ' \
use 

test_message = (
    r'Error att.....'
    r'htt.....')

instead of using \ to break lines

https://codereview.appspot.com/110090043/diff/2/plaso/parsers/coldfusion_test...
plaso/parsers/coldfusion_test.py:75: self.assertEquals(event_object.application,
',')
is application just a comma? is that right? should that be the case?

https://codereview.appspot.com/110090043/diff/2/plaso/parsers/coldfusion_test...
plaso/parsers/coldfusion_test.py:77: 
one empty line

https://codereview.appspot.com/110090043/diff/2/plaso/parsers/coldfusion_test...
plaso/parsers/coldfusion_test.py:82: self.assertEquals(event_object.severity,
'Error')
and I don't see a test for the message string

[ash, 2014-07-01 13:48:59]

.

[ash, 2014-07-01 13:53:03]

.

[ash, 2014-07-01 13:58:46]

.

[ash, 2014-07-01 14:19:18]

.

[ash, 2014-07-01 14:20:22]

Added the test file that the upload wasn't picking up, did some refactors
similar to IIS parser, and changed the behavior so null values don't give a
random comma for an attribute.

https://codereview.appspot.com/110090043/diff/2/plaso/formatters/coldfusion.py
File plaso/formatters/coldfusion.py (right):

https://codereview.appspot.com/110090043/diff/2/plaso/formatters/coldfusion.p...
plaso/formatters/coldfusion.py:1: #!/usr/bin/python # MAKE THE UPLOADER WORK
On 2014/06/21 17:01:41, kiddi wrote:
> remove this

Done.

https://codereview.appspot.com/110090043/diff/2/plaso/formatters/coldfusion.p...
plaso/formatters/coldfusion.py:4: # Copyright 2013 The Plaso Project Authors.
On 2014/06/21 17:01:41, kiddi wrote:
> new file in 2014

Done.

https://codereview.appspot.com/110090043/diff/2/plaso/formatters/coldfusion.p...
plaso/formatters/coldfusion.py:18: """Formatter for Windows firewall log
files."""
On 2014/06/21 17:01:41, kiddi wrote:
> is it?

Done.

https://codereview.appspot.com/110090043/diff/2/plaso/formatters/coldfusion.p...
plaso/formatters/coldfusion.py:33: ]
On 2014/06/21 17:01:41, kiddi wrote:
> close the bracket on the line above (same with the short pieces list)

Done.

https://codereview.appspot.com/110090043/diff/2/plaso/parsers/coldfusion.py
File plaso/parsers/coldfusion.py (right):

https://codereview.appspot.com/110090043/diff/2/plaso/parsers/coldfusion.py#n...
plaso/parsers/coldfusion.py:1: #!/usr/bin/python # MAKE THE UPLOADER WORK
On 2014/06/21 17:01:41, kiddi wrote:
> remove this

Done.

https://codereview.appspot.com/110090043/diff/2/plaso/parsers/coldfusion.py#n...
plaso/parsers/coldfusion.py:4: # Copyright 2013 The Plaso Project Authors.
On 2014/06/21 17:01:42, kiddi wrote:
> new one in 2014

Done.

https://codereview.appspot.com/110090043/diff/2/plaso/parsers/coldfusion.py#n...
plaso/parsers/coldfusion.py:29: 
On 2014/06/21 17:01:41, kiddi wrote:
> two empty lines before __author and same before class

Done.

https://codereview.appspot.com/110090043/diff/2/plaso/parsers/coldfusion.py#n...
plaso/parsers/coldfusion.py:50: HEADER =
pyparsing.Literal('"Severity","ThreadID","Date","Time",' \
On 2014/06/21 17:01:42, kiddi wrote:
> indentation here
> 
> pyparsing.Literal(
>     '"Severity",......
>     '"Time......)

Done.

https://codereview.appspot.com/110090043/diff/2/plaso/parsers/coldfusion.py#n...
plaso/parsers/coldfusion.py:76: DATA_TYPE = 'coldfusion:log_entry'
On 2014/06/21 17:01:42, kiddi wrote:
> data type should be defined in the event object, not the parser

Done.

https://codereview.appspot.com/110090043/diff/2/plaso/parsers/coldfusion.py#n...
plaso/parsers/coldfusion.py:85: super(ColdfusionParser, self).__init__(pre_obj,
config)
On 2014/06/21 17:01:42, kiddi wrote:
> you are not changing anything in the __init__ function, no need to define it
> 

Done.

https://codereview.appspot.com/110090043/diff/2/plaso/parsers/coldfusion.py#n...
plaso/parsers/coldfusion.py:105: log_dict = structure.asDict()
On 2014/06/21 17:01:41, kiddi wrote:
> any reason you want the dict instead of just using the structure object?

Done.

https://codereview.appspot.com/110090043/diff/2/plaso/parsers/coldfusion.py#n...
plaso/parsers/coldfusion.py:117: zone = pytz.utc
On 2014/06/21 17:01:42, kiddi wrote:
> utc is the default zone, no need to specifically define that if the timestamp
is
> UTC.... and then you don'pt need to import pytz

Done.

https://codereview.appspot.com/110090043/diff/2/plaso/parsers/coldfusion.py#n...
plaso/parsers/coldfusion.py:126: timestamp,
eventdata.EventTimestamp.WRITTEN_TIME, self.DATA_TYPE)
On 2014/06/21 17:01:41, kiddi wrote:
> create a new event object class,
> 
> ColdfusionEvent or something like that, which inherits from TimestampEvent,
that
> class defines the DATA_TYPE

Done.

https://codereview.appspot.com/110090043/diff/2/plaso/parsers/coldfusion.py#n...
plaso/parsers/coldfusion.py:128: for key, value in structure.asDict().items():
On 2014/06/21 17:01:42, kiddi wrote:
> so you already have log_dict, which is structure.asDict, no need to call that
> twice.....
> 

Done.

https://codereview.appspot.com/110090043/diff/2/plaso/parsers/coldfusion.py#n...
plaso/parsers/coldfusion.py:131: if key in ('message'):
Not necessary anymore, killed it. 

On 2014/06/21 17:01:42, kiddi wrote:
> or if key == 'message' ?

https://codereview.appspot.com/110090043/diff/2/plaso/parsers/coldfusion.py#n...
plaso/parsers/coldfusion.py:132: value = str(value).replace('"', '')
Don't need this silly replace anymore, changed pyparsing word def.

On 2014/06/21 17:01:41, kiddi wrote:
> str() ??? what if it is unicode????
> 
> you can use the utils functions
> 
> from plaso.lib import utils
> 
> https://code.google.com/p/plaso/source/browse/plaso/lib/utils.py#127
> 
> utils.GetUnicodeString(value) (uses similar but uses decode to get an unicode
> string back)

https://codereview.appspot.com/110090043/diff/2/plaso/parsers/coldfusion.py#n...
plaso/parsers/coldfusion.py:134: setattr(event_object, key, ''.join(value))
On 2014/06/21 17:01:42, kiddi wrote:
> u''.join

Done.

https://codereview.appspot.com/110090043/diff/2/plaso/parsers/coldfusion.py#n...
plaso/parsers/coldfusion.py:137: save_value = int(value)
On 2014/06/21 17:01:42, kiddi wrote:
> use base, int(value, 10)

Done.

https://codereview.appspot.com/110090043/diff/2/plaso/parsers/coldfusion_test.py
File plaso/parsers/coldfusion_test.py (right):

https://codereview.appspot.com/110090043/diff/2/plaso/parsers/coldfusion_test...
plaso/parsers/coldfusion_test.py:4: # Copyright 2013 The Plaso Project Authors.
On 2014/06/21 17:01:43, kiddi wrote:
> new file in 2014

Done.

https://codereview.appspot.com/110090043/diff/2/plaso/parsers/coldfusion_test...
plaso/parsers/coldfusion_test.py:18: """Tests for the Windows firewall log
parser."""
On 2014/06/21 17:01:42, kiddi wrote:
> windows firewall? really?

Done.

https://codereview.appspot.com/110090043/diff/2/plaso/parsers/coldfusion_test...
plaso/parsers/coldfusion_test.py:28: 
On 2014/06/21 17:01:42, kiddi wrote:
> two empty lines before the class

Done.

https://codereview.appspot.com/110090043/diff/2/plaso/parsers/coldfusion_test...
plaso/parsers/coldfusion_test.py:30: """Tests for the Windows firewall log
parser."""
On 2014/06/21 17:01:42, kiddi wrote:
> firewall?

Done.

https://codereview.appspot.com/110090043/diff/2/plaso/parsers/coldfusion_test...
plaso/parsers/coldfusion_test.py:53: 
On 2014/06/21 17:01:42, kiddi wrote:
> only one empty lien here

Done.

https://codereview.appspot.com/110090043/diff/2/plaso/parsers/coldfusion_test...
plaso/parsers/coldfusion_test.py:59: u'Information: Application: ,')
Changed parser behavior, now no , as a result, just doesn't set the attribute if
it's not there. 

On 2014/06/21 17:01:42, kiddi wrote:
> why is the ": ," string there? should that be the case?

https://codereview.appspot.com/110090043/diff/2/plaso/parsers/coldfusion_test...
plaso/parsers/coldfusion_test.py:68: test_message = r'Error attempting to
resolve the template ' \
On 2014/06/21 17:01:42, kiddi wrote:
> use 
> 
> test_message = (
>     r'Error att.....'
>     r'htt.....')
> 
> instead of using \ to break lines

Done.

https://codereview.appspot.com/110090043/diff/2/plaso/parsers/coldfusion_test...
plaso/parsers/coldfusion_test.py:75: self.assertEquals(event_object.application,
',')
Changed parser behavior, now it just won't set if it's empty, and not give a
random comma. 

On 2014/06/21 17:01:42, kiddi wrote:
> is application just a comma? is that right? should that be the case?

https://codereview.appspot.com/110090043/diff/2/plaso/parsers/coldfusion_test...
plaso/parsers/coldfusion_test.py:77: 
On 2014/06/21 17:01:42, kiddi wrote:
> one empty line

Done.

https://codereview.appspot.com/110090043/diff/2/plaso/parsers/coldfusion_test...
plaso/parsers/coldfusion_test.py:82: self.assertEquals(event_object.severity,
'Error')
On 2014/06/21 17:01:42, kiddi wrote:
> and I don't see a test for the message string

Done.

[kiddi, 2014-07-12 18:33:03]

Very minor comments, otherwise almost ready

https://codereview.appspot.com/110090043/diff/70002/plaso/parsers/coldfusion.py
File plaso/parsers/coldfusion.py (right):

https://codereview.appspot.com/110090043/diff/70002/plaso/parsers/coldfusion....
plaso/parsers/coldfusion.py:45: super(ColdFusionEventObject,
self).__init__(timestamp,
indent wrong

.__init__(
    timestamp, ....

https://codereview.appspot.com/110090043/diff/70002/plaso/parsers/coldfusion....
plaso/parsers/coldfusion.py:137: month, day, year = date
could you put up a minor comment above describing how the timestamp looks like

https://codereview.appspot.com/110090043/diff/70002/plaso/parsers/coldfusion_...
File plaso/parsers/coldfusion_test.py (right):

https://codereview.appspot.com/110090043/diff/70002/plaso/parsers/coldfusion_...
plaso/parsers/coldfusion_test.py:22: from plaso.formatters import coldfusion
have this as import coldfusion as coldfusion_formatter

with the pylint suppression above

since you are importing coldfusion few lines below and thus overwriting this
import

https://codereview.appspot.com/110090043/diff/70002/plaso/parsers/coldfusion_...
plaso/parsers/coldfusion_test.py:70: test_message = (r'Error attempting to
resolve the template ' +
don't use the + notation

test_message = (
    u'Error....'
    u'comthih.....')

[ash, 2014-08-05 05:57:11]

Updated to reflect recent codebase changes & addressed comments.

https://codereview.appspot.com/110090043/diff/70002/plaso/parsers/coldfusion.py
File plaso/parsers/coldfusion.py (right):

https://codereview.appspot.com/110090043/diff/70002/plaso/parsers/coldfusion....
plaso/parsers/coldfusion.py:45: super(ColdFusionEventObject,
self).__init__(timestamp,
On 2014/07/12 18:33:02, kiddi wrote:
> indent wrong
> 
> .__init__(
>     timestamp, ....

Done.

https://codereview.appspot.com/110090043/diff/70002/plaso/parsers/coldfusion....
plaso/parsers/coldfusion.py:137: month, day, year = date
On 2014/07/12 18:33:02, kiddi wrote:
> could you put up a minor comment above describing how the timestamp looks like

Done.

https://codereview.appspot.com/110090043/diff/70002/plaso/parsers/coldfusion_...
File plaso/parsers/coldfusion_test.py (right):

https://codereview.appspot.com/110090043/diff/70002/plaso/parsers/coldfusion_...
plaso/parsers/coldfusion_test.py:22: from plaso.formatters import coldfusion
On 2014/07/12 18:33:02, kiddi wrote:
> have this as import coldfusion as coldfusion_formatter
> 
> with the pylint suppression above
> 
> since you are importing coldfusion few lines below and thus overwriting this
> import

Done.

https://codereview.appspot.com/110090043/diff/70002/plaso/parsers/coldfusion_...
plaso/parsers/coldfusion_test.py:70: test_message = (r'Error attempting to
resolve the template ' +
On 2014/07/12 18:33:02, kiddi wrote:
> don't use the + notation
> 
> test_message = (
>     u'Error....'
>     u'comthih.....')

Done.

[ash, 2014-08-05 06:02:18]

.

[ash, 2014-08-05 06:04:38]

.

[ash, 2014-08-05 06:05:51]

.

[kiddi, 2014-08-20 02:42:46]

Mostly fine, few comments here.

Also, tomorrow Joachim is about to push a fairly large change, could you update
the code to make it work again and answer these comments, then you are probably
good to go.

https://codereview.appspot.com/110090043/diff/150001/plaso/parsers/coldfusion.py
File plaso/parsers/coldfusion.py (right):

https://codereview.appspot.com/110090043/diff/150001/plaso/parsers/coldfusion...
plaso/parsers/coldfusion.py:42: number of seconds since Jan 1, 1970 00:00:00
UTC.
you are sending the timestamp in the plaso format, so that would be number of
microseconds since Jan 1....

https://codereview.appspot.com/110090043/diff/150001/plaso/parsers/coldfusion...
plaso/parsers/coldfusion.py:45: super(ColdFusionEventObject,
self).__init__(timestamp,
indent not fixed, 

super(Cold...__init__(
    timestamp, eventda....

as in the timestamp attribute indented as well

https://codereview.appspot.com/110090043/diff/150001/plaso/parsers/coldfusion...
plaso/parsers/coldfusion.py:137: # Date looks like: MM/DD/YYYY; time looks like:
HH:MM:SS.
it doesn't really look like that since you need to do += 2000

https://codereview.appspot.com/110090043/diff/150001/plaso/parsers/coldfusion...
File plaso/parsers/coldfusion_test.py (right):

https://codereview.appspot.com/110090043/diff/150001/plaso/parsers/coldfusion...
plaso/parsers/coldfusion_test.py:71: test_message = (u'Error attempting to
resolve the template '
keep the first line indented as well