UnboundLocalError raised when response body exceeds max size

[callahad]

A bug with 1.25.0 was reported in the Synapse Package Maintainer's channel:

I got a nice Traceback in 1.25.0 after start:

2021-01-15 20:32:45,345 - synapse.http.matrixfederationclient - 987 - WARNING - GET-25- {GET-O-1} [matrix.org] Requested file is too large > 10485760 bytes
2021-01-15 20:32:45,345 - synapse.rest.media.v1.media_repository - 417 - ERROR - GET-25- Failed to fetch remote media matrix.org/cPeSAplLYzzcKlpJjLtwlzrT
Traceback (most recent call last):                                                                                     
  File "/usr/local/lib/python3.7/site-packages/synapse/rest/media/v1/media_repository.py", line 384, in _download_remote_file
    "allow_remote": "false"                                                                                            
  File "/usr/local/lib/python3.7/site-packages/synapse/http/matrixfederationclient.py", line 1004, in get_file
    length,                                                                                                            
UnboundLocalError: local variable 'length' referenced before assignment

From a (very) quick glance at the source I would guess that synapse hits except BodyExceededMaxSize before the length = ... statement in the try-block, which causes the logger.info statement to reference a non-existing length variable.
Otherwise things appear to work fine. :D

I suspect this was introduced as part of this commit:

ff5c4da#diff-a53ea02bf6a0cbb98e925b47e71f4ffb149075536543c1eca15366fc65c90aefL977-R983

The diagnosis looks right to me: when read_body_with_max_size throws, it skips the length assignment statement two lines later. However, we don't raise or return from the exception handler, so we continue through to a logging statement which tries to use the unassigned length variable.

[clokep]

Just to cross-reference, this is a bug in #8950.