Allow HTTP 200 response for HTTP file upload PUT request

[lorenz]

Is your feature request related to a problem? Please describe.
Allow Siskin to use HTTP file upload with backend services returning HTTP 200 for new files. For example all object stores based on the widely-used Amazon S3 API do this.

Describe the solution you'd like
Accept HTTP 200 as a valid response for the HTTP PUT request.

Something like this should take care of it as all non 200/201 status codes are already filtered out.

- if code == 200 {
-   completionHandler(.failure(.invalidResponseCode(url: slot.getUri)));
- } else {
  completionHandler(.success(slot.getUri));
- }

Describe alternatives you've considered
Otherwise it's not possible to directly use object stores and all PUTs need to be forcibly redirected through a proxy which is annoying.

Additional context
Most other clients (Conversations, Dino, ...) are fine with this, just Siskin doesn't like it.

[licaon-kter]

Isn't this like a server feature? Like ejabberd has a s3 contrib module, maybe that module says 201 instead?

How are you storing the files? Which xmpp server software?

[lorenz]

Yes, this is a server feature. But the thing is that the thing that's serving the HTTP upload requests is not the XMPP server, but instead an object storage directly. The "files" are objects in a Minio S3 cluster and the upload goes straight to that (the XMPP server only provides the presigned URL serving as an authorization). This is with Prosody and mod_http_upload_s3. ejabberd with S3 should IMO have the same problem, do we know that that works?

[hantu85]

Siskin is closely following specification of XEP-0363: HTTP File Upload, see #37 (comment) for the details as to why Siskin shows warning dialog.

[lorenz]

Yes, I have read the XEP and I know that it wants a 201. I would also fix it if that would be something that's easily doable but as that endpoint is served by a S3-style API if I were to just patch Minio to return 201 other applications would be unhappy. IMO the XEP should be fixed, but even Conversations written by the author of that XEP does support 200 since its introduction [1].
I just think that it is counterproductive to show that message when the operation clearly worked because of a technicality in the XEP, especially when there seems to be consensus among all other clients that this is OK and this not really carrying any risk of introducing ambiguities or incompatibilities.

I'm going to contact Daniel Gultsch to see if anything can be done on the standards side.

[1] https://codeberg.org/iNPUTmice/Conversations/commit/e48788e821c1c2cdab3647a0f4cce197ea626fe9

[arthef]

@lorenz the best and the correct way to ensure all software works together and is compatible, is when everybody follows the spec. Suggesting to deviate from the spec to be compatible with others who deviated from the spec is counterproductive. If we follow this path, we end up with chaos.

If the spec is wrong, change the spec and let everybody adjust software to adhere to the spec.

[lorenz]

@arturhefczyc I generally agree, I contacted the XEP author about this issue, hopefully we can get an errata or similar legitimizing using both 200 as well as 201.

[raucao]

+1 for supporting 200. The spec shouldn't even require 201 specifically, but leave that to the HTTP spec IMO. 200 is just as good as 201 across almost all of the Web. It's an easy fix here, and we have the same issue with the ejabberd module indeed (i.e. not with the module, but with Garage/S3). Just switched to it a couple of days ago, and all clients I tested work fine, except for only the Apple ones (Beagle, Siskin, Snikket, Monal).

[hantu85]

@raucao Out of curiosity, I've checked HTTP 1.1 RFC 2616 for a response on a PUT request and according to the RFC servers after creating a new object due to a PUT request should respond with201:

If a new resource is created, the origin server MUST inform the user agent via the 201 (Created) response. If an existing resource is modified, either the 200 (OK) or 204 (No Content) response codes SHOULD be sent to indicate successful completion of the request

and checking HTTP/2 RFC 9110 we have:

If the target resource does not have a current representation and the PUT successfully creates one, then the origin server MUST inform the user agent by sending a 201 (Created) response. If the target resource does have a current representation and that representation is successfully modified in accordance with the state of the enclosed representation, then the origin server MUST send either a 200 (OK) or a 204 (No Content) response to indicate successful completion of the request.

In the case of HTTP File Upload, we are always creating a new resource after the upload (as there is no support for modification), so requirement of 201 response code is OK in this case (XEP matches expected result from the HTTP specification). However, following just HTTP specification would allow us to accept also 200 and 204, but not responding with 201 on HTTP PUT creating a new resource is not complying with HTTP specification (but could even be considered a violation of HTTP protocol).

Due to that even leaving response code just to HTTP, we should be expecting 201 response but we could accept 200 or 204.

[lorenz]

I would indeed prefer if S3 returned 201 (for POST this is even configurable with success_action_status) as that would be the more correct response. But that ship has sailed more than 10 years ago, S3 is THE object storage API (pretty much any object storage system implements it). It is basically impossible to get AWS to change their implementation (as it's a breaking change) and thus all third-party implementations won't either.

I contacted the author of the XEP to maybe update it or issue an errata, but haven't heard back.

[raucao]

@hantu85 Yeah, I just meant the spec doesn't have to include that, since it's already in the HTTP spec. But even then, half the Web is violating that spec for PUTs anyway, and as @lorenz said, we won't get the S3 API to change either.

Btw, here's how I fixed it for our API, using the Nginx Lua Module.