Skip to content

Conversation

bjh7242
Copy link
Contributor

@bjh7242 bjh7242 commented Mar 17, 2016

Implemented remove_comments_char transformation for libmodsecurity.

Removes the following characters

  • /*
  • */

@zimmerle zimmerle self-assigned this Mar 24, 2016
@zimmerle zimmerle modified the milestones: v3.0.0, v3.0.0 owasp compatible, v3.0.0 feature complete Mar 24, 2016
@bjh7242 bjh7242 closed this Apr 1, 2016
@bjh7242 bjh7242 reopened this Apr 1, 2016
@zimmerle
Copy link
Contributor

zimmerle commented Apr 4, 2016

Hi @bjh7242,

Thanks for the patch. I will try to have a look on it today.

zimmerle added a commit to owasp-modsecurity/secrules-language-tests that referenced this pull request Apr 4, 2016
zimmerle added a commit that referenced this pull request Apr 4, 2016
zimmerle added a commit that referenced this pull request Apr 4, 2016
Trimming the pull request #1098
@zimmerle
Copy link
Contributor

zimmerle commented Apr 4, 2016

Hi @bjh7242,

Your pull request is now merged. Thanks again for patch.

Few comments:

Usually, before add a functionality to version 3 we expect to have regression tests that covers its basic usage, plus a few corner cases. Those unit tests are very important, among of other things, those tests are executed by our buildbots, so we can make sure that the functionality is really working in all supported platforms.

Other detail that you may want to consider is the fact that we try to not use lines bigger than 80 characters.

Changes that I have made:

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