Skip to content
This repository was archived by the owner on May 14, 2021. It is now read-only.

Fix handling of AdhocReport response (#47)#48

Merged
gschneider-r7 merged 1 commit into
rapid7:masterfrom
asecurityteam:fix-csv-report-parsing
Jan 11, 2018
Merged

Fix handling of AdhocReport response (#47)#48
gschneider-r7 merged 1 commit into
rapid7:masterfrom
asecurityteam:fix-csv-report-parsing

Conversation

@fruechel

@fruechel fruechel commented Jan 9, 2018

Copy link
Copy Markdown
Contributor

Not using XML directly any more. This can cause large reports to fail
parsing. Removed comment that indicated we shouldn't be doing this ;-)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

Not using XML directly any more. This can cause large reports to fail
parsing. Removed comment that indicated we shouldn't be doing this ;-)
@fruechel

fruechel commented Jan 9, 2018

Copy link
Copy Markdown
Contributor Author

@gschneider-r7 this is in relation to #47 and fixes the error we're seeing. The bug was present already but only exposed through my PR #43. Now we're not using as_xml directly but rather only on the part that's actually an XML response. the other part is properly treated as CSV straight away.

Tested the CSV component but not how an XML report is handled. However, the way the code changed there should not be any difference.

@gschneider-r7

Copy link
Copy Markdown
Contributor

I don't have time to test this myself currently, so if you need this merged and published I'll have to take your word for it that it works. 😉 Just let me know if you need me to push it out.

The changes otherwise look fine to me.

@fruechel

fruechel commented Jan 9, 2018

Copy link
Copy Markdown
Contributor Author

Yup, we're currently pinned to the dev commit in my repo and it keeps happily importing. Haven't seen any issues since we pushed out this fix. Thanks.

@gschneider-r7 gschneider-r7 merged commit af01267 into rapid7:master Jan 11, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants