JSON Body Processor

[urma]

After a long time tinkering with the mapping between JSON and ARGS, the JSON body processor is finally ready for a first review. A brief description:

• It uses yajl (http://lloyd.github.com/yajl/) for streaming JSON parsing
• Only leaf nodes in the JSON stream will be registered under ARGS, because we currently only assign scalar values to ARGS, not JSON structures:
  { 'data': { 'a': 1, 'b': 2 } } would result in ARGS:data.a = 1 and ARGS:data.b = 2, but ARGS:data would be undefined
• Multiple values for a given key are supported, but due to the previous mapping method, there is no way to distinguish between some scenarios:
  [ { 'data': 1 }, { 'data': 2 } ] would result in ARGS:data = 1 and ARGS:data = 2, while
  { 'data': [ 1, 2 ] } would also result in ARGS:data = 1 and ARGS:data = 2

There are some TODO comments in the msc_json.c code which indicate aspects of the code I am not sure if I got right -- any feedback on those would be much appreaciated.

[brenosilva]

Hello Ulisses,

Could you please provide the patch for trunk ? not to the master. Then i can review in the current development tree.

[urma]

Hello Breno

I forked the project a few hours ago, and applied my changes on the current master of SpiderLabs/ModSecurity -- so, I think this is for trunk already, or am I understanding it wrong?

Also, do you need the actual patch file, or does GitHub's 'Files Changed' section for Pull Requests work as well? Everything that would be included in the patch is available there.

Thanks!

[brenosilva]

Hello Ulisses,

master != trunk. Trunk is our development branch.
Master is our current stable modsecurity version.

So the patch for master will not apply in the current trunk code.

We have some instructions here how to submit the pull request to the trunk http://www.modsecurity.org/developers/

Thanks

[brenosilva]

Ulisses,

Just another small fix... you are trying to change Makefile.in. I think it is not part of this patch right ?

[urma]

Breno

Yes, Makefile.in is not part of the patch, sorry about that. I started working on this in remotes/trunk, and will submit it again later today. Since that would apply to a different branch, I'm assuming once this is finished I should close this pull request and open a second one pointing to the correct branch, right?

[brenosilva]

Yes. Just submit a new one. Then we can close this.

Also.. just a question...

According to RFC 4627 the content type for json is application/json
Should we define the request body processor by default to JSON in modsecurity.c ? like we did for example to multipart ?

I know some applications send a different content types for json, so not sure if this is a good idea.

Thanks

Breno

[brenosilva]

Just a small comment.

Could you please initialize the variables in yajl_map_key, json_add_argument , yajl_map_key and yajl_end_map ?

[urma]

Breno,

I did it based on the current XML body processor model -- I defined a rule which checks the content type and defines the body processor accordingly (I should include that in modsecurity.conf-recommended and include it in the patch). I think that is more flexible because, as you said, some applications do not use the 'official' content type for JSON data.

Regarding variable initialization -- got it, will change the code accordingly.

[brenosilva]

Sounds good.

Thanks