fix: proper fetching of binary data during get requests

[RobertMirandola]

This MR allows for synchronous and asynchronous GET requests that fetch binary data to be returned in a new field called XMLHttpRequest.response with the expected binary data. XMLHttpRequest.responseText is still text that is encoded in 'utf8' format.

[RobertMirandola]

The original MR for this was wrriten by @georgestagg and was written to be merged into a forked version of this repo by @driverdan: https://github.com/driverdan/node-XMLHttpRequest/pull/198/commits/d251dae286eca9eada9133fff9bdbaeb91ac22f9

The reason I am re opening the MR here is because there were some merge conficts between this repo and the other forked version by @driverdan that did not seem like they were worth resolving. Also it doesn't seem like that forked repo is being maintained given the time of the last commit.

[mjwwit]

Thanks for this! The inactivity of the original repository and other forks of it was the reasoning behind starting this one. As the package built from this repository is depended on by a large number of other packages I'll have to test it thoroughly, so please bear with me.

[RobertMirandola]

@mjwwit Just wanted to let you know that I just added an extra commit dealing with local files containing binary data as well as support for this.response within this.abort()

[RobertMirandola]

@mjwwit I've read your changes and made a commit in response. I did not know that setting binary to the setEncoding function was an alias to latin1. So I instead completely removed response.setEncoding() so that I could receive the raw data in chunks. I just create a buffer now with that raw data with no encoding specified to properly read the binary data. The responseText stays the same, as it always has (a toString() with utf8 encoding). Also, for local files, I got rid of the buffer creation and instead just read the file without any encodings at all to get the correct data for this.response, and the encoded the string, this.responseText, to 'utf8' encoding, as was done before. The duplication of work is now eliminated as well.

Thanks and I look forward to your thoughts!

[RobertMirandola]

@mjwwit I also just commited a test file as well

[mjwwit]

That is tons better! The only thing missing now is a reference to the new file (and the renamed one) in the package.json test script. If you add that I think we're good for a merge. I'm surprised you got through the code, even for very old code it's absolute garbage. Who knows, maybe I'll rewrite it properly some day...

[RobertMirandola]

@mjwwit Done! Thanks so much for taking the time to review this PR, some of the work we do involves downloading raw binary data so trying to find a fix for this repo was unfortunately very necessary since we do use it😆

[mjwwit]

I found a last problem in the code, but it's not (and was never) covered by any tests. As this is not really your problem I'll fix this myself after the merge.

[georgestagg]

Thanks for your further work on this @RobertMirandola & @mjwwit and for getting this change merged. This will be very useful for me too and I look forward to being able to switch from my fork back to a proper npm package!

[mjwwit]

I've released version 2.1.0 with your PR, my fix (and a test to verify)