Merge pull request #48 from bitinn/bugfix

Prepare for v1.3.3 release

.travis.yml

@@ -3,5 +3,6 @@ node_js:
   - "0.10"
   - "0.12"
   - "iojs"
+  - "node"
 before_install: npm install -g npm
 script: npm run coverage

CHANGELOG.md

@@ -5,7 +5,14 @@ Changelog
 
 # 1.x release
 
-## v1.3.2 (master)
+## v1.3.3 (master)
+
+- Fix: make sure `Content-Length` header is set when body is string for POST/PUT/PATCH requests
+- Fix: handle body stream error, for cases such as incorrect `Content-Encoding` header
+- Fix: when following certain redirects, use `GET` on subsequent request per Fetch Spec
+- Fix: `Request` and `Response` constructors now parse headers input using `Headers`
+
+## v1.3.2
 
 - Enhance: allow auto detect of form-data input (no `FormData` spec on node.js, this is form-data specific feature)
 

index.js

@@ -82,6 +82,16 @@ function Fetch(url, opts) {
 			headers.set('content-type', 'multipart/form-data; boundary=' + options.body.getBoundary());
 		}
 
+		// bring node-fetch closer to browser behavior by setting content-length automatically for POST, PUT, PATCH requests when body is empty or string
+		if (!headers.has('content-length') && options.method.substr(0, 1).toUpperCase() === 'P') {
+			if (typeof options.body === 'string') {
+				headers.set('content-length', Buffer.byteLength(options.body));
+			// this is only necessary for older nodejs releases (before iojs merge)
+			} else if (options.body === undefined || options.body === null) {
+				headers.set('content-length', '0');
+			}
+		}
+
 		options.headers = headers.raw();
 
 		// http.request only support string as host header, this hack make custom host header possible
@@ -122,6 +132,15 @@ function Fetch(url, opts) {
 					return;
 				}
 
+				// per fetch spec, for POST request with 301/302 response, or any request with 303 response, use GET when following redirect
+				if (res.statusCode === 303
+					|| ((res.statusCode === 301 || res.statusCode === 302) && options.method === 'POST'))
+				{
+					options.method = 'GET';
+					delete options.body;
+					delete options.headers['content-length'];
+				}
+
 				options.counter++;
 
 				resolve(Fetch(resolve_url(options.url, res.headers.location), options));

lib/response.js

@@ -87,6 +87,11 @@ Response.prototype._decode = function() {
 			}, self.timeout);
 		}
 
