hippware / wocky

Server side (Erlang/Elixir) software
13 stars 3 forks source link

Discuss: Migrate to XEP-0363 HTTP file upload #479

Closed toland closed 7 years ago

toland commented 7 years ago

MongooseIM 2.0.1 supports XEP-363 HTTP File Upload. Using a module already in MIM and backed by a XEP would probably be preferable to supporting our own implementation.

mod_http_upload docs

bernardd commented 7 years ago

In general I'm in favour of using the built-in stuff wherever we can, however in this case I'm not sure it will provide what we need (at least in its current form). Reading through the docs it looks like, funactionality wise, it does essentially exactly what TROS/S3 does right now - that is, provide a mechanism to get an S3 upload URL.

However it's missing a few key elements that we currently implement:

While we could work around most of these if we really wanted to, I'm not sure there's any benefit beyond XEP conformity (a change which would also mean work on the client side). There's another, arguably more important issue though:

Because this, at its core, does exactly what we currently do, it means it doesn't do exactly what we want to do in #452. There's no scope for inspecting the images (or whatever other file type) prior to making them available to other users.

So all in all, while it's a nice idea on the surface, I'm going to vote it down for the moment.

What might be worth considering, however, is modifying the TROS protocol to be XEP-363 compliant (with download extensions). At a glance it looks like they're similar enough that it wouldn't be a huge body of work. I'm open to opinions on that one.

toland commented 7 years ago

What might be worth considering, however, is modifying the TROS protocol to be XEP-363 compliant (with download extensions). At a glance it looks like they're similar enough that it wouldn't be a huge body of work. I'm open to opinions on that one.

👍

I am in favor of making the TROS interface comply with XEP-0363.

bengtan commented 7 years ago

I am in favor of making the TROS interface comply with XEP-0363.

I'm reluctant for anything which requires changes on the client. I believe this requires minor changes on the client?

If so ... in the long term, we'll do it. In the short term, I think we have more important issues.

bernardd commented 7 years ago

It would definitely require client changes. I'm with you - I think it should be done, but I think it should be done in the future rather than right now.

toland commented 7 years ago

I have created an issue in the icebox for this change: #486.

I believe the discussion is complete, so I am closing this issue.