Skip to content

Implemented removeWhitespace transformation - Issue #972 #1115

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 5 commits into from
Closed

Implemented removeWhitespace transformation - Issue #972 #1115

wants to merge 5 commits into from

Conversation

bjh7242
Copy link
Contributor

@bjh7242 bjh7242 commented Apr 5, 2016

Implemented removeWhitespace transformation to address #972

Removes the following characters:

  • space (' ')
  • form feed ('\f')
  • line feed ('\n')
  • carriage return ('\r')
  • horizontal tab ('\t')
  • vertical tab ('\v')
  • non breaking space character

This patch passes the existing unit tests from https://github.com/SpiderLabs/secrules-language-tests/blob/master/transformations/removeWhitespace.json

@zimmerle
Copy link
Contributor

zimmerle commented Apr 5, 2016

Hi @bjh7242,

Thanks for the patch!!

Few comments:

  1. You may want to put your name on the git configuration
  2. You may want to create a new branch for each pull request, so that you always get a fresh tree. For instance:
$ git checkout libmodsecurity
$ git checkout -b libmodsecurity_remove_whitespace
  1. There was some coding style warnings:
warning: ./ModSecurity/src/actions/transformations/remove_whitespace.cc:42:  Use int16/int64/etc, rather than the C type long  [runtime/int] [4]
warning: ./ModSecurity/src/actions/transformations/remove_whitespace.cc:45:  Missing space before ( in while(  [whitespace/parens] [5]
warning: ./ModSecurity/src/actions/transformations/remove_whitespace.cc:47:  Missing spaces around ||  [whitespace/operators] [3]
warning: ./ModSecurity/src/actions/transformations/remove_whitespace.cc:48:  Line ends in whitespace.  Consider deleting these extra spaces.  [whitespace/end_of_line] [4]
warning: ./ModSecurity/src/actions/transformations/remove_whitespace.cc:50:  An else should appear on the same line as the preceding }  [whitespace/newline] [4]
warning: ./ModSecurity/src/actions/transformations/remove_whitespace.cc:50:  If an else has a brace on one side, it should have it on both  [readability/braces] [5]

Modification that I did are available here: 1539a8c

Buildbots output after mine modifications:
http://www.modsecurity.org/developers/buildbot/libmodsecurity/builders/Linux64%20-%20Linux/builds/167/steps/coding%20style/logs/warnings%20%2833%29

Patch was merged! Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants