BREAKING CHANGE: do not override content-type in res.send

Author: Julien Gilli

docs/guides/server.md

@@ -427,10 +427,18 @@ server.get('/websocket/attach', function upgradeRoute(req, res, next) {
 
 ## Content Negotiation
 
-If you're using `res.send()` restify will automatically select the content-type
-to respond with, by finding the first registered `formatter` defined.  Note in
-the examples above we've not defined any formatters, so we've been leveraging
-the fact that restify ships with `application/json`, `text/plain` and
+If you're using `res.send()` restify will determine the content-type to respond
+with by:
+
+1. negotiating the content-type by matching available formatters with the
+   request's `accept` header
+1. otherwise, using `application/json` if the body is an object that is not a
+   Buffer instance
+1. otherwise, using the value of `res.contentType` if present
+1. otherwise, using the value of the `Content-Type` response header if set
+
+Note in the examples above we've not defined any formatters, so we've been
+leveraging the fact that restify ships with `application/json`, `text/plain` and
 `application/octet-stream` formatters. You can add additional formatters to
 restify by passing in a hash of content-type -> parser at server creation time:
 
@@ -450,9 +458,14 @@ var server = restify.createServer({
 });
 ```
 
-If a content-type can't be negotiated, then restify will default to using the
-`application/octet-stream` formatter. For example, attempting to send a
-content-type that does not have a defined formatter:
+If a content-type can't be determined, then restify will respond with an error.
+
+If a content-type can be negotiated, but that content-type doesn't have a
+corresponding formatter, restify will not format the response with any
+formatter.
+
+For example, attempting to send a content-type that does not have a defined
+formatter:
 
 ```js
 server.get('/foo', function(req, res, next) {
@@ -462,12 +475,13 @@ server.get('/foo', function(req, res, next) {
 });
 ```
 
-Will result in a response with a content-type of `application/octet-stream`:
+Will result in a response with a content-type of `text/css` even though no
+`'text/css'` formatter is present:
 
 ```sh
 $ curl -i localhost:3000/
 HTTP/1.1 200 OK
-Content-Type: application/octet-stream
+Content-Type: text/css
 Content-Length: 2
 Date: Thu, 02 Jun 2016 06:50:54 GMT
 Connection: keep-alive

docs/index.md

@@ -437,10 +437,18 @@ server.get('/websocket/attach', function upgradeRoute(req, res, next) {
 
 ## Content Negotiation
 
-If you're using `res.send()` restify will automatically select the content-type
-to respond with, by finding the first registered `formatter` defined.  Note in
-the examples above we've not defined any formatters, so we've been leveraging
-the fact that restify ships with `application/json`, `text/plain` and
+If you're using `res.send()` restify will determine the content-type to respond
+with by:
+
+1. negotiating the content-type by matching available formatters with the
+   request's `accept` header
+1. otherwise, using `application/json` if the body is an object that is not a
+   Buffer instance
+1. otherwise, using the value of `res.contentType` if present
+1. otherwise, using the value of the `Content-Type` response header if set
+
+Note in the examples above we've not defined any formatters, so we've been
+leveraging the fact that restify ships with `application/json`, `text/plain` and
 `application/octet-stream` formatters. You can add additional formatters to
 restify by passing in a hash of content-type -> parser at server creation time:
 
@@ -460,9 +468,14 @@ var server = restify.createServer({
 });
 ```
 
-If a content-type can't be negotiated, then restify will default to using the
-`application/octet-stream` formatter. For example, attempting to send a
-content-type that does not have a defined formatter:
+If a content-type can't be determined, then restify will respond with an error.
+
+If a content-type can be negotiated, but that content-type doesn't have a
+corresponding formatter, restify will not format the response with any
+formatter.
+
+For example, attempting to send a content-type that does not have a defined
+formatter:
 
 ```js
 server.get('/foo', function(req, res, next) {
@@ -472,12 +485,13 @@ server.get('/foo', function(req, res, next) {
 });
 ```
 
-Will result in a response with a content-type of `application/octet-stream`:
+Will result in a response with a content-type of `text/css` even though no
+`'text/css'` formatter is present:
 
 ```sh
 $ curl -i localhost:3000/
 HTTP/1.1 200 OK
-Content-Type: application/octet-stream
+Content-Type: text/css
 Content-Length: 2
 Date: Thu, 02 Jun 2016 06:50:54 GMT
 Connection: keep-alive

lib/response.js

@@ -448,12 +448,14 @@ function patch(Response) {
         // Derive type if not provided by the user
         type = type || self.req.accepts(self.acceptable);
 
-        // Check to see if we can find a valid formatter
+        // Check to see if we could find a content type to use for the response.
         if (!type) {
             return formatterError(
                 self,
                 new errors.NotAcceptableError({
-                    message: 'could not find suitable formatter'
+                    message:
+                        'could not find suitable content-type to use ' +
+                        'for the response'
                 })
             );
         }
@@ -464,33 +466,20 @@ function patch(Response) {
             type = mime.lookup(type);
         }
 
-        // If we were unable to derive a valid type, default to treating it as
-        // arbitrary binary data per RFC 2046 Section 4.5.1
-        if (!self.formatters[type] && self.acceptable.indexOf(type) === -1) {
-            type = 'application/octet-stream';
-        }
-
         formatter = self.formatters[type] || self.formatters['*/*'];
 
-        // If after the above attempts we were still unable to derive a
-        // formatter, provide a meaningful error message
-        if (!formatter) {
-            return formatterError(
-                self,
-                new errors.InternalServerError({
-                    message:
-                        'could not find formatter for application/octet-stream'
-                })
-            );
-        }
-
         if (self._charSet) {
             type = type + '; charset=' + self._charSet;
         }
 
-        // Update header to the derived content type for our formatter
+        // Update Content-Type header to the one originally set or to the type
+        // inferred from the most relevant formatter found.
         self.setHeader('Content-Type', type);
 
+        if (!formatter) {
+            return flush(self, body);
+        }
+
         // Finally, invoke the formatter and flush the request with it's results
         return flush(self, formatter(self.req, self, body));
     };

test/formatter.test.js

@@ -189,8 +189,8 @@ test(
 );
 
 test(
-    'GH-937 should return 500 when no default formatter found ' +
-        'and octet-stream is not available',
+    'GH-937 should return 200 even when no formatter matching client found ' +
+        'but content-type set manually',
     function(t) {
         // ensure client accepts only a type not specified by server
         var opts = {
@@ -201,10 +201,11 @@ test(
         };
 
         CLIENT.get(opts, function(err, req, res, data) {
-            t.ok(err);
+            t.ifError(err);
             t.ok(req);
             t.ok(res);
-            t.equal(res.statusCode, 500);
+            t.equal(res.statusCode, 200);
+            t.equal(res.contentType(), 'text/html');
             t.end();
         });
     }

test/response.test.js

@@ -497,10 +497,9 @@ test('writeHead should emit a header event', function(t) {
     });
 });
 
-test('should fail to set header due to missing formatter', function(t) {
-    // when a formatter is not set up for a specific content-type, restify will
-    // default to octet-stream.
-
+test('should send 200 when formatter missing', function(t) {
+    // res.send still sends a response even when a formatter is not set up for a
+    // specific content-type.
     SERVER.get('/11', function handle(req, res, next) {
         res.header('content-type', 'application/hal+json');
         res.send(200, JSON.stringify({ hello: 'world' }));
@@ -510,7 +509,7 @@ test('should fail to set header due to missing formatter', function(t) {
     CLIENT.get(join(LOCALHOST, '/11'), function(err, _, res) {
         t.ifError(err);
         t.equal(res.statusCode, 200);
-        t.equal(res.headers['content-type'], 'application/octet-stream');
+        t.equal(res.headers['content-type'], 'application/hal+json');
         t.end();
     });
 });