Unknown opened 1 year ago
|
|
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? |
|
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? |
|
Siskin is closely following specification of XEP-0363: HTTP File Upload, see https://github.com/tigase/siskin-im/issues/37#issuecomment-624294330 for the details as to why Siskin shows warning dialog. |
|
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 |
|
@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. |
|
@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. |
|
+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). |
|
@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 with
and checking HTTP/2 RFC 9110 we have:
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 Due to that even leaving response code just to HTTP, we should be expecting 201 response but we could accept 200 or 204. |
|
I would indeed prefer if S3 returned 201 (for POST this is even configurable with I contacted the author of the XEP to maybe update it or issue an errata, but haven't heard back. |
|
@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. |
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.
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.