Filename included in media upload url doesn't work

poVoq commented

2022-01-22 18:10:59 +00:00

(Migrated from gitlab.com)

When uploading files the Matrix specs say that the actual filename should be included in the URL:
https://matrix.org/docs/spec/client_server/r0.6.1#get-matrix-media-r0-download-servername-mediaid-filename

However Conduit only constructs the URL up to the media id.

When requesting the same file with the actual filename in the end it fails with "404 Not Found".

This is a problem with using Heisenbridge as it adds the file name to the download URL when pasting the link into the IRC channel.

Thanks for looking into this

When uploading files the Matrix specs say that the actual filename should be included in the URL: https://matrix.org/docs/spec/client_server/r0.6.1#get-matrix-media-r0-download-servername-mediaid-filename However Conduit only constructs the URL up to the media id. When requesting the same file with the actual filename in the end it fails with "404 Not Found". This is a problem with using Heisenbridge as it adds the file name to the download URL when pasting the link into the IRC channel. Thanks for looking into this

timokoesters commented

2022-01-25 11:14:13 +00:00

(Migrated from gitlab.com)

Is this a new endpoint? It looks like we don't support it at all yet.

jplatte commented

2022-01-25 11:38:35 +00:00

(Migrated from gitlab.com)

This is not a new endpoint, it's ruma::api::client::r0::media::get_content_as_filename.

This is not a new endpoint, it's `ruma::api::client::r0::media::get_content_as_filename`.

ticho34782694 commented

2022-01-26 23:37:58 +00:00

(Migrated from gitlab.com)

mentioned in merge request !266

ticho34782694 commented

2022-01-27 17:40:21 +00:00

(Migrated from gitlab.com)

This can be closed, as !266 adds the endpoint.

poVoq commented

2022-01-27 23:50:05 +00:00

(Migrated from gitlab.com)

So I just update to the new build and there is a different issue now.

It newly fails to account for the actual server name and sends http://localhost:6167/... instead of the actually configured hostname.

The remaining part of the url seems to be correct with file name at the end now and it does work when I manually replace it with the correct domain.

I didn't change any other settings and before it got the domain just fine.

So I just update to the new build and there is a different issue now. It newly fails to account for the actual server name and sends `http://localhost:6167/...` instead of the actually configured hostname. The remaining part of the url seems to be correct with file name at the end now and it does work when I manually replace it with the correct domain. I didn't change any other settings and before it got the domain just fine.

ticho34782694 commented

2022-01-27 23:56:39 +00:00

(Migrated from gitlab.com)

I'm not sure I understand what you are describing. Media download works for me - both local and federated, and according to my web logs correct requests are being sent...

poVoq commented

2022-01-28 00:09:53 +00:00

(Migrated from gitlab.com)

Yes on the Matrix side everything seems fine, but that was also before.

But Heisenbrige makes a link for IRC out of it and while the link with the file-name as by the Matrix specs works now with Conduit, Heisenbridge does not anymore get the real hostname. This is some sort of regression as it managed to do so before this fix.

Yes on the Matrix side everything seems fine, but that was also before. But Heisenbrige makes a link for IRC out of it and while the link with the file-name as by the Matrix specs works now with Conduit, Heisenbridge does not anymore get the real hostname. This is some sort of regression as it managed to do so before this fix.

ticho34782694 commented

2022-01-28 22:20:43 +00:00

(Migrated from gitlab.com)

