[Pkg-javascript-commits] [node-stream-http] 79/208: Make content-type header correct by default

[Bastien Roucariès]

This is an automated email from the git hooks/post-receive script.

rouca pushed a commit to branch master
in repository node-stream-http.

commit da4085e33d4b2469dd1d70c5d0cd939a0e8177a3
Author: John Hiesey <john at hiesey.com>
Date:   Thu Aug 20 00:54:17 2015 -0700

    Make content-type header correct by default
    
    The default behavior now preserves the 'content-type' header, at
    the expense of not providing pseudo-streming in Safari, generic
    WebKit, and older Chrome. Applications that require streaming
    and already use 'prefer-stream' mode will not be affected.
    
    The 'prefer-fast' mode is now deprecated, as that behavior now
    matches the default. The 'allow-wrong-content-type' mode is new;
    it gives the same behavior as the old default in case binary
    streaming is essential.
    
    Fixes #8
---
 README.md                        | 19 +++++++++++++------
 lib/capability.js                |  4 +++-
 lib/request.js                   | 13 +++++++------
 test/browser/binary-streaming.js |  7 +++++--
 test/browser/headers.js          | 29 +++++++++++++++++++++++++++++
 5 files changed, 57 insertions(+), 15 deletions(-)

diff --git a/README.md b/README.md
index 0b6f403..416a9a3 100644
--- a/README.md
+++ b/README.md
@@ -51,14 +51,21 @@ but sometimes it helps to get a little input from the user.
 * The `options.mode` field passed into `http.request` or `http.get` can take on one of the
 following values:
   * 'default' (or any falsy value, including undefined): Try to provide partial data before
