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

Omit quotes around body #32

Closed
dandv opened this issue Apr 19, 2015 · 5 comments
Closed

Omit quotes around body #32

dandv opened this issue Apr 19, 2015 · 5 comments
Labels

Comments

@dandv
Copy link
Contributor

dandv commented Apr 19, 2015

I want to return a raw string as the response data but restivus wraps the string in double quotes.

return 'foo"bar'

Will produce

"foo\"bar"

I've tried setting the Content-type to text/plain or text/html but I get the same type of output.

What I ultimately want to do is return a JPEG image from a custom collection/:id/photo route that extracts the base64-encoded image from a document.

@kahmali
Copy link
Owner

kahmali commented Apr 19, 2015

I'm looking into this. I'll keep you posted. If you have any suggestions just let me know.

@kahmali
Copy link
Owner

kahmali commented Apr 19, 2015

I'm pretty sure I know what's causing this. I was naively JSON.stringifying the response, regardless of the Content-Type. I just need to find a way to consistently pass a String or Buffer to response.write() in my Route._respond() method, while maintaining the proper content type. I've included the offending code below:

  ###
    Respond to an HTTP request
  ###
  _respond: (endpointContext, body, statusCode=200, headers) ->
    # Allow cross-domain requests to be made from the browser
    endpointContext.response.setHeader 'Access-Control-Allow-Origin', '*'

    # Ensure that a content type is set (will be overridden if also included in given headers)
    endpointContext.response.setHeader 'Content-Type', 'text/json'

    ##################################################################
    # BUG: Only JSON Content-Type responses should be JSON.stringified
    ##################################################################
    # Prettify JSON if configured in API
    if @api.config.prettyJson
      bodyAsJson = JSON.stringify body, undefined, 2
    else
      bodyAsJson = JSON.stringify body

    # Send response
    endpointContext.response.writeHead statusCode, headers
    endpointContext.response.write bodyAsJson
    endpointContext.response.end()

kahmali added a commit that referenced this issue Apr 23, 2015
Fix Issue #32:
- Prevent non-JSON response types from being wrapped in quotes
  - This bug required clients to do additional parsing of non-JSON types
    (removing quotes and escaped characters)
- Add test for checking non-JSON response Content-Type
- Update change log
@kahmali
Copy link
Owner

kahmali commented Apr 23, 2015

The commit referenced below should fix this issue. I added a basic Tinytest and it passed that test along with the rest of the suite, so everything looks good to go from my end. Would you like to test it out on your use case before I bump the version and publish this?

EDIT: Just realized that I could make that test description more generic, since Restivus shouldn't wrap any non-JSON Content-Types in quotes, not just text/plain. I'll need to expand the test coverage anyway, so I'll just fix that when I do.

kahmali added a commit that referenced this issue Apr 26, 2015
Fix Issue #32:
- Prevent non-JSON response types from being wrapped in quotes
  - This bug required clients to do additional parsing of non-JSON types
    (removing quotes and escaped characters)
- Add test for checking non-JSON response Content-Type
- Update change log
kahmali referenced this issue in ethanwillis/meteor-restivus Apr 26, 2015
@kahmali
Copy link
Owner

kahmali commented Apr 27, 2015

I went ahead and merged this fix into master since it continued to pass all my tests. It has been published in v0.6.5. Please let me know if it doesn't behave as expected now.

@kahmali kahmali closed this as completed Apr 27, 2015
@dandv
Copy link
Contributor Author

dandv commented Apr 28, 2015

Thanks @kahmali. Tests pass here as well. I've used "text/plain" and "text/html".

PS: congrats on getting an avatar, it's nice to put a face to the name ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants