sync policy, MIME override, encoding support, and more

[Bhpsngum]

Synchronous policy and fixes

• Add new non-standard option syncPolicy: "warn" | "enabled" | "disabled" (default is "warn") to explicitly enable/disable synchronous XHR with or without warning
  • "warn": enable synchronous xhr but shows a warning when the user attempts to call xhr.open() with sync flag:

    [Deprecation] Synchronous XMLHttpRequest is deprecated because of its detrimental effects to the end user's experience. For more information, see https://xhr.spec.whatwg.org/#sync-flag

  • "enabled": enable synchronous xhr, no warnings
  • "disabled": disable synchronous, calling xhr.open() with sync flag won't show any warning, but an exception is thrown when calling xhr.send() afterwards:

    Synchronous requests are disabled for this instance.

• Fix problem with data not encoded properly when using synchronous request (XMLHttpRequest .response encoding error when .responseType is "arraybuffer" and synchronous #29)
• Sync responses now write data into OS temporary directory instead of cwd (since rwx permissions can be altered for cwd)
• If file system operations fail, an error message will be thrown:

  Synchronous operation aborted: Unable to access the OS temporary directory for read/write operations.

Encoding and MIME type support

• Add xhr.overrideMimeType(type) (Missing overrideMimeType method #27)
• Add support for encodings other than utf-8 (can be read from Content-Type header or through user's custom encoding via xhr.overrideMimeType())

Non-standard methods

• Removed xhr.setDisableHeaderCheck() and instead added a non-standard disableHeaderCheck: true | false (default is false)
• Add deprecation message for xhr.getRequestHeader():

  xhr.getRequestHeader() is deprecated and will be removed in a future release. It’s non-standard and not part of the XHR spec.

Others

• Prevent XMLHttpRequest to be called without new operator
• Unifies createFileOrSyncResponse and createResponse into one method createResponse
• Brings some methods back to private scope to prevent tampering (especially the readyState value)
• Add immutable flags value UNSENT, OPENED, HEADERS_RECEIVED, LOADING, and DONE to both XMLHttpRequest and any newly-created xhr object
• Add two more options, textDecoder: <func> for users to specify custom text decoder (to support more range of encodings for old Node versions) and xmlParser: <func> to support parsing for xhr.responseXML

[Bhpsngum]

A few note on this: Since TextDecoder only adds support for more encodings on later node versions after v12, the test on v12 failed

We can either increase the minimum node version limit or change to use an external dependency for decoding (such as iconv-lite), there is a new option called textEncoder so user can pass their custom decoder into.

[mjwwit]

At first glance this looks really solid, thanks for the great work! ❤️
I'll have a more detailed look in the coming days, but we can probably get this merged this week.

We can either increase the minimum node version limit or change to use an external dependency for decoding (such as iconv-lite), there is a new option called textEncoder so user can pass their custom decoder into.

I have no problems raising the minimum version to 13 or 14.

[Bhpsngum]

I have no idea if we can keep the minimum version as v12, with caveats that encoding won't work with many options and need to specify a custom decoder (textDecoder: function (buf, enc) { return iconv.decode(buf, enc) } for example), or we raise it to the version where it already supports more options.

Please let me know what you think about this.

[mjwwit]

I have no idea if we can keep the minimum version as v12, with caveats that encoding won't work with many options and need to specify a custom decoder (textDecoder: function (buf, enc) { return iconv.decode(buf, enc) } for example), or we raise it to the version where it already supports more options.

Please let me know what you think about this.

Keeping the minimum version with caveats is probably a bad idea. People will probably not read about these caveats proactively, and the package won't be fully functional on Node.js 12. I'll make the next release a major. People can still choose not to upgrade because of that.

[Bhpsngum]

alright, that sounds good for me too 👍

[mjwwit]

Looking good! The syncPolicy "disabled" behavior contains a bug though; when sending an async request it still throws an error. Maybe this would also be a good candidate for a test.

[Bhpsngum]

I've fixed the problems you addressed, and went further and added a basic measure to anti-prototype pollution, there is also a new test i added for that purpose (test-pollution.js).

[mjwwit]

It looks complete now, I'll run some usage tests to see if I can discover any problems. I'm not expecting to find anything, so I'll probably merge quickly after that. The release will follow shortly after that, probably on the same day.

[Bhpsngum]

Alright, all sounds good to me 👍

[mjwwit]

Thanks again for the solid work ❤️. I've released it as version 4.0.0.