set proper charset in content-type for res.send

This will write strings to sockets with an explicit "utf8" encoding
(which is the default) and will override the charset in the
Content-Type so it properly relfects the encoding of the response.

closes #1631
closes #2092

History.md

@@ -2,6 +2,7 @@
 ===
 
  * keep previous `Content-Type` for `res.jsonp`
+ * set proper `charset` in `Content-Type` for `res.send`
  * update connect to 2.17.1
    - fix `res.charset` appending charset when `content-type` has one
    - deps: express-session@1.2.0

lib/response.js

@@ -9,6 +9,7 @@ var http = require('http')
   , sign = require('cookie-signature').sign
   , normalizeType = require('./utils').normalizeType
   , normalizeTypes = require('./utils').normalizeTypes
+  , setCharset = require('./utils').setCharset
   , deprecate = require('./utils').deprecate
   , etag = require('./utils').etag
   , statusCodes = http.STATUS_CODES
@@ -83,6 +84,8 @@ res.links = function(links){
 res.send = function(body){
   var req = this.req;
   var head = 'HEAD' == req.method;
+  var type;
+  var encoding;
   var len;
 
   // settings
@@ -140,6 +143,17 @@ res.send = function(body){
     }
   }
 
+  // write strings in utf-8
+  if ('string' === typeof body) {
+    encoding = 'utf8';
+    type = this.get('Content-Type');
+
+    // reflect this in content-type
+    if ('string' === typeof type) {
+      this.set('Content-Type', setCharset(type, 'utf-8'));
+    }
+  }
+
   // freshness
   if (req.fresh) this.statusCode = 304;
 
@@ -152,7 +166,8 @@ res.send = function(body){
   }
 
   // respond
-  this.end(head ? null : body);
+  this.end((head ? null : body), encoding);
+
   return this;
 };
 

lib/utils.js

@@ -14,6 +14,11 @@ var mime = require('connect').mime
 
 var toString = {}.toString;
 
+/**
+ * Simple detection of charset parameter in content-type
+ */
+var charsetRegExp = /;\s*charset\s*=/;
+
 /**
  * Deprecate function, like core `util.deprecate`,
  * but with NODE_ENV and color support.
@@ -367,3 +372,34 @@ exports.compileTrust = function(val) {
 
   return proxyaddr.compile(val || []);
 }
+
+/**
+ * Set the charset in a given Content-Type string.
+ *
+ * @param {String} type
+ * @param {String} charset
+ * @return {String}
+ * @api private
+ */
+
+exports.setCharset = function(type, charset){
+  if (!type || !charset) return type;
+
+  var exists = charsetRegExp.test(type);
+
+  // removing existing charset
+  if (exists) {
+    var parts = type.split(';');
+
+    for (var i = 1; i < parts.length; i++) {
+      if (charsetRegExp.test(';' + parts[i])) {
+        parts.splice(i, 1);
+        break;
+      }
+    }
+
+    type = parts.join(';');
+  }
+
+  return type + '; charset=' + charset;
+};

test/res.attachment.js

