500 Internal server error on thumbnail download if width/height params missing

[babolivier]

Description

When I request the thumbnail for a picture, Synapse responds with a 500 Internal error.

Steps to reproduce

• Go to https://[your homeserver]/_matrix/media/r0/thumbnail/matrix.org/[media ID]
• Witness the 500 happen with the following body instead of a thumbnailed picture:

{
    "errcode": "M_UNKNOWN",
    "error": "Internal server error"
}

There's also a trace in the logs when I perform the request:

Dec 27 12:34:16 aether python2.7[29031]: 2017-12-27 12:34:16,979 - synapse.http.server - 139 - ERROR - GET-2115019- Failed handle request synapse.rest.media.v1.thumbnail_resource._async_render_GET on <synapse.rest.media.v1.thumbnail_resource.ThumbnailResource instance at 0x7f0b102e2ea8>: <SynapseRequest at 0x7f0ae09754d0 method=GET uri=/_matrix/media/r0/thumbnail/matrix.org/[media ID] clientproto=HTTP/1.1 site=8008>
Dec 27 12:34:16 aether python2.7[29031]: Traceback (most recent call last):
Dec 27 12:34:16 aether python2.7[29031]:   File "/usr/lib/python2.7/site-packages/synapse/http/server.py", line 118, in wrapped_request_handler
Dec 27 12:34:16 aether python2.7[29031]:     yield request_handler(self, request)
Dec 27 12:34:16 aether python2.7[29031]:   File "/usr/lib/python2.7/site-packages/twisted/internet/defer.py", line 1384, in _inlineCallbacks
Dec 27 12:34:16 aether python2.7[29031]:     result = result.throwExceptionIntoGenerator(g)
Dec 27 12:34:16 aether python2.7[29031]:   File "/usr/lib/python2.7/site-packages/twisted/python/failure.py", line 408, in throwExceptionIntoGenerator
Dec 27 12:34:16 aether python2.7[29031]:     return g.throw(self.type, self.value, self.tb)
Dec 27 12:34:16 aether python2.7[29031]:   File "/usr/lib/python2.7/site-packages/synapse/rest/media/v1/thumbnail_resource.py", line 76, in _async_render_GET
Dec 27 12:34:16 aether python2.7[29031]:     width, height, method, m_type
Dec 27 12:34:16 aether python2.7[29031]:   File "/usr/lib/python2.7/site-packages/twisted/internet/defer.py", line 1386, in _inlineCallbacks
Dec 27 12:34:16 aether python2.7[29031]:     result = g.send(result)
Dec 27 12:34:16 aether python2.7[29031]:   File "/usr/lib/python2.7/site-packages/synapse/rest/media/v1/thumbnail_resource.py", line 239, in _respond_remote_thumbnail
Dec 27 12:34:16 aether python2.7[29031]:     width, height, method, m_type, thumbnail_infos
Dec 27 12:34:16 aether python2.7[29031]:   File "/usr/lib/python2.7/site-packages/synapse/rest/media/v1/thumbnail_resource.py", line 334, in _select_thumbnail
Dec 27 12:34:16 aether python2.7[29031]:     size_quality = abs((d_w - t_w) * (d_h - t_h))
Dec 27 12:34:16 aether python2.7[29031]: TypeError: unsupported operand type(s) for -: 'NoneType' and 'int'
Dec 27 12:34:16 aether python2.7[29031]: 2017-12-27 12:34:16,986 - synapse.access.http.8008 - 91 - INFO - GET-2115019- 127.0.0.1 - 8008 - {None} Processed request: 266ms (11ms, 1ms) (25ms/8) 55B 500 "GET /_matrix/media/r0/thumbnail/matrix.org/[media ID] HTTP/1.1" "Mozilla/5.0 (X11; Linux x86_64; rv:57.0) Gecko/20100101 Firefox/57.0"

The media ID is redacted because I witnessed it on several medias (all hosted on matrix.org, though, I didn't try the other way round).

Version information

• Homeserver: This issue was witnessed on both the matrix.org homeserver and the one my organisation is running, matrix.trancendances.fr. In both cases, the media is hosted on the matrix.org homeserver.

About matrix.trancendances.fr:

• Version: Synapse/0.25.1
• Install method: Official package from Archlinux repos: https://www.archlinux.org/packages/community/any/matrix-synapse/
• Platform: Standard VPS running Archlinux

[richvdh]

@babolivier: are you saying that no thumbnails work at all? Or is this just affecting some media?

[babolivier]

This failed with each thumbnail I tried to load.

[richvdh]

is it still a problem?

[babolivier]

I just realised I didn't specify width and height parameters. Works now with them, and still returns a 500 error when I don't input them.

That being said, I think the error is quite misleading and made me think "there's an issue with synapse" instead of "I should check the spec to see if I didn't forget anything".

[richvdh]

sure, missing params should be a 400, not a 500

[turt2live]

The spec claims those parameters are optional atm, fwiw.

[richvdh]

blocked on a decision on matrix-org/matrix-spec-proposals#1883

[richvdh]

No longer blocked; we should return a 400 if the params are missing

[richvdh]

fixed by #5258