Bug 1363581 - (part 1) Buffer RESTRequest response in memory before decoding to avoid corruption r?markh
MozReview-Commit-ID: BS0h4iIr91V
--- a/services/common/rest.js
+++ b/services/common/rest.js
@@ -13,17 +13,47 @@ ChromeUtils.import("resource://gre/modul
ChromeUtils.import("resource://gre/modules/NetUtil.jsm");
ChromeUtils.import("resource://gre/modules/XPCOMUtils.jsm");
ChromeUtils.import("resource://gre/modules/Log.jsm");
ChromeUtils.import("resource://services-common/utils.js");
ChromeUtils.defineModuleGetter(this, "CryptoUtils",
"resource://services-crypto/utils.js");
-const Prefs = new Preferences("services.common.");
+function decodeString(data, charset) {
+ if (!data || !charset) {
+ return data;
+ }
+
+ // This could be simpler if we assumed the charset is only ever UTF-8.
+ // It's unclear to me how willing we are to assume this, though...
+ let stringStream = Cc["@mozilla.org/io/string-input-stream;1"]
+ .createInstance(Ci.nsIStringInputStream);
+ stringStream.setData(data, data.length);
+
+ let converterStream = Cc["@mozilla.org/intl/converter-input-stream;1"]
+ .createInstance(Ci.nsIConverterInputStream);
+
+ converterStream.init(stringStream, charset, 0,
+ converterStream.DEFAULT_REPLACEMENT_CHARACTER);
+
+ let remaining = data.length;
+ let body = "";
+ while (remaining > 0) {
+ let str = {};
+ let num = converterStream.readString(remaining, str);
+ if (!num) {
+ break;
+ }
+ remaining -= num;
+ body += str.value;
+ }
+ return body;
+}
+
/**
* Single use HTTP requests to RESTish resources.
*
* @param uri
* URI for the request. This can be an nsIURI object or a string
* that can be used to create one. An exception will be thrown if
* the string is not a valid URI.
@@ -82,18 +112,17 @@ function RESTRequest(uri) {
// 'uri' isn't a valid URI string.
if (!(uri instanceof Ci.nsIURI)) {
uri = Services.io.newURI(uri);
}
this.uri = uri;
this._headers = {};
this._log = Log.repository.getLogger(this._logName);
- this._log.level =
- Log.Level[Prefs.get("log.logger.rest.request")];
+ this._log.manageLevelFromPref("services.common.log.logger.rest.request");
}
RESTRequest.prototype = {
_logName: "Services.Common.RESTRequest",
QueryInterface: XPCOMUtils.generateQI([
Ci.nsIBadCertListener2,
Ci.nsIInterfaceRequestor,
@@ -427,20 +456,18 @@ RESTRequest.prototype = {
return;
}
this.status = this.IN_PROGRESS;
this._log.trace("onStartRequest: " + channel.requestMethod + " " +
channel.URI.spec);
- // Create a response object and fill it with some data.
- let response = this.response = new RESTResponse();
- response.request = this;
- response.body = "";
+ // Create a new response object.
+ this.response = new RESTResponse(this);
this.delayTimeout();
},
onStopRequest: function onStopRequest(channel, context, statusCode) {
if (this.timeoutTimer) {
// Clear the abort timer now that the channel is done.
this.timeoutTimer.clear();
@@ -468,16 +495,30 @@ RESTRequest.prototype = {
if (!this.onComplete) {
this._log.error("Unexpected error: onComplete not defined in " +
"abortRequest.");
this.onProgress = null;
return;
}
+ try {
+ // Decode this.response._rawBody,
+ this.response.body = decodeString(this.response._rawBody, this.response.charset);
+ this.response._rawBody = null;
+ // Call the 'progress' callback a single time with all the data.
+ this.onProgress();
+ } catch (ex) {
+ this._log.warn(`Exception handling response - ${this.method} ${channel.URI.spec}`, ex);
+ this.status = this.ABORTED;
+ this.onComplete(ex);
+ this.onComplete = this.onProgress = null;
+ return;
+ }
+
// Throw the failure code and stop execution. Use Components.Exception()
// instead of Error() so the exception is QI-able and can be passed across
// XPCOM borders while preserving the status code.
if (!statusSuccess) {
let message = Components.Exception("", statusCode).name;
let error = Components.Exception(message, statusCode);
this._log.debug(this.method + " " + uri + " failed: " + statusCode + " - " + message);
this.onComplete(error);
@@ -511,71 +552,27 @@ RESTRequest.prototype = {
}
this.onComplete = this.onProgress = null;
return;
}
if (channel.contentCharset) {
this.response.charset = channel.contentCharset;
-
- if (!this._converterStream) {
- this._converterStream = Cc["@mozilla.org/intl/converter-input-stream;1"]
- .createInstance(Ci.nsIConverterInputStream);
- }
- this._converterStream.init(stream, channel.contentCharset, 0,
- this._converterStream.DEFAULT_REPLACEMENT_CHARACTER);
-
- try {
- let remaining = count;
- while (remaining > 0) {
- let str = {};
- let num = this._converterStream.readString(remaining, str);
- if (!num) {
- break;
- }
- remaining -= num;
- this.response.body += str.value;
- }
- } catch (ex) {
- this._log.warn("Exception thrown reading " + count + " bytes from " +
- "the channel", ex);
- throw ex;
- }
} else {
this.response.charset = null;
-
- if (!this._inputStream) {
- this._inputStream = Cc["@mozilla.org/scriptableinputstream;1"]
- .createInstance(Ci.nsIScriptableInputStream);
- }
-
- this._inputStream.init(stream);
-
- this.response.body += this._inputStream.read(count);
}
- try {
- this.onProgress();
- } catch (ex) {
- this._log.warn("Got exception calling onProgress handler, aborting " +
- this.method + " " + channel.URI.spec, ex);
- this.abort();
+ if (!this._inputStream) {
+ this._inputStream = Cc["@mozilla.org/scriptableinputstream;1"]
+ .createInstance(Ci.nsIScriptableInputStream);
+ }
+ this._inputStream.init(stream);
- if (!this.onComplete) {
- this._log.error("Unexpected error: onComplete not defined in " +
- "onDataAvailable.");
- this.onProgress = null;
- return;
- }
-
- this.onComplete(ex);
- this.onComplete = this.onProgress = null;
- return;
- }
+ this.response._rawBody += this._inputStream.read(count);
this.delayTimeout();
},
/** nsIInterfaceRequestor **/
getInterface(aIID) {
return this.QueryInterface(aIID);
@@ -637,20 +634,22 @@ RESTRequest.prototype = {
callback.onRedirectVerifyCallback(Cr.NS_OK);
}
};
/**
* Response object for a RESTRequest. This will be created automatically by
* the RESTRequest.
*/
-function RESTResponse() {
+function RESTResponse(request = null) {
+ this.body = "";
+ this._rawBody = "";
+ this.request = request;
this._log = Log.repository.getLogger(this._logName);
- this._log.level =
- Log.Level[Prefs.get("log.logger.rest.response")];
+ this._log.manageLevelFromPref("services.common.log.logger.rest.response");
}
RESTResponse.prototype = {
_logName: "Services.Common.RESTResponse",
/**
* Corresponding REST request
*/
--- a/services/common/tests/unit/test_restrequest.js
+++ b/services/common/tests/unit/test_restrequest.js
@@ -136,17 +136,16 @@ add_test(function test_get() {
request.onProgress = request.onComplete = function() {
do_throw("This function should have been overwritten!");
};
let onProgress_called = false;
function onProgress() {
onProgress_called = true;
- Assert.equal(this.status, request.IN_PROGRESS);
Assert.ok(this.response.body.length > 0);
Assert.ok(!!(this.channel.loadFlags & Ci.nsIRequest.LOAD_BYPASS_CACHE));
Assert.ok(!!(this.channel.loadFlags & Ci.nsIRequest.INHIBIT_CACHING));
}
function onComplete(error) {
Assert.equal(error, null);
@@ -317,17 +316,16 @@ function check_posting_data(method) {
request.onProgress = request.onComplete = function() {
do_throw("This function should have been overwritten!");
};
let onProgress_called = false;
function onProgress() {
onProgress_called = true;
- Assert.equal(this.status, request.IN_PROGRESS);
Assert.ok(this.response.body.length > 0);
}
function onComplete(error) {
Assert.equal(error, null);
Assert.equal(this.status, this.COMPLETED);
Assert.ok(this.response.success);
@@ -387,17 +385,16 @@ add_test(function test_delete() {
request.onProgress = request.onComplete = function() {
do_throw("This function should have been overwritten!");
};
let onProgress_called = false;
function onProgress() {
onProgress_called = true;
- Assert.equal(this.status, request.IN_PROGRESS);
Assert.ok(this.response.body.length > 0);
}
function onComplete(error) {
Assert.equal(error, null);
Assert.equal(this.status, this.COMPLETED);
Assert.ok(this.response.success);