The Wayback Machine - https://web.archive.org/web/20201019195754/https://github.com/restify/node-restify/issues/1589
Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Additional content-type parameters are thrown away #1589

Open
ThomWright opened this issue Jan 2, 2018 · 1 comment
Open

Additional content-type parameters are thrown away #1589

ThomWright opened this issue Jan 2, 2018 · 1 comment

Comments

@ThomWright
Copy link

@ThomWright ThomWright commented Jan 2, 2018

  • Used appropriate template for the issue type
  • Searched both open and closed issues for duplicates of this issue
  • Title adequately and concisely reflects the feature or the bug

Bug Report

Restify Version

6.3.4

Node.js Version

8.9.1

Expected behaviour

Given the following code, I would expect the content-type string I set to be used.

res.contentType = "text/plain; version=0.0.4; charset=utf-8"
res.send(...)

Should produce:

Content-Type: text/plain; version=0.0.4; charset=utf-8

For context, the content-type I'm using comes from: Prometheus client, Prometheus docs

Actual behaviour

Only the contents of the string before the first ; are used, resulting in:

Content-Type: text/plain

Repro case

var restify = require('restify');

const server = restify.createServer({
  name: 'myapp',
  version: '1.0.0'
});

server.get('/test', function (req, res, next) {
  res.contentType = "text/plain; version=0.0.4; charset=utf-8"
  res.send(200, "some text")
  return next();
});

server.listen(8080, function () {
  console.log('%s listening at %s', server.name, server.url);
});

Cause

The string is split here, throwing away the rest:

type = type.split(';')[0];

Are you willing and able to fix this?

Maybe 😄

Assuming there isn't some reason this is intended behaviour, I can have a look at fixing.

@retrohacker
Copy link
Member

@retrohacker retrohacker commented Feb 8, 2018

Hey @ThomWright!
Thank you for opening such a thorough bug report ❤️

I believe you are correct. If we are provided a content type by the user, we should use it. We currently support specifying a charSet explicitly which is added on line 493 but we don't have an option for configuring a version.

According to the RFC, everything that comes after the content-type is the media-type, and we should respect the user's request for a specific media-type.

The question I would have is: what effect does specifying a media-type have on the formatter for that content-type? Does the formatter need to be aware of the media-types being asked for?

I think the easiest thing to do here, for the short term, is to just append the media-type back to the type after resolving the formatter. Thoughts?

@hekike hekike added the Critical label Apr 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.