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

feat: optimize report saving #729

Merged
merged 2 commits into from
Mar 3, 2023
Merged

feat: optimize report saving #729

merged 2 commits into from
Mar 3, 2023

Conversation

vjerci
Copy link
Contributor

@vjerci vjerci commented Mar 3, 2023

Description

Previously due to a couple of spiraled bugs one top of each other we were crashing in certain scenarios:

  1. we were storing auxilary rules result in jsonlines report. (this pr addresses this problem)
  2. we have a rule dangerous_insert_html which bascially had auxilary for any kind of string detection.
  3. jsonlines library uses bufio.scanner, which limits token/single json line/ single detection size to 64 kb.

All that above problems coupled with a nasty webpacks compiled javascript which used eval of a string bigger than 64kb disabled us from parsing our worker report file.

This pr optimizes how we use auxilary rules in a way that we don't store them in worker report fixing above mentioned problem 1. and 2.

problem 3 will be dealt with in separate pr

Related

#718

Checklist

  • I've added test coverage that shows my fix or feature works as expected.
  • I've updated or added documentation if required.
  • I've included usage information in the description if CLI behavior was updated or added.
  • PR title follows Conventional Commits format

@vjerci vjerci marked this pull request as ready for review March 3, 2023 17:51
@vjerci vjerci merged commit 86cdb46 into main Mar 3, 2023
@vjerci vjerci deleted the feat/js-auxilary-rules branch March 3, 2023 18:02
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.

2 participants