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

Support HTTP File Upload (XEP-0363) #24

Closed
nesium opened this issue Aug 18, 2023 · 15 comments
Closed

Support HTTP File Upload (XEP-0363) #24

nesium opened this issue Aug 18, 2023 · 15 comments

Comments

@nesium
Copy link
Contributor

nesium commented Aug 18, 2023

https://xmpp.org/extensions/xep-0363.html

Also encryption?
https://xmpp.org/extensions/xep-0448.html

Required Prosody module: mod_http_file_share

@nesium
Copy link
Contributor Author

nesium commented Feb 8, 2024

@valeriansaliou I'm leaning heavily towards letting the clients handle the uploads. While I could pull in reqwest which seems to work with Wasm, on iOS we need to use the native URLSessionUploadTask so that we can continue uploading when the app is moved to the background. This means we wouldn't have any cross-platform gains by adding the dependency. I'm open to implement any state handling however if that would help.

I'll add a method tomorrow to send messages with attachments (and references while I'm at it) and to extract attachments from messages.

Does that sound good to you?

Btw. it seems that the Prosody module is not enabled on prose.org yet. I've enabled it on my test server by adding the following to the prosody.cfg.lua (+ domain + certificate changes)

-- https://prosody.im/doc/modules/mod_http_file_share
Component "upload.nsm.chat" "http_file_share"
  name = "HTTP File Upload"
  -- ...

@valeriansaliou
Copy link
Member

valeriansaliou commented Feb 8, 2024

I agree w/ letting the client handle the actual file upload. I was also thinking the same, this is platform-specific.

On the upload module, let me enable it for you after lunch...

@valeriansaliou
Copy link
Member

It's enabled on prose.org. The internal component host is upload.prose.local, and the public URL for GET/PUTs is https://app.prose.org/upload/

The component should be aware of this external URL and serve it accordingly in the stanzas for the GET/PUT URLs, let me know. Also, the upload.prose.local domain should be returned in server's disco items, let me know if it's not the case.

@nesium
Copy link
Contributor Author

nesium commented Feb 9, 2024

Thanks!

Edit: Nope. Still returning…

<iq xmlns="jabber:client" from="prose.org" id="0386134f-dc42-4bd5-bec5-e4b899a8120a" to="marc@prose.org/kZswoMl4" type="result">
  <query xmlns="http://jabber.org/protocol/disco#items">
    <item jid="groups.prose.org" name="Chatrooms" />
  </query>
</iq>

for the discovery request.

Edit2: Requesting the slot works if I hard-code upload.prose.local. But it should really be returned in the discovery response.

@valeriansaliou
Copy link
Member

Thank you, I’ll add it. Thought it would be automatic but apparently if the root domains do not match then it doesn’t add it.

@nesium
Copy link
Contributor Author

nesium commented Feb 9, 2024

It's now possible to send and receive messages with attachments.

Here's the new API…

export class ProseClient {
  // …
  requestUploadSlot(file_name: string, file_size: bigint): Promise<UploadSlot>;
}

export interface RoomBase {
    // …
    sendMessage(request: SendMessageRequest): Promise<void>;
    updateMessage(messageID: string, request: SendMessageRequest): Promise<void>;
}

export class Message {
  // …
  readonly attachments: Attachment[];
}

export class UploadSlot {
  readonly downloadURL: string;
  readonly uploadHeaders: UploadHeader[];
  readonly uploadURL: string;
}

export class UploadHeader {
  readonly name: string;
  readonly value: string;
}

export class SendMessageRequest {
  constructor();
  attachments: Attachment[];
  body?: string;
}

export class Attachment {
  constructor(url: string);
  description?: string;
  url: string;
}

As you can see RoomBase::sendMessage and RoomBase::updateMessage now accept a SendMessageRequest instead of just a String…

let attachment = new Attachment("https://www.prose.org/my-image.jpg");
attachment.description = "Optional attachment description";

let req = new SendMessageRequest();
req.body = message;
req.attachments = [attachment];

and Message now has a attachments property.

