Skip to content

Commit

Permalink
Merge pull request #342 from ably/cloudflare-xhr
Browse files Browse the repository at this point in the history
Disable upgrade to xhr_streaming if cloudflare headers detected
  • Loading branch information
SimonWoolf authored Oct 21, 2016
2 parents 30041cd + 8dcfb40 commit c8bb648
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 7 deletions.
15 changes: 10 additions & 5 deletions browser/lib/transport/xhrrequest.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ var XHRRequest = (function() {
return isIE && (version = ieVersion()) && version === 10;
}

function getContentType(xhr) {
return xhr.getResponseHeader && xhr.getResponseHeader('content-type');
function getHeader(xhr, header) {
return xhr.getResponseHeader && xhr.getResponseHeader(header);
}

/* Safari mysteriously returns 'Identity' for transfer-encoding
Expand Down Expand Up @@ -158,8 +158,9 @@ var XHRRequest = (function() {

function onEnd() {
try {
var contentType = getContentType(xhr),
headers = null,
var contentType = getHeader(xhr, 'content-type'),
headers,
server,
json = contentType ? (contentType == 'application/json') : (xhr.responseType == 'text');

responseBody = json ? xhr.responseText : xhr.response;
Expand All @@ -178,6 +179,10 @@ var XHRRequest = (function() {
successResponse = (statusCode < 400);
headers = responseBody.headers;
responseBody = responseBody.response;
} else {
headers = {};
if(contentType) { headers['content-type'] = contentType };
if(server = getHeader(xhr, 'server')) { headers['server'] = server };
}
} catch(e) {
var err = new Error('Malformed response body from server: ' + e.message);
Expand All @@ -187,7 +192,7 @@ var XHRRequest = (function() {
}

if(successResponse) {
self.complete(null, responseBody, headers || (contentType && {'content-type': contentType}), unpacked);
self.complete(null, responseBody, headers, unpacked);
return;
}

Expand Down
17 changes: 16 additions & 1 deletion common/lib/transport/comettransport.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,20 @@ var CometTransport = (function() {
REQ_RECV_POLL = 2,
REQ_RECV_STREAM = 3;

function actOnConnectHeaders(headers, host, connectionManager) {
if(headers && headers.server && (headers.server.indexOf('cloudflare') > -1)) {
/* Cloudflare doesn't support xhr streaming */
var blacklist = connectionManager.transportHostBlacklist[host];
if(!blacklist) {
connectionManager.transportHostBlacklist[host] = ['xhr_streaming'];
return;
}
if(!Utils.arrIn(blacklist, 'xhr_streaming')) {
blacklist.push('xhr_streaming');
}
}
}

/*
* A base comet transport class
*/
Expand Down Expand Up @@ -64,7 +78,8 @@ var CometTransport = (function() {
}
self.onData(data);
});
connectRequest.on('complete', function(err) {
connectRequest.on('complete', function(err, _body, headers) {
actOnConnectHeaders(headers, host, self.connectionManager);
if(!self.recvRequest) {
/* the transport was disposed before we connected */
err = err || new ErrorInfo('Request cancelled', 80000, 400);
Expand Down
8 changes: 7 additions & 1 deletion common/lib/transport/connectionmanager.js
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,8 @@ var ConnectionManager = (function() {
* transport, it'll just be that one. */
this.baseTransport = Utils.intersect(Defaults.transports, this.transports)[0];
this.upgradeTransports = Utils.intersect(this.transports, Defaults.upgradeTransports);
/* Map of hosts to an array of transports to not be tried for that host */
this.transportHostBlacklist = {};
this.transportPreference = null;

this.httpHosts = Defaults.getHosts(options);
Expand Down Expand Up @@ -218,8 +220,12 @@ var ConnectionManager = (function() {
* @param callback
*/
ConnectionManager.prototype.tryATransport = function(transportParams, candidate, callback) {
var self = this;
var self = this, host = transportParams.host;
Logger.logAction(Logger.LOG_MICRO, 'ConnectionManager.tryATransport()', 'trying ' + candidate);
if((host in this.transportHostBlacklist) && Utils.arrIn(this.transportHostBlacklist[host], candidate)) {
Logger.logAction(Logger.LOG_MINOR, 'ConnectionManager.tryATransport()', candidate + ' transport is blacklisted for host ' + transportParams.host);
return;
}
(ConnectionManager.supportedTransports[candidate]).tryConnect(this, this.realtime.auth, transportParams, function(err, transport) {
var state = self.state;
if(state == self.states.closing || state == self.states.closed || state == self.states.failed) {
Expand Down

0 comments on commit c8bb648

Please sign in to comment.