+		// handle stream error, such as incorrect content-encoding
+		self.body.on('error', function(err) {
+			reject(new Error('invalid response body at: ' + self.url + ' reason: ' + err.message));
+		});
+
 		self.body.on('data', function(chunk) {
 			if (self._abort || chunk === null) {
 				return;

test/server.js

@@ -80,6 +80,13 @@ TestServer.prototype.router = function(req, res) {
 		res.end('fake sdch string');
 	}
 
+	if (p === '/invalid-content-encoding') {
+		res.statusCode = 200;
+		res.setHeader('Content-Type', 'text/plain');
+		res.setHeader('Content-Encoding', 'gzip');
+		res.end('fake gzip string');
+	}
+
 	if (p === '/timeout') {
 		setTimeout(function() {
 			res.statusCode = 200;

test/test.js

@@ -225,6 +225,54 @@ describe('node-fetch', function() {
 		});
 	});
 
+	it('should follow POST request redirect code 301 with GET', function() {
+		url = base + '/redirect/301';
+		opts = {
+			method: 'POST'
+			, body: 'a=1'
+		};
+		return fetch(url, opts).then(function(res) {
+			expect(res.url).to.equal(base + '/inspect');
+			expect(res.status).to.equal(200);
+			return res.json().then(function(result) {
+				expect(result.method).to.equal('GET');
+				expect(result.body).to.equal('');
+			});
+		});
+	});
+
+	it('should follow POST request redirect code 302 with GET', function() {
+		url = base + '/redirect/302';
+		opts = {
+			method: 'POST'
+			, body: 'a=1'
+		};
+		return fetch(url, opts).then(function(res) {
+			expect(res.url).to.equal(base + '/inspect');
+			expect(res.status).to.equal(200);
+			return res.json().then(function(result) {
+				expect(result.method).to.equal('GET');
+				expect(result.body).to.equal('');
+			});
+		});
+	});
+
+	it('should follow redirect code 303 with GET', function() {
+		url = base + '/redirect/303';
+		opts = {
+			method: 'PUT'
+			, body: 'a=1'
+		};
+		return fetch(url, opts).then(function(res) {
+			expect(res.url).to.equal(base + '/inspect');
+			expect(res.status).to.equal(200);
+			return res.json().then(function(result) {
+				expect(result.method).to.equal('GET');
+				expect(result.body).to.equal('');
+			});
+		});
+	});
+
 	it('should obey maximum redirect', function() {
 		url = base + '/redirect/chain';
 		opts = {
@@ -348,6 +396,14 @@ describe('node-fetch', function() {
 		});
 	});
 
+	it('should reject if response compression is invalid', function() {
+		url = base + '/invalid-content-encoding';
+		return fetch(url).then(function(res) {
+			expect(res.headers.get('content-type')).to.equal('text/plain');
+			return expect(res.text()).to.eventually.be.rejectedWith(Error);
+		});
+	});
+
 	it('should allow disabling auto decompression', function() {
 		url = base + '/gzip';
 		opts = {
@@ -416,6 +472,8 @@ describe('node-fetch', function() {
 			return res.json();
 		}).then(function(res) {
 			expect(res.method).to.equal('POST');
+			expect(res.headers['transfer-encoding']).to.be.undefined;
+			expect(res.headers['content-length']).to.equal('0');
 		});
 	});
 
@@ -430,6 +488,8 @@ describe('node-fetch', function() {
 		}).then(function(res) {
 			expect(res.method).to.equal('POST');
 			expect(res.body).to.equal('a=1');
+			expect(res.headers['transfer-encoding']).to.be.undefined;
+			expect(res.headers['content-length']).to.equal('3');
 		});
 	});
 
@@ -444,6 +504,8 @@ describe('node-fetch', function() {
 		}).then(function(res) {
 			expect(res.method).to.equal('POST');
 			expect(res.body).to.equal('a=1');
+			expect(res.headers['transfer-encoding']).to.equal('chunked');
+			expect(res.headers['content-length']).to.be.undefined;
 		});
 	});
 
@@ -796,6 +858,40 @@ describe('node-fetch', function() {
 		});
 	});
 
+	it('should support empty options in Response constructor', function() {
+		var body = resumer().queue('a=1').end();
+		body = body.pipe(new stream.PassThrough());
+		var res = new Response(body);
+		return res.text().then(function(result) {
+			expect(result).to.equal('a=1');
+		});
+	});
+
+	it('should support parsing headers in Response constructor', function() {
+		var body = resumer().queue('a=1').end();
+		body = body.pipe(new stream.PassThrough());
+		var res = new Response(body, {
+			headers: {
+				a: '1'
+			}
+		});
+		expect(res.headers.get('a')).to.equal('1');
+		return res.text().then(function(result) {
+			expect(result).to.equal('a=1');
+		});
+	});
+
+	it('should support parsing headers in Request constructor', function() {
+		url = base;
+		var req = new Request(url, {
+			headers: {
+				a: '1'
+			}
+		});
+		expect(req.url).to.equal(url);
+		expect(req.headers.get('a')).to.equal('1');
+	});
+
 	it('should support https request', function() {
 		this.timeout(5000);
 		url = 'https://github.com/';
@@ -808,18 +904,4 @@ describe('node-fetch', function() {
 		});
 	});
 
-	it('should support parsing headers in Response constructor', function(){
-
-		var r = new Response(null, {headers: {'foo': 'bar'}});
-		expect(r.headers.get('foo')).to.equal('bar');
-
-	});
-
-	it('should support parsing headers in Request constructor', function(){
-
-		var r = new Request('http://foo', {headers: {'foo': 'bar'}});
-		expect(r.headers.get('foo')).to.equal('bar');
-
-	});
-
 });