Use OS.File for file reads in Zotero.File.get(Binary)ContentsAsync()

This is the recommended approach (since NetUtil can still do some main-thread
I/O for files) and avoids warnings in the console.

For getContentsAsync(), also sends nsIURIs and string URIs to
Zotero.HTTP.request(), which should be used instead.

This makes getBinaryContentsAsync() much slower (due to the conversion from an
array of bytes to a binary string), but it's only used in tests. For one test
that compares two large files, use MD5 instead.

chrome/content/zotero/xpcom/file.js

@@ -109,14 +109,7 @@ Zotero.File = new function(){
 	 */
 	this.getSample = function (file) {
 		var bytes = 200;
-		return this.getContentsAsync(file, null, bytes)
+		return this.getContentsAsync(file, null, bytes);
-		.catch(function (e) {
-			if (e.name == 'NS_ERROR_ILLEGAL_INPUT') {
-				Zotero.debug("Falling back to raw bytes");
-				return this.getBinaryContentsAsync(file, bytes);
-			}
-			throw e;
-		}.bind(this));
 	}
 	
 	
@@ -199,110 +192,119 @@ Zotero.File = new function(){
 	/**
 	 * Get the contents of a text source asynchronously
 	 *
-	 * @param {nsIURI|nsIFile|string spec|string path|nsIChannel|nsIInputStream} source The source to read
+	 * @param {string path|nsIFile|file URI|nsIChannel|nsIInputStream} source The source to read
 	 * @param {String} [charset] The character set; defaults to UTF-8
 	 * @param {Integer} [maxLength] Maximum length to fetch, in bytes
 	 * @return {Promise} A promise that is resolved with the contents of the file
 	 */
-	this.getContentsAsync = function (source, charset, maxLength) {
+	this.getContentsAsync = Zotero.Promise.coroutine(function* (source, charset, maxLength) {
 		Zotero.debug("Getting contents of "
 			+ (source instanceof Components.interfaces.nsIFile
 				? source.path
 				: (source instanceof Components.interfaces.nsIInputStream ? "input stream" : source)));
 		
-		// If path is given, convert to file:// URL
+		// Send URIs to Zotero.HTTP.request()
-		if (typeof source == 'string' && !source.match(/^file:/)) {
+		if (source instanceof Components.interfaces.nsIURI
-			source = 'file://' + source;
+				|| typeof source == 'string' && !source.startsWith('file:') && source.match(/^[a-z]{3,}:/)) {
+			Zotero.logError("Passing a URI to Zotero.File.getContentsAsync() is deprecated "
+				+ "-- use Zotero.HTTP.request() instead");
+			return Zotero.HTTP.request("GET", source);
 		}
 		
-		var options = {
+		// Use NetUtil.asyncFetch() for input streams and channels
-			charset: charset ? charset : "UTF-8",
+		if (source instanceof Components.interfaces.nsIInputStream
-			replacement: 65533
+				|| source instanceof Components.interfaces.nsIChannel) {
-		};
+			var deferred = Zotero.Promise.defer();
-		
+			try {
-		var deferred = Zotero.Promise.defer();
+				NetUtil.asyncFetch(source, function(inputStream, status) {
-		try {
+					if (!Components.isSuccessCode(status)) {
-			NetUtil.asyncFetch(source, function(inputStream, status) {
+						deferred.reject(new Components.Exception("File read operation failed", status));
-				if (!Components.isSuccessCode(status)) {
-					deferred.reject(new Components.Exception("File read operation failed", status));
-					return;
-				}
-				
-				try {
-					try {
-						var bytesToFetch = inputStream.available();
-					}
-					catch (e) {
-						// The stream is closed automatically when end-of-file is reached,
-						// so this throws for empty files
-						if (e.name == "NS_BASE_STREAM_CLOSED") {
-							Zotero.debug("RESOLVING2");
-							deferred.resolve("");
-						}
-						deferred.reject(e);
-					}
-					
-					if (maxLength && maxLength < bytesToFetch) {
-						bytesToFetch = maxLength;
-					}
-					
-					if (bytesToFetch == 0) {
-						deferred.resolve("");
 						return;
 					}
 					
-					deferred.resolve(NetUtil.readInputStreamToString(
+					try {
-						inputStream,
+						try {
-						bytesToFetch,
+							var bytesToFetch = inputStream.available();
-						options
+						}
-					));
+						catch (e) {
-				}
+							// The stream is closed automatically when end-of-file is reached,
-				catch (e) {
+							// so this throws for empty files
-					deferred.reject(e);
+							if (e.name == "NS_BASE_STREAM_CLOSED") {
-				}
+								Zotero.debug("RESOLVING2");
-			});
+								deferred.resolve("");
+							}
+							deferred.reject(e);
+						}
+						
+						if (maxLength && maxLength < bytesToFetch) {
+							bytesToFetch = maxLength;
+						}
+						
+						if (bytesToFetch == 0) {
+							deferred.resolve("");
+							return;
+						}
+						
+						deferred.resolve(NetUtil.readInputStreamToString(
+							inputStream,
+							bytesToFetch,
+							options
+						));
+					}
+					catch (e) {
+						deferred.reject(e);
+					}
+				});
+			}
+			catch(e) {
+				// Make sure this get logged correctly
+				Zotero.logError(e);
+				throw e;
+			}
+			return deferred.promise;
 		}
-		catch(e) {
+		
-			// Make sure this get logged correctly
+		// Use OS.File for files
-			Zotero.logError(e);
+		if (source instanceof Components.interfaces.nsIFile) {
-			throw e;
+			source = source.path;
 		}
-		return deferred.promise;
+		else if (source.startsWith('^file:')) {
-	}
+			source = OS.Path.fromFileURI(source);
+		}
+		var options = {
+			encoding: charset ? charset : "utf-8"
+		};
+		if (maxLength) {
+			options.bytes = maxLength;
+		}
+		return OS.File.read(source, options);
+	});
 	
 	
 	/**
 	 * Get the contents of a binary source asynchronously
 	 *
-	 * @param {nsIURI|nsIFile|string spec|nsIChannel|nsIInputStream} source The source to read
+	 * This is quite slow and should only be used in tests.
-	 * @param {Integer} [maxLength] Maximum length to fetch, in bytes (unimplemented)
+	 *
-	 * @return {Promise} A promise that is resolved with the contents of the source
+	 * @param {string path|nsIFile|file URI} source The source to read
+	 * @param {Integer} [maxLength] Maximum length to fetch, in bytes
+	 * @return {Promise<String>} A promise for the contents of the source as a binary string
 	 */
-	this.getBinaryContentsAsync = function (source, maxLength) {
+	this.getBinaryContentsAsync = Zotero.Promise.coroutine(function* (source, maxLength) {
-		if (typeof source == 'string') {
+		// Use OS.File for files
-			source = this.pathToFile(source);
+		if (source instanceof Components.interfaces.nsIFile) {
+			source = source.path;
 		}
-		var deferred = Zotero.Promise.defer();
+		else if (source.startsWith('^file:')) {
-		NetUtil.asyncFetch(source, function(inputStream, status) {
+			source = OS.Path.fromFileURI(source);
-			if (!Components.isSuccessCode(status)) {
+		}
-				deferred.reject(new Components.Exception("Source read operation failed", status));
+		var options = {};
-				return;
+		if (maxLength) {
-			}
+			options.bytes = maxLength;
-			try {
+		}
-				var availableBytes = inputStream.available();
+		var buf = yield OS.File.read(source, options);
-				deferred.resolve(
+		return [...buf].map(x => String.fromCharCode(x)).join("");
-					NetUtil.readInputStreamToString(
+	});
-						inputStream,
-						maxLength ? Math.min(maxLength, availableBytes) : availableBytes
-					)
-				);
-			}
-			catch (e) {
-				deferred.reject(e);
-			}
-		});
-		return deferred.promise;
-	}
 	
 	
 	/*

test/tests/fileTest.js

@@ -38,8 +38,43 @@ describe("Zotero.File", function () {
 			assert.lengthOf(contents, 3);
 			assert.equal(contents, "A\uFFFDB");
 		})
+		
+		it("should respect maxLength", function* () {
+			var contents = yield Zotero.File.getContentsAsync(
+				OS.Path.join(getTestDataDirectory().path, "test.txt"),
+				false,
+				6
+			);
+			assert.lengthOf(contents, 6);
+			assert.equal(contents, "Zotero");
+		});
 	})
 	
+	describe("#getBinaryContentsAsync()", function () {
+		var magicPNG = ["89", "50", "4e", "47", "0d", "0a", "1a", "0a"].map(x => parseInt(x, 16));
+		
+		it("should return a binary string", function* () {
+			var contents = yield Zotero.File.getBinaryContentsAsync(
+				OS.Path.join(getTestDataDirectory().path, "test.png")
+			);
+			assert.isAbove(contents.length, magicPNG.length);
+			for (let i = 0; i < magicPNG.length; i++) {
+				assert.equal(magicPNG[i], contents.charCodeAt(i));
+			}
+		});
+		
+		it("should respect maxLength", function* () {
+			var contents = yield Zotero.File.getBinaryContentsAsync(
+				OS.Path.join(getTestDataDirectory().path, "test.png"),
+				magicPNG.length
+			);
+			assert.lengthOf(contents, magicPNG.length)
+			for (let i = 0; i < contents.length; i++) {
+				assert.equal(magicPNG[i], contents.charCodeAt(i));
+			}
+		});
+	});
+	
 	describe("#copyDirectory()", function () {
 		it("should copy all files within a directory", function* () {
 			var tmpDir = Zotero.getTempDirectory().path;

test/tests/fulltextTest.js

@@ -109,10 +109,12 @@ describe("Zotero.Fulltext", function () {
 			yield Zotero.Fulltext.downloadPDFTool('info', pdfToolsVersion);
 			
 			assert.ok(Zotero.Fulltext.pdfInfoIsRegistered());
+			
 			assert.equal(
-				(yield Zotero.File.getBinaryContentsAsync(cacheExecPath)),
+				(yield Zotero.Utilities.Internal.md5Async(cacheExecPath, false)),
-				(yield Zotero.File.getBinaryContentsAsync(execPath))
+				(yield Zotero.Utilities.Internal.md5Async(execPath, false))
 			);
+			
 			if (!Zotero.isWin) {
 				assert.equal((yield OS.File.stat(execPath)).unixMode, 0o755);
 			}