The procedure is

  • request one or more UploadSlot(s) from the client
  • upload the file(s) (don't forget to set the uploadHeaders on your PUT request)
  • construct a SendMessageRequest to send the message

Edit: I will address "references" in another commit, since that has a bit more to it if you look more closely.

@valeriansaliou
Copy link
Member

valeriansaliou commented Feb 10, 2024

Mhhh I've updated the library and changed the sendMessage/updateMessage prototype, while it's working fine to send a message to someone else, I don't receive the delegate's messagesAppended and messagesUpdated for my own sent messages (anymre). I only receive such delegates for messages sent to me by remote parties.

Note that it seems to be only affecting 1-to-1 rooms.

@valeriansaliou
Copy link
Member

valeriansaliou commented Feb 10, 2024

Also, second thing, is there a way I could get the contentType in the Attachment structure? (sending it over the XMPP message stanza if possible, optional)

Edit: just noticed that the OOB data XEP does not allow setting a MIME type, that's okay, I'll assume it in the views based on the file extension, for display.

@valeriansaliou
Copy link
Member

valeriansaliou commented Feb 10, 2024

Some updates:

  • File uploads implemented in the app + audio recordings uploads
  • Files now show in the views

However, I'd need a way to send the following attributes for those specific file types, so that I can retrieve those metas from the Message structure:

  • Images: image size (width + height) in pixels, as numbers (this helps pre-compute the space used by an image preview, before it loads, so that I can assign the space in the views and prevent "jumping" message view issues when images load and appear on-screen)
  • Audios: audio duration, in seconds, as a number (this lets me show the total duration of an audio message, before it is played and loaded from the server)

Regarding the MIME type, I'm currently guessing it from the file extension, would be good to be able to send it and have it in the Message structure as well.

@nesium
Copy link
Contributor Author

nesium commented Feb 11, 2024

I guess we could support XEP-0385 in addition to XEP-0066 which would us bring a bit closer to our goal.

From the comment in this issue it seems that Movim performs a HEAD request for each shared file to determine the content-type.

Here are some more related issues on the Conversations issue tracker:

@valeriansaliou
Copy link
Member

valeriansaliou commented Feb 11, 2024

XEP-0385 sounds perfect for the use case. I’ll be needing to share thumbnails so that we don’t load the whole pictures in the preview at some point anyway.

What’s especially important to me is the ability to share image width/height and audio duration in the stanza. The specification allows to define a width/height for the thumbnail, which is perfect for me to determine a w/h ratio for display.

Also, it'd be a plus if I could share a thumbnail for video files, also for image files, since I can generate them from the browser upon upload, and upload both the file + its thumbnail, then send it all as part of XEP-0385. The thumbnail images will be especially useful for the message view, since it'll allow me to generate a preview of the video (which looks much nicer than a small play button), and also it'll help save quite some network resources for images, by loading a smaller image in its preview (I currently load the original image in the preview).

@nesium
Copy link
Contributor Author

nesium commented Feb 14, 2024

Quite a bit of new API/changes…

Attachment now needs more info about the file but it can be derived from the UploadSlot (AttachmentMetadata.fromSlot(slot)).

The Attachment constructor is gone but there are 4 dedicated initializers Attachment.imageAttachment, Attachment.audioAttachment, Attachment.videoAttachment and Attachment.fileAttachment.

So shouldn't need too much changes but you can provide a Thumbnail and a duration depending on the initializer.

/// The type of attachment. Derived from the attachment's media type.
export enum AttachmentType {
  Image = 0,
  Audio = 1,
  Video = 2,
  File = 3,
}

export class AttachmentMetadata {
  /// Instantiates a new `AttachmentMetadata`. If you have an `UploadSlot` available, prefer to
  /// use `AttachmentMetadata.fromSlot` instead.
  constructor(url: string, media_type: string, file_name: string, file_size: bigint)

  /// Instantiates a new `AttachmentMetadata` from an `UploadSlot`.
  static fromSlot(slot: UploadSlot): AttachmentMetadata
}

export class Attachment {
  /// Creates an attachment with an image. Provide a thumbnail for the inline preview.
  static imageAttachment(metadata: AttachmentMetadata, thumbnail: Thumbnail): Attachment

  /// Creates an attachment with an audio file. Provide the duration (in seconds) of the
  /// audio clip for the preview.
  static audioAttachment(metadata: AttachmentMetadata, duration: bigint): Attachment

  /// Creates an attachment with a video. Provide the duration of the video and a thumbnail
  /// for the inline preview.
  static videoAttachment(
    metadata: AttachmentMetadata,
    duration: bigint,
    thumbnail: Thumbnail
  ): Attachment

  /// Creates an attachment with a generic file.
  static fileAttachment(metadata: AttachmentMetadata): Attachment

  /// The duration of the attachment in seconds. Only available (but not necessarily) if `type`
  /// is `AttachmentType.Audio`.
  readonly duration: bigint | undefined

  /// The file name of the attachment.
  readonly fileName: string

  /// The size of the attachment in bytes (if available).
  readonly fileSize: bigint | undefined

  /// The media type of the attachment.
  readonly mediaType: string

  /// A thumbnail for inline preview of the attachment. Only available (but not necessarily)
  /// if `type` is `AttachmentType.Image` or `AttachmentType.Video`.
  readonly thumbnail: Thumbnail | undefined

  /// The type of the attachment.
  readonly type: AttachmentType

  /// The URL of the attachment.
  readonly url: string
}

export class Thumbnail {
  /// Instantiates a new `Thumbnail`. If you have an `UploadSlot` available, prefer to
  /// use `Thumbnail.fromSlot` instead.
  constructor(url: string, media_type: string, width: number, height: number)

  /// Instantiates a new `Thumbnail` from an `UploadSlot` and a given `width` and `height`.
  static fromSlot(slot: UploadSlot, width: number, height: number): Thumbnail

  /// The height of the thumbnail in pixels.
  readonly height: number | undefined

  /// The media type of the thumbnail.
  readonly mediaType: string

  /// The URL of the thumbnail.
  readonly url: string

  /// The width of the thumbnail in pixels.
  readonly width: number | undefined
}

export class UploadSlot {
  /// The URL where the file will be available after the upload.
  readonly downloadURL: string

  /// The name of the file from the initial request.
  readonly fileName: string

  /// The size of the file from the initial request.
  readonly fileSize: bigint

  /// The media type of the file from the initial request.
  readonly mediaType: string

  /// Set these headers on your PUT request when uploading.
  readonly uploadHeaders: UploadHeader[]

  /// The URL where the file should be uploaded to.
  readonly uploadURL: string
}

@nesium
Copy link
Contributor Author

nesium commented Feb 14, 2024

UI-wise are we eventually going to a message editor where a message and multiple attachments can be combined?

Screenshot 2024-02-14 at 11 09 53

@valeriansaliou
Copy link
Member

Thanks you, will make the changes. On the form, yes, that will be the plan, I just need to implement message formatting first so that we can have a rich form editor, and then I can also inject image previews there.

@valeriansaliou
Copy link
Member

All implemented and working great! prose-im/prose-app-web@8136613

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

2 participants