-the equest completes, but not at the cost of correctness for binary data. In some cases
-the implementation may be a bit slow.
+the request completes, but not at the cost of correctness for binary data or correctness of
+the 'content-type' response header. This mode will also avoid slower code paths whenever
+possible, which is particularly useful when making large requests in a browser like Safari
+that has a weaker javascript engine.
+  * 'allow-wrong-content-type': Provides partial data in more cases than 'default', but
+at the expense of causing the 'content-type' response header to be incorrectly reported
+(as 'text/plain; charset=x-user-defined') in some browsers, notably Safari and Chrome 42
+and older. Preserves binary data whenever possible. In some cases the implementation may
+also be a bit slow. This was the default in versions of this module before 1.5.
   * 'prefer-stream': Provide data before the request completes even if binary data (anything
 that isn't a single-byte ASCII or utf8 character) will be corrupted. Of course, this option
-is only safe for text data.
-  * 'prefer-fast': Use an implementation that does less processing even if it means that
-partial data isn't available. This is particularly useful when making large requests in
-a browser like Safari that has a weaker javascript engine.
+is only safe for text data. May also cause the 'content-type' response header to be
+incorrectly reported (as 'text/plain; charset=x-user-defined').
+  * 'prefer-fast': Deprecated; now a synonym for 'default', which has the same performance
+characteristics as this mode did in versions before 1.5.
 
 ### Features missing compared to node
 
diff --git a/lib/capability.js b/lib/capability.js
index 56f7c81..747bd73 100644
--- a/lib/capability.js
+++ b/lib/capability.js
@@ -17,7 +17,9 @@ function checkTypeSupport (type) {
 	return false
 }
 
-var haveArrayBuffer = isFunction(window.ArrayBuffer)
+// For some strange reason, Safari 7.0 reports typeof window.ArrayBuffer === 'object'.
+// Safari 7.1 appears to have fixed this bug.
+var haveArrayBuffer = typeof window.ArrayBuffer !== 'undefined'
 var haveSlice = haveArrayBuffer && isFunction(window.ArrayBuffer.prototype.slice)
 
 exports.arraybuffer = haveArrayBuffer && checkTypeSupport('arraybuffer')
diff --git a/lib/request.js b/lib/request.js
index 7092f37..81e6dbc 100644
--- a/lib/request.js
+++ b/lib/request.js
@@ -42,14 +42,15 @@ var ClientRequest = module.exports = function (opts) {
 
 	var preferBinary
 	if (opts.mode === 'prefer-streaming') {
-		// If streaming is a high priority but binary compatibility isn't
+		// If streaming is a high priority but binary compatibility and
+		// the accuracy of the 'content-type' header aren't
 		preferBinary = false
-	} else if (opts.mode === 'prefer-fast') {
-		// If binary is preferred for speed
-		preferBinary = true
-	} else if (!opts.mode || opts.mode === 'default') {
-		// By default, use binary if text streaming may corrupt data
+	} else if (opts.mode === 'allow-wrong-content-type') {
+		// If streaming is more important than preserving the 'content-type' header
 		preferBinary = !capability.overrideMimeType
+	} else if (!opts.mode || opts.mode === 'default' || opts.mode === 'prefer-fast') {
+		// Use binary if text streaming may corrupt data or the content-type header, or for speed
+		preferBinary = true
 	} else {
 		throw new Error('Invalid value for opts.mode')
 	}
diff --git a/test/browser/binary-streaming.js b/test/browser/binary-streaming.js
index 283ef7e..4d2d63e 100644
--- a/test/browser/binary-streaming.js
+++ b/test/browser/binary-streaming.js
@@ -26,7 +26,10 @@ for(var i = 0; i < COPIES; i++) {
 }
 
 test('binary streaming', function (t) {
-	http.get('/browserify.png?copies=' + COPIES, function (res) {
+	http.get({
+		path: '/browserify.png?copies=' + COPIES,
+		mode: 'allow-wrong-content-type'
+	}, function (res) {
 		var buffers = []
 		res.on('end', function () {
 			if (skipVerification)
@@ -50,7 +53,7 @@ test('binary streaming', function (t) {
 test('large binary request without streaming', function (t) {
 	http.get({
 		path: '/browserify.png?copies=' + COPIES,
-		mode: 'prefer-fast',
+		mode: 'default',
 	}, function (res) {
 		var buffers = []
 		res.on('end', function () {
diff --git a/test/browser/headers.js b/test/browser/headers.js
index 33b632b..02e4ae8 100644
--- a/test/browser/headers.js
+++ b/test/browser/headers.js
@@ -2,6 +2,7 @@ var Buffer = require('buffer').Buffer
 var fs = require('fs')
 var keys = require('object-keys')
 var test = require('tape')
+var UAParser = require('ua-parser-js')
 
 var http = require('../..')
 
@@ -53,4 +54,32 @@ test('headers', function (t) {
 			buffers.push(data)
 		})
 	})
+})
+
+test('content-type response header', function (t) {
+	http.get('/testHeaders', function (res) {
+		t.equal(res.headers['content-type'], 'application/json', 'content-type preserved')
+		t.end()
+	})
+})
+
+var browser = (new UAParser()).setUA(navigator.userAgent).getBrowser()
+var browserName = browser.name
+var browserVersion = browser.major
+// The content-type header is broken when 'prefer-streaming' or 'allow-wrong-content-type'
+// is passed in browsers that rely on xhr.overrideMimeType(), namely older chrome and newer safari
+var wrongMimeType = ((browserName === 'Chrome' && browserVersion <= 42) ||
+	((browserName === 'Safari' || browserName === 'Mobile Safari') && browserVersion >= 6))
+
+test('content-type response header with forced streaming', function (t) {
+	http.get({
+		path: '/testHeaders',
+		mode: 'prefer-streaming'
+	}, function (res) {
+		if (wrongMimeType)
+			t.equal(res.headers['content-type'], 'text/plain; charset=x-user-defined', 'content-type overridden')
+		else
+			t.equal(res.headers['content-type'], 'application/json', 'content-type preserved')
+		t.end()
+	})
 })
\ No newline at end of file

-- 
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-javascript/node-stream-http.git