@@ -36,7 +36,7 @@ describe('res', function(){
 
       app.use(function(req, res){
         res.attachment('/path/to/image.png');
-        res.send('foo');
+        res.send(new Buffer(4));
       });
 
       request(app)

test/res.charset.js

@@ -18,7 +18,7 @@ describe('res', function(){
       .expect("text/x-foo; charset=utf-8", done);
     })
 
-    it('should take precedence over res.send() defaults', function(done){
+    it('should be replaced by real charset in res.send', function(done){
       var app = express();
 
       app.use(function(req, res){
@@ -28,7 +28,7 @@ describe('res', function(){
 
       request(app)
       .get('/')
-      .expect('Content-Type', 'text/html; charset=whoop', done);
+      .expect('Content-Type', 'text/html; charset=utf-8', done);
     })
   })
 })

test/res.format.js

@@ -111,7 +111,7 @@ function test(app) {
     request(app)
     .get('/')
     .set('Accept', 'text/html; q=.5, text/plain')
-    .expect('Content-Type', 'text/plain; charset=UTF-8')
+    .expect('Content-Type', 'text/plain; charset=utf-8')
     .expect('hey', done);
   })
 
@@ -119,12 +119,12 @@ function test(app) {
     request(app)
     .get('/')
     .set('Accept', 'text/html')
-    .expect('Content-Type', 'text/html; charset=UTF-8');
+    .expect('Content-Type', 'text/html; charset=utf-8');
 
     request(app)
     .get('/')
     .set('Accept', 'text/plain')
-    .expect('Content-Type', 'text/plain; charset=UTF-8');
+    .expect('Content-Type', 'text/plain; charset=utf-8');
 
     request(app)
     .get('/')

test/res.json.js

@@ -17,6 +17,20 @@ describe('res', function(){
       .expect('{"foo":"bar"}', done);
     })
 
+    it('should not override previous Content-Types', function(done){
+      var app = express();
+
+      app.get('/', function(req, res){
+        res.type('application/vnd.example+json');
+        res.json({ hello: 'world' });
+      });
+
+      request(app)
+      .get('/')
+      .expect('Content-Type', 'application/vnd.example+json; charset=utf-8')
+      .expect(200, '{"hello":"world"}', done);
+    })
+
     describe('when given primitives', function(){
       it('should respond with json for null', function(done){
         var app = express();
@@ -27,11 +41,8 @@ describe('res', function(){
 
         request(app)
         .get('/')
-        .end(function(err, res){
-          res.headers.should.have.property('content-type', 'application/json; charset=utf-8');
-          res.text.should.equal('null');
-          done();
-        })
+        .expect('Content-Type', 'application/json; charset=utf-8')
+        .expect(200, 'null', done)
       })
 
       it('should respond with json for Number', function(done){
@@ -43,12 +54,8 @@ describe('res', function(){
 
         request(app)
         .get('/')
-        .end(function(err, res){
-          res.statusCode.should.equal(200);
-          res.headers.should.have.property('content-type', 'application/json; charset=utf-8');
-          res.text.should.equal('300');
-          done();
-        })
+        .expect('Content-Type', 'application/json; charset=utf-8')
+        .expect(200, '300', done)
       })
 
       it('should respond with json for String', function(done){
@@ -60,12 +67,8 @@ describe('res', function(){
 
         request(app)
         .get('/')
-        .end(function(err, res){
-          res.statusCode.should.equal(200);
-          res.headers.should.have.property('content-type', 'application/json; charset=utf-8');
-          res.text.should.equal('"str"');
-          done();
-        })
+        .expect('Content-Type', 'application/json; charset=utf-8')
+        .expect(200, '"str"', done)
       })
     })
 
@@ -79,11 +82,8 @@ describe('res', function(){
 
         request(app)
         .get('/')
-        .end(function(err, res){
-          res.headers.should.have.property('content-type', 'application/json; charset=utf-8');
-          res.text.should.equal('["foo","bar","baz"]');
-          done();
-        })
+        .expect('Content-Type', 'application/json; charset=utf-8')
+        .expect(200, '["foo","bar","baz"]', done)
       })
     })
     
@@ -97,11 +97,8 @@ describe('res', function(){
 
         request(app)
         .get('/')
-        .end(function(err, res){
-          res.headers.should.have.property('content-type', 'application/json; charset=utf-8');
-          res.text.should.equal('{"name":"tobi"}');
-          done();
-        })
+        .expect('Content-Type', 'application/json; charset=utf-8')
+        .expect(200, '{"name":"tobi"}', done)
       })
     })
 
@@ -121,10 +118,8 @@ describe('res', function(){
 
         request(app)
         .get('/')
-        .end(function(err, res){
-          res.text.should.equal('{"name":"tobi"}');
-          done();
-        });
+        .expect('Content-Type', 'application/json; charset=utf-8')
+        .expect(200, '{"name":"tobi"}', done)
       })
     })
 
@@ -152,10 +147,8 @@ describe('res', function(){
 
         request(app)
         .get('/')
-        .end(function(err, res){
-          res.text.should.equal('{\n  "name": "tobi",\n  "age": 2\n}');
-          done();
-        });
+        .expect('Content-Type', 'application/json; charset=utf-8')
+        .expect(200, '{\n  "name": "tobi",\n  "age": 2\n}', done)
       })
     })
   })
@@ -170,12 +163,8 @@ describe('res', function(){
 
       request(app)
       .get('/')
-      .end(function(err, res){
-        res.statusCode.should.equal(201);
-        res.headers.should.have.property('content-type', 'application/json; charset=utf-8');
-        res.text.should.equal('{"id":1}');
-        done();
-      })
+      .expect('Content-Type', 'application/json; charset=utf-8')
+      .expect(201, '{"id":1}', done)
     })
   })
 
@@ -189,12 +178,8 @@ describe('res', function(){
 
       request(app)
       .get('/')
-      .end(function(err, res){
-        res.statusCode.should.equal(201);
-        res.headers.should.have.property('content-type', 'application/json; charset=utf-8');
-        res.text.should.equal('{"id":1}');
-        done();
-      })
+      .expect('Content-Type', 'application/json; charset=utf-8')
+      .expect(201, '{"id":1}', done)
     })
 
     it('should use status as second number for backwards compat', function(done){
@@ -206,12 +191,8 @@ describe('res', function(){
 
       request(app)
       .get('/')
-      .end(function(err, res){
-        res.statusCode.should.equal(201);
-        res.headers.should.have.property('content-type', 'application/json; charset=utf-8');
-        res.text.should.equal('200');
-        done();
-      })
+      .expect('Content-Type', 'application/json; charset=utf-8')
+      .expect(201, '200', done)
     })
   })
 
@@ -225,11 +206,7 @@ describe('res', function(){
 
     request(app)
     .get('/')
-    .end(function(err, res){
-      res.statusCode.should.equal(200);
-      res.headers.should.have.property('content-type', 'application/vnd.example+json');
-      res.text.should.equal('{"hello":"world"}');
-      done();
-    })
+    .expect('content-type', 'application/vnd.example+json; charset=utf-8')
+    .expect(200, '{"hello":"world"}', done)
   })
 })

test/res.jsonp.js

@@ -46,11 +46,8 @@ describe('res', function(){
 
       request(app)
       .get('/?callback[a]=something')
-      .end(function(err, res){
-        res.headers.should.have.property('content-type', 'application/json; charset=utf-8');
-        res.text.should.equal('{"count":1}');
-        done();
-      })
+      .expect('Content-Type', 'application/json; charset=utf-8')
+      .expect(200, '{"count":1}', done)
     })
 
     it('should allow renaming callback', function(done){
@@ -119,6 +116,34 @@ describe('res', function(){
       });
     });
 
+    it('should not override previous Content-Types with no callback', function(done){
+      var app = express();
+
+      app.get('/', function(req, res){
+        res.type('application/vnd.example+json');
+        res.jsonp({ hello: 'world' });
+      });
+
+      request(app)
+      .get('/')
+      .expect('Content-Type', 'application/vnd.example+json; charset=utf-8')
+      .expect(200, '{"hello":"world"}', done);
+    })
+
+    it('should override previous Content-Types with callback', function(done){
+      var app = express();
+
+      app.get('/', function(req, res){
+        res.type('application/vnd.example+json');
+        res.jsonp({ hello: 'world' });
+      });
+
+      request(app)
+      .get('/?callback=cb')
+      .expect('Content-Type', 'text/javascript; charset=utf-8')
+      .expect(200, /cb\(\{"hello":"world"\}\);$/, done);
+    })
+
     describe('when given primitives', function(){
       it('should respond with json', function(done){
         var app = express();
@@ -129,11 +154,8 @@ describe('res', function(){
 
         request(app)
         .get('/')
-        .end(function(err, res){
-          res.headers.should.have.property('content-type', 'application/json; charset=utf-8');
-          res.text.should.equal('null');
-          done();
-        })
+        .expect('Content-Type', 'application/json; charset=utf-8')
+        .expect(200, 'null', done)
       })
     })
 
@@ -147,11 +169,8 @@ describe('res', function(){
 
         request(app)
         .get('/')
-        .end(function(err, res){
-          res.headers.should.have.property('content-type', 'application/json; charset=utf-8');
-          res.text.should.equal('["foo","bar","baz"]');
-          done();
-        })
+        .expect('Content-Type', 'application/json; charset=utf-8')
+        .expect(200, '["foo","bar","baz"]', done)
       })
     })
 
@@ -165,11 +184,8 @@ describe('res', function(){
 
         request(app)
         .get('/')
-        .end(function(err, res){
-          res.headers.should.have.property('content-type', 'application/json; charset=utf-8');
-          res.text.should.equal('{"name":"tobi"}');
-          done();
-        })
+        .expect('Content-Type', 'application/json; charset=utf-8')
+        .expect(200, '{"name":"tobi"}', done)
       })
     })
 
@@ -183,11 +199,8 @@ describe('res', function(){
 
         request(app)
         .get('/')
-        .end(function(err, res){
-          res.headers.should.have.property('content-type', 'application/json; charset=utf-8');
-          res.text.should.equal('null');
-          done();
-        })
+        .expect('Content-Type', 'application/json; charset=utf-8')
+        .expect(200, 'null', done)
       })
 
       it('should respond with json for Number', function(done){
@@ -199,12 +212,8 @@ describe('res', function(){
 
         request(app)
         .get('/')
-        .end(function(err, res){
-          res.statusCode.should.equal(200);
-          res.headers.should.have.property('content-type', 'application/json; charset=utf-8');
-          res.text.should.equal('300');
-          done();
-        })
+        .expect('Content-Type', 'application/json; charset=utf-8')
+        .expect(200, '300', done)
       })
 
       it('should respond with json for String', function(done){
@@ -216,12 +225,8 @@ describe('res', function(){
 
         request(app)
         .get('/')
-        .end(function(err, res){
-          res.statusCode.should.equal(200);
-          res.headers.should.have.property('content-type', 'application/json; charset=utf-8');
-          res.text.should.equal('"str"');
-          done();
-        })
+        .expect('Content-Type', 'application/json; charset=utf-8')
+        .expect(200, '"str"', done)
       })
     })
 
@@ -241,10 +246,8 @@ describe('res', function(){
 
         request(app)
         .get('/')
-        .end(function(err, res){
-          res.text.should.equal('{"name":"tobi"}');
-          done();
-        });
+        .expect('Content-Type', 'application/json; charset=utf-8')
+        .expect(200, '{"name":"tobi"}', done)
       })
     })
 
@@ -272,10 +275,8 @@ describe('res', function(){
 
         request(app)
         .get('/')
-        .end(function(err, res){
-          res.text.should.equal('{\n  "name": "tobi",\n  "age": 2\n}');
-          done();
-        });
+        .expect('Content-Type', 'application/json; charset=utf-8')
+        .expect(200, '{\n  "name": "tobi",\n  "age": 2\n}', done)
       })
     })
   })
@@ -290,12 +291,8 @@ describe('res', function(){
 
       request(app)
       .get('/')
-      .end(function(err, res){
-        res.statusCode.should.equal(201);
-        res.headers.should.have.property('content-type', 'application/json; charset=utf-8');
-        res.text.should.equal('{"id":1}');
-        done();
-      })
+      .expect('Content-Type', 'application/json; charset=utf-8')
+      .expect(201, '{"id":1}', done)
     })
   })
 
@@ -309,12 +306,8 @@ describe('res', function(){
 
       request(app)
       .get('/')
-      .end(function(err, res){
-        res.statusCode.should.equal(201);
-        res.headers.should.have.property('content-type', 'application/json; charset=utf-8');
-        res.text.should.equal('{"id":1}');
-        done();
-      })
+      .expect('Content-Type', 'application/json; charset=utf-8')
+      .expect(201, '{"id":1}', done)
     })
 
     it('should use status as second number for backwards compat', function(done){
@@ -326,12 +319,8 @@ describe('res', function(){
 
       request(app)
       .get('/')
-      .end(function(err, res){
-        res.statusCode.should.equal(201);
-        res.headers.should.have.property('content-type', 'application/json; charset=utf-8');
-        res.text.should.equal('200');
-        done();
-      })
+      .expect('Content-Type', 'application/json; charset=utf-8')
+      .expect(201, '200', done)
     })
   })
 
@@ -345,11 +334,7 @@ describe('res', function(){
 
     request(app)
     .get('/')
-    .end(function(err, res){
-      res.statusCode.should.equal(200);
-      res.headers.should.have.property('content-type', 'application/vnd.example+json');
-      res.text.should.equal('{"hello":"world"}');
-      done();
-    })
+    .expect('content-type', 'application/vnd.example+json; charset=utf-8')
+    .expect(200, '{"hello":"world"}', done)
   })
 })

test/res.send.js

@@ -135,9 +135,34 @@ describe('res', function(){
 
       request(app)
       .get('/')
-      .expect('Content-Type', 'text/plain')
-      .expect('hey')
-      .expect(200, done);
+      .expect('Content-Type', 'text/plain; charset=utf-8')
+      .expect(200, 'hey', done);
+    })
+
+    it('should override charset in Content-Type', function(done){
+      var app = express();
+
+      app.use(function(req, res){
+        res.set('Content-Type', 'text/plain; charset=iso-8859-1').send('hey');
+      });
+
+      request(app)
+      .get('/')
+      .expect('Content-Type', 'text/plain; charset=utf-8')
+      .expect(200, 'hey', done);
+    })
+
+    it('should keep charset in Content-Type for Buffers', function(done){
+      var app = express();
+
+      app.use(function(req, res){
+        res.set('Content-Type', 'text/plain; charset=iso-8859-1').send(new Buffer('hi'));
+      });
+
+      request(app)
+      .get('/')
+      .expect('Content-Type', 'text/plain; charset=iso-8859-1')
+      .expect(200, 'hi', done);
     })
   })
 
@@ -201,11 +226,8 @@ describe('res', function(){
 
       request(app)
       .get('/')
-      .end(function(err, res){
-        res.headers.should.have.property('content-type', 'application/json; charset=utf-8');
-        res.text.should.equal('{"name":"tobi"}');
-        done();
-      })
+      .expect('Content-Type', 'application/json; charset=utf-8')
+      .expect(200, '{"name":"tobi"}', done)
     })
   })