I looked into how heisenbridge works (been wanting to set it up anyway), and I think you just need to set "mediaurl" in heisenbridge to the correct url of your homeserver. Also check output (you may need -v or -vv on heisenbridge startup for lines like:

DEBUG:root:Did not find .well-known for HS
DEBUG:root:Could not use direct connection to HS
WARNING:root:Using internal URL for homeserver, media links are likely broken!

I looked into how heisenbridge works (been wanting to set it up anyway), and I think you just need to set "mediaurl" in heisenbridge to the correct url of your homeserver. Also check output (you may need `-v` or `-vv` on heisenbridge startup for lines like: ``` DEBUG:root:Did not find .well-known for HS DEBUG:root:Could not use direct connection to HS WARNING:root:Using internal URL for homeserver, media links are likely broken! ```

poVoq commented

2022-01-29 11:26:07 +00:00

(Migrated from gitlab.com)

True, manually setting the mediaurl seems to work.

But it is odd that this was not necessary before this change.

True, manually setting the mediaurl seems to work. But it is odd that this was not necessary before this change.

poVoq commented

2022-02-06 10:23:37 +00:00

(Migrated from gitlab.com)

@ticho34782694 I noticed that image links are also presented as downloads when accessing them externally through Heisenbridge for example, instead of being displayed directly by the browser.

I think this is a html header issue that might be fixable in conduit? (or alternatively maybe in the default Nginx setup?).

@ticho34782694 I noticed that image links are also presented as downloads when accessing them externally through Heisenbridge for example, instead of being displayed directly by the browser. I think this is a html header issue that might be fixable in conduit? (or alternatively maybe in the default Nginx setup?).

ticho34782694 commented

2022-02-06 13:24:53 +00:00

(Migrated from gitlab.com)

This is between the browser and the webserver that is serving the content. See documentation on "Content-Disposition" and "Content-Type" HTTP headers, and configure your webserver accordingly.

In any case, this is wildly off-topic here.

This is between the browser and the webserver that is serving the content. See documentation on "Content-Disposition" and "Content-Type" HTTP headers, and configure your webserver accordingly. In any case, this is wildly off-topic here.

poVoq commented

2022-02-13 22:44:20 +00:00

(Migrated from gitlab.com)

Seems like there is actually another issue with the content-type for images being set to application/json

You can try an image link shared though my Conduit server here. It does show up fine in Element, but when accessing it though Heisenbridge etc the behavior is a bit strange.

When you curl it like this:

curl -IL https://irc.freegamedev.net/_matrix/media/r0/download/irc.freegamedev.net/shJ1j4Kf75Wa5aivULQZMSZVsae8eRWw/XMPP_logo.png

It shows the problem

content-type: application/json
content-length: 0

Seems like there is actually another issue with the content-type for images being set to `application/json` You can try an image link shared though my Conduit server [here](https://irc.freegamedev.net/_matrix/media/r0/download/irc.freegamedev.net/shJ1j4Kf75Wa5aivULQZMSZVsae8eRWw/XMPP_logo.png). It does show up fine in Element, but when accessing it though Heisenbridge etc the behavior is a bit strange. When you curl it like this: `curl -IL https://irc.freegamedev.net/_matrix/media/r0/download/irc.freegamedev.net/shJ1j4Kf75Wa5aivULQZMSZVsae8eRWw/XMPP_logo.png` It shows the problem ``` content-type: application/json content-length: 0 ```

ticho34782694 commented

2022-02-20 14:29:57 +00:00

(Migrated from gitlab.com)

I'm sorry, but I cannot reproduce it, with your example link or any of several random images from various homeservers. The Content-Type header always gets set to whatever the origin server sets. Usually it's one of image/*, or at worst application/octet-stream.

I'm sorry, but I cannot reproduce it, with your example link or any of several random images from various homeservers. The Content-Type header always gets set to whatever the origin server sets. Usually it's one of `image/*`, or at worst `application/octet-stream`.

tulir commented

2022-02-20 14:45:21 +00:00

(Migrated from gitlab.com)

Seems like a bug with HEAD request handling (missing HEAD support altogether?), it returns HTTP 400 and application/json rather than 200 and the correct type.

Seems like a bug with `HEAD` request handling (missing `HEAD` support altogether?), it returns HTTP 400 and `application/json` rather than 200 and the correct type.

ticho34782694 commented

2022-02-20 14:50:06 +00:00

(Migrated from gitlab.com)

Hm, that seems to be a regression after the rocket->axum switch. Log shows:

Feb 20 15:48:44.554  WARN http_request{path=/_matrix/media/r0/download/:server_name/:media_id/:filename}: conduit::ruma_wrapper::axum: MethodMismatch { expected: GET, received: HEAD }
Feb 20 15:48:44.554  WARN http_request{path=/_matrix/media/r0/download/:server_name/:media_id/:filename}: conduit::error: 400 Bad Request: M_BAD_JSON: Failed to deserialize request.

HEAD seems to work fine on a rocket-using conduit instance.

Hm, that seems to be a regression after the rocket->axum switch. Log shows: ``` Feb 20 15:48:44.554 WARN http_request{path=/_matrix/media/r0/download/:server_name/:media_id/:filename}: conduit::ruma_wrapper::axum: MethodMismatch { expected: GET, received: HEAD } Feb 20 15:48:44.554 WARN http_request{path=/_matrix/media/r0/download/:server_name/:media_id/:filename}: conduit::error: 400 Bad Request: M_BAD_JSON: Failed to deserialize request. ``` HEAD seems to work fine on a rocket-using conduit instance.

timokoesters commented

2022-02-20 16:53:32 +00:00

(Migrated from gitlab.com)

The spec does not mention HEAD, I don't know if we need to support it.

ticho34782694 commented

2022-02-20 16:56:29 +00:00

(Migrated from gitlab.com)

Matrix spec does not mention it, but HTTP spec does. See a similar issue for Synapse: https://github.com/matrix-org/synapse/issues/6008

shadowjonathan commented

2022-02-20 16:57:46 +00:00

(Migrated from gitlab.com)

I believe requests need to be marked "safe" for HEAD requests, we don't want a random POST or the likes being triggered by a HEAD probe.

I'd say to only enable HEAD for GET endpoints for now.

I believe requests need to be marked "safe" for HEAD requests, we don't want a random POST or the likes being triggered by a HEAD probe. I'd say to only enable `HEAD` for `GET` endpoints for now.

tulir commented

2022-02-20 17:28:53 +00:00

(Migrated from gitlab.com)

HEAD is the same as GET by definition (minus the response body). There shouldn't be any need to mark things as safe (other than defining that an endpoint allows GET)

`HEAD` is the same as `GET` by definition (minus the response body). There shouldn't be any need to mark things as safe (other than defining that an endpoint allows `GET`)

jplatte commented

2022-02-20 19:21:16 +00:00

(Migrated from gitlab.com)

I have an idea for how to fix this, can probably post an MR later.

poVoq commented

2022-03-05 10:37:11 +00:00

(Migrated from gitlab.com)

@jplatte did you manage to look into this?

jplatte commented

2022-03-05 19:18:39 +00:00

(Migrated from gitlab.com)

I think I remember sbd. in the Matrix room saying they want to take a look at it, but maybe I'm getting things confused. Either way I think this can be solved by changing this line to return MethodFilter::GET | MethodFilter::HEAD and adding a middleware that strips the response body when the request uses a HEAD method. Writing the middleware should be pretty easy using axum::middleware::from_fn. I don't think I'll write the patch, but please let me know if those instructions are unclear or don't work.

I think I remember sbd. in the Matrix room saying they want to take a look at it, but maybe I'm getting things confused. Either way I think this can be solved by changing [this line](https://gitlab.com/famedly/conduit/-/blob/36a6d724fe00a8da3219ea42d0776be7a9694e8a/src/main.rs#L430) to return `MethodFilter::GET | MethodFilter::HEAD` and adding a middleware that strips the response body when the request uses a `HEAD` method. Writing the middleware should be pretty easy using [axum::middleware::from_fn](https://docs.rs/axum/latest/axum/middleware/fn.from_fn.html). I don't think I'll write the patch, but please let me know if those instructions are unclear or don't work.

ticho34782694 commented

2022-03-13 09:05:29 +00:00

(Migrated from gitlab.com)

I haven't tried adding the middleware itself, but merely adjusting the mapping in method_to_filter() function did not cause any change - conduit still hits conduit::ruma_wrapper::axum: MethodMismatch { expected: GET, received: HEAD }.

In that state, I expected a HEAD request to at least return a response with a complete body.

I haven't tried adding the middleware itself, but merely adjusting the mapping in method_to_filter() function did not cause any change - conduit still hits `conduit::ruma_wrapper::axum: MethodMismatch { expected: GET, received: HEAD }`. In that state, I expected a HEAD request to at least return a response with a complete body.

M0dEx commented

2022-04-02 19:37:37 +00:00

(Migrated from gitlab.com)

@jplatte

Either way I think this can be solved by changing this line to return MethodFilter::GET | MethodFilter::HEAD and adding a middleware that strips the response body when the request uses a HEAD method. Writing the middleware should be pretty easy using axum::middleware::from_fn.

Unfortunately, I do not think it will be that easy. The MethodMismatch error is from Ruma, not Conduit.

@jplatte > Either way I think this can be solved by changing [this line](https://gitlab.com/famedly/conduit/-/blob/36a6d724fe00a8da3219ea42d0776be7a9694e8a/src/main.rs#L430) to return `MethodFilter::GET | MethodFilter::HEAD` and adding a middleware that strips the response body when the request uses a `HEAD` method. Writing the middleware should be pretty easy using [axum::middleware::from_fn](https://docs.rs/axum/latest/axum/middleware/fn.from_fn.html). Unfortunately, I do not think it will be that easy. The MethodMismatch error is from Ruma, not Conduit.

Filename included in media upload url doesn't work #218