oasis-tcs / cti-taxii2

OASIS CTI TC: An official CTI TC repository for TAXII 2 work
https://github.com/oasis-tcs/cti-taxii2
Other
9 stars 4 forks source link

Allow HTTP status code 200 (OK) for POST requests #71

Closed jordan2175 closed 6 years ago

jordan2175 commented 6 years ago

Currently the spec only allows HTTP 202 (Accepted) responses to an HTTP POST. This was done to enable asynchronous responses. However, there are times when a sever can and will process the request synchronously. The spec should allow the server to respond with an HTTP 200 (OK) to single to the client that the request is done.

The server, depending on how it is designed, may implement asynchronous processing, synchronous processing, or both. The client would need to support both, however, the asynchronous is the longer pole in the tent and the synchronous processing in the client is just a short circuit.

jordan2175 commented 6 years ago

We discussed this on the working call on 2018-05-15. While there was no consensus out of the call, there seems to be a view by some that servers could just respond with a 202 (Accepted) with a status resource that showed all objects were added and nothing was left pending.

I think this might be okay, but that would mean that a client would have to actually process the entire status resource to verify that. Where as if the server could return just a 200 (OK), then the client could just accept that and move on. Further, if the server could just respond with a 200 (OK) then it would not need to send the status resource at all.

varnerac commented 6 years ago

I just want to note that an HTTP Status Code 200 OK is not idiomatic for creating an object on POST. A 201 Created is the standard way to do this. However, we can't use 201 because we don't have a single URL to return that points to the versions created.

If we are going to work to return some other status code, I think we should shoot for Created instead of OK.

gtback commented 6 years ago

@jordan2175: If a server returned a 200 OK (or 201, as @varnerac suggests), would it still be allowed to (or required to, but probably not) return the status resource in the body of the response? That could be important if #65 is accepted.

johnwunder commented 6 years ago

As we discussed on the call, this change doesn't seem to buy us anything. The existing 202 status code with a status resource in the response supports the ability of a server to support both synchronous and asynchronous processing and is no extra work for the client. Since everything can be supported as-is with relatively clean semantics, I don't think we should make any changes.

jordan2175 commented 6 years ago

@gtback I think with a 200(OK) you could, but would not be required to send a status resource since the 200(OK) tells you everything you need to know. @johnwunder I think it is about being clean with how we design our REST API. Overloading things, does not really help long-term. I think this would be a very very simple thing to allow. Now if we wanted to also do a 201(Created) then, I think that will be a bit more work and would lend itself to sending content to a TAXII server outside of a STIX bundle.

gtback commented 6 years ago

tells you everything you need to know

True, unless the server is passing back additional information about successfully processed objects (which is part of #65).

Still, I agree sending a full status object should probably not be required in the case of 200 OK.

jordan2175 commented 6 years ago

We talked about this on the working call on 2018-05-22. I expressed that this was really a performance enhancement to prevent the server from having to send back a very large status resource. Allan Thomson proposed an alternative proposal, that is, make the details about the success in the status resource optional. That way, the server does not have to send all the data back just for the client to see that it was a success. Drew brought up that a client could simple use the counter to check. We decided to table this and come back to in a few months.

gtback commented 6 years ago

I don't see anything in the spec that requires the server to send back a very large status resource. All of the "list​ of typestatus-details" properties in a status resource (successes, failures, and pendings) are optional, and everything else is relatively fixed sized (beyond enormously large integers).

make the details about the success in the status resource optional

I think they already are? Only success_count is required.

That said, I'm pretty ambivalent to this issue. I don't think it's necessary, but won't fight against it.

jordan2175 commented 6 years ago

This does not seem to have consensus to do, so I am closing this issue.