Skip to content

Commit

Permalink
Fix an issue where the browser throws an exception on responseXML
Browse files Browse the repository at this point in the history
When trying to use the XHR api set with a content type of 'text' the
browser throws an error it is unable to read responseXML as the response
isn't valid xml and the response type is not an empty string or
'document' as according to spec.

this change was implemented due to inconsistent implementation across
browsers and 3rd party network libraries which do parse xml responses
even when the response type is not set according to the spec.

In order to support all clients we now try to assign the responseXML
property when possible but fail silently when we cannot.

This will keep both behaviors consistent as incorrectly implemented
clients will be able to read the XML response while still supporting the
correctly implemented clients to not fail the request when trying to
access responseXML on an invalid request.

please see issues #86 #89 for more information.
  • Loading branch information
morsdyce committed Jun 11, 2018
1 parent 7fefce8 commit 0a8e16a
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 5 deletions.
10 changes: 7 additions & 3 deletions dist/xhook.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// XHook - v1.4.8 - https://github.com/jpillora/xhook
// Jaime Pillora <[email protected]> - MIT Copyright 2018
(function(undefined) {
(function(window,undefined) {
var AFTER, BEFORE, COMMON_EVENTS, EventEmitter, FETCH, FIRE, FormData, NativeFetch, NativeFormData, NativeXMLHttp, OFF, ON, READY_STATE, UPLOAD_EVENTS, WINDOW, XHookFetchRequest, XHookFormData, XHookHttpRequest, XMLHTTP, convertHeaders, depricatedProp, document, fakeEvent, mergeObjects, msie, nullify, proxyEvents, slice, useragent, xhook, _base,
__indexOf = [].indexOf || function(item) { for (var i = 0, l = this.length; i < l; i++) { if (i in this && this[i] === item) return i; } return -1; };

Expand Down Expand Up @@ -333,7 +333,11 @@ XHookHttpRequest = WINDOW[XMLHTTP] = function() {
if (!xhr.responseType || xhr.responseType === "text") {
response.text = xhr.responseText;
response.data = xhr.responseText;
response.xml = xhr.responseXML;
try {
response.xml = xhr.responseXML;
} catch (_error) {

}
} else if (xhr.responseType === "document") {
response.xml = xhr.responseXML;
response.data = xhr.responseXML;
Expand Down Expand Up @@ -693,4 +697,4 @@ if (typeof define === "function" && define.amd) {
WINDOW.xhook = xhook;
}

}.call(this));
}.call(this,window));
2 changes: 1 addition & 1 deletion dist/xhook.min.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 7 additions & 1 deletion src/xhook.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,13 @@ XHookHttpRequest = WINDOW[XMLHTTP] = ->
if !xhr.responseType or xhr.responseType is "text"
response.text = xhr.responseText
response.data = xhr.responseText
response.xml = xhr.responseXML
try
response.xml = xhr.responseXML
catch
# unable to set responseXML due to response type, we attempt to assign responseXML
# when the type is text even though it's against the spec due to several libraries
# and browser vendors who allow this behavior. causing these requests to fail when
# xhook is installed on a page.
else if xhr.responseType is "document"
response.xml = xhr.responseXML
response.data = xhr.responseXML
Expand Down

0 comments on commit 0a8e16a

Please sign in to comment.