Skip to content
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

Preallocate memory when SecStreamInBodyInspection is on #1538

Conversation

allanbomsft
Copy link

This is an improved fix for #1366 .

  • Preallocate memory when SecStreamInBodyInspection is on. On my dev box this gave 20x speed improvement for 10mb upload (and even more for larger files).
  • Simplified the structure of modsecurity_request_body_to_stream
  • Removed null termination for the stream_input_data buffer, as I did not see a reason for having this. The buffer is not a string. A request body can contain binary data.
  • Fixed a typo in build_yajl.bat
  • Added a check of the return value of modsecurity_request_body_to_stream

Thanks!

@zimmerle zimmerle self-requested a review August 31, 2017 11:32
@zimmerle zimmerle self-assigned this Oct 5, 2017
zimmerle pushed a commit that referenced this pull request Oct 5, 2017
@zimmerle
Copy link
Contributor

zimmerle commented Oct 5, 2017

Fixed a typo in build_yajl.bat -> merged! thanks!

Looking the other stuff.

zimmerle pushed a commit that referenced this pull request Oct 5, 2017
zimmerle pushed a commit that referenced this pull request Oct 6, 2017
@zimmerle
Copy link
Contributor

zimmerle commented Oct 6, 2017

Indeed, that seems to be a good optimization. However, it may not be an optimization for certain users where the uploads are very small, therefore making it an optional compilation flag.

Thanks @allanbomsft

@zimmerle zimmerle closed this Oct 6, 2017
zimmerle pushed a commit that referenced this pull request Oct 6, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants