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

Confirm service does not accept absent body #45

Open
ahggns opened this issue Sep 5, 2018 · 4 comments
Open

Confirm service does not accept absent body #45

ahggns opened this issue Sep 5, 2018 · 4 comments

Comments

@ahggns
Copy link
Contributor

ahggns commented Sep 5, 2018

The confirm endpoints for the optional test should accept a plain null body, but

$ curl -s http://localhost:8000/confirm/receiveRawOptionalExample/0 -XPOST -d 'null'                                                                                                             
{"errorCode":"INVALID_ARGUMENT","errorName":"Conjureverification:UnsupportedContentType","errorInstanceId":"5bac689e-96de-45a2-bb3d-6f4745903347","parameters":{}}%
@qinfchen
Copy link
Contributor

qinfchen commented Sep 5, 2018

@ahggns, have you tried adding content type?

-H 'content-type: application/json' -d 'null'

@ahggns
Copy link
Contributor Author

ahggns commented Sep 5, 2018

Ah yeah that does it, I was hoping that was what was wrong with palantir/conjure-python#16 :/

@ahggns ahggns closed this as completed Sep 5, 2018
@ahggns
Copy link
Contributor Author

ahggns commented Sep 6, 2018

@qinfchen it seems that python's requests library doesn't send a body in the POST when the json argument is None which what is happening here. That does seem to trip up the verification server, but not sure whether or not it should:

$ curl -s http://localhost:8000/confirm/receiveRawOptionalExample/0 -XPOST -H 'content-type: application/json'                                                                       
{"errorCode":"INVALID_ARGUMENT","errorName":"Conjureverification:InvalidRequestBody","errorInstanceId":"ec3a4ec0-4bbd-42da-8058-14357fb1014a","parameters":{}}%                                                

$ curl -s http://localhost:8000/confirm/receiveRawOptionalExample/0 -XPOST                                                                                                            
{"errorCode":"INVALID_ARGUMENT","errorName":"Conjureverification:UnsupportedContentType","errorInstanceId":"a0148636-7de7-41a6-8a90-1127a7510cec","parameters":{}}%

@ahggns ahggns reopened this Sep 6, 2018
@ahggns ahggns changed the title Confirm service does not accept null body Confirm service does not accept absent body Sep 6, 2018
@dansanduleac
Copy link
Contributor

@ahggns According to the wire spec:

Content-Type header - For Conjure endpoints that define a body argument, a Content-Type header MUST be added. If the body is of type binary, the content-type application/octet-stream MUST be used. Otherwise, clients MUST send Content-Type: application/json.

I think it's reasonable to force the client to send application/json here if what they're sending is null, a valid JSON token.

On the other hand, I'm concerned that the the spec also accepts (even recommends) sending an empty body, which doesn't seem to be valid JSON:

[when] the de-aliased argument is type optional<T> and the value is not present: it is RECOMMENDED to send an empty request body, although clients MAY alternatively send the JSON value null

Specifically, it sounds incorrect to me that we MUST send application/json when sending the empty body. I'll create a separate issue for that.

I agree that the verification server doesn't currently take this into account, and it should, but we should flesh out the spec to be consistent with what's valid JSON and what's not, before doing that.

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

No branches or pull requests

3 participants