Skip to content

Commit

Permalink
Fixed Naive Parsing of Content-Type HTTP header
Browse files Browse the repository at this point in the history
It turns out checking for exactly "application/json" is a bad idea since
some servers/web frameworks include stuff like encoding in the header.
For example: Express.js since 4.16.x now set's the content-type header
to: 'application/json; charset=utf-8'.

To remedy this, we now check for the inclusion of application/json in
the string, but don't attempt to match the exact value
  • Loading branch information
danielstocks committed Oct 28, 2017
1 parent 6de9010 commit 6b0646e
Show file tree
Hide file tree
Showing 3 changed files with 4 additions and 3 deletions.
3 changes: 2 additions & 1 deletion lib/request.js
Expand Up @@ -15,7 +15,8 @@ module.exports = function(method, endpoint, cb, headers) {
}),
}).then(res => {
status = res.status
if(res.headers.get('content-type') == 'application/json') {
var contentType = res.headers.get("content-type");
if(contentType && contentType.includes("application/json")) {
return res.json();
} else {
return res.text();
Expand Down
2 changes: 1 addition & 1 deletion package.json
@@ -1,6 +1,6 @@
{
"name": "yak-orm",
"version": "0.0.17",
"version": "0.0.18",
"description": "Yak is an ORM that maps REST resources to JavaScript models/collections",
"main": "index.js",
"scripts": {
Expand Down
2 changes: 1 addition & 1 deletion test/request_test.js
Expand Up @@ -23,7 +23,7 @@ describe('Request', function() {
status: 200,
headers: {
get: function() {
return 'application/json';
return 'application/json; charset=utf-8';
}
},
json: function() {
Expand Down

0 comments on commit 6b0646e

Please sign in to comment.