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

READY FOR REVIEW - Fix #169. Set correct Content-Type for JSON-RPC responses. #170

Merged
merged 1 commit into from
Jan 7, 2016
Merged

READY FOR REVIEW - Fix #169. Set correct Content-Type for JSON-RPC responses. #170

merged 1 commit into from
Jan 7, 2016

Conversation

posita
Copy link

@posita posita commented Sep 15, 2015

Fix #169. Set correct Content-Type for JSON-RPC responses.

I don't see an existing test harness for JsonrpcRequestHandler or RequestHandler, otherwise I would have included unit tests. However, it does appear to work when I test it by hand (both with normal and error responses):

% curl --verbose -H 'Content-Type: application/json' -H 'Accept: application/json' -X POST --data [JSONRPC_DATA] [JSONRPC_URL]
...
< Content-Type: application/json; charset=UTF-8
...

@posita
Copy link
Author

posita commented Sep 15, 2015

UPDATE: Failed tests are unrelated to this PR. See this comment and #164. Original comment preserved below.

Hmmm...I haven't modified any of the tests, and I don't think the failing one (test_set_cookie_expires_days) is implicated by my code change. Were these already failing? 😕

@posita posita mentioned this pull request Sep 15, 2015
vltr pushed a commit to vltr/cyclone that referenced this pull request Oct 16, 2015
@dpnova
Copy link
Collaborator

dpnova commented Jan 6, 2016

👍 from me on this one @fiorix - thanks @posita

fiorix added a commit that referenced this pull request Jan 7, 2016
…-type

READY FOR REVIEW - Fix #169. Set correct Content-Type for JSON-RPC responses.
@fiorix fiorix merged commit 6ab75d8 into fiorix:master Jan 7, 2016
@posita posita deleted the posita/169-jsonrpchandler-content-type branch January 7, 2016 21:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants