From 459ede696680b277de8eeed06f8fc01b7ee90b22 Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Fri, 10 Oct 2025 11:51:12 +0100 Subject: [PATCH] Add soft_limit, increase_uri and rename info_url to info_uri --- .../configuration/config_documentation.md | 10 +- schema/synapse-config.schema.yaml | 16 +- synapse/api/errors.py | 15 +- synapse/config/repository.py | 35 ++++- synapse/media/media_repository.py | 14 +- tests/rest/client/test_media.py | 140 +++++++++++++++++- 6 files changed, 209 insertions(+), 21 deletions(-) diff --git a/docs/usage/configuration/config_documentation.md b/docs/usage/configuration/config_documentation.md index 5b49085973..0e64f99282 100644 --- a/docs/usage/configuration/config_documentation.md +++ b/docs/usage/configuration/config_documentation.md @@ -2180,7 +2180,11 @@ Options for each entry include: * `max_size` (byte size): Amount of data that can be uploaded in the time period by the user. Required. -* `msc4335_info_url` (string): Experimental MSC4335 URL to a page with more information about the upload limit. Optional. +* `msc4335_info_uri` (string): Experimental MSC4335 URI to where the user can find information about the upload limit. Optional. + +* `msc4335_soft_limit` (boolean): Experimental MSC4335 value to say if the limit can be increased. Optional. + +* `msc4335_increase_uri` (string): Experimental MSC4335 URI to where the user can increase the upload limit. Required if msc4335_soft_limit is true. Example configuration: ```yaml @@ -2189,7 +2193,9 @@ media_upload_limits: max_size: 100M - time_period: 1w max_size: 500M - msc4335_info_url: https://example.com/quota + msc4335_info_uri: https://example.com/quota + msc4335_soft_limit: true + msc4335_increase_uri: https://example.com/increase-quota ``` --- ### `max_image_pixels` diff --git a/schema/synapse-config.schema.yaml b/schema/synapse-config.schema.yaml index 2d74fee1cb..60340570ae 100644 --- a/schema/synapse-config.schema.yaml +++ b/schema/synapse-config.schema.yaml @@ -2438,16 +2438,26 @@ properties: description: >- Amount of data that can be uploaded in the time period by the user. Required. - msc4335_info_url: + msc4335_info_uri: type: string description: >- - Experimental MSC4335 URL to a page with more information about the upload limit. Optional. + Experimental MSC4335 URI to where the user can find information about the upload limit. Optional. + msc4335_soft_limit: + type: boolean + description: >- + Experimental MSC4335 value to say if the limit can be increased. Optional. + msc4335_increase_uri: + type: string + description: >- + Experimental MSC4335 URI to where the user can increase the upload limit. Required if msc4335_soft_limit is true. examples: - - time_period: 1h max_size: 100M - time_period: 1w max_size: 500M - msc4335_info_url: https://example.com/quota + msc4335_info_uri: https://example.com/quota + msc4335_soft_limit: true + msc4335_increase_uri: https://example.com/increase-quota max_image_pixels: $ref: "#/$defs/bytes" description: Maximum number of pixels that will be thumbnailed. diff --git a/synapse/api/errors.py b/synapse/api/errors.py index 1ca20b4c27..b400149023 100644 --- a/synapse/api/errors.py +++ b/synapse/api/errors.py @@ -524,11 +524,20 @@ class MSC4335UserLimitExceededError(SynapseError): self, code: int, msg: str, - info_url: str, + info_uri: str, + soft_limit: bool, + increase_uri: Optional[str] = None, ): - additional_fields = { - "org.matrix.msc4335.info_url": info_url, + if soft_limit and increase_uri is None: + raise ValueError("increase_uri must be provided if soft_limit is True") + + additional_fields: dict[str, Union[str, bool]] = { + "org.matrix.msc4335.info_uri": info_uri, + "org.matrix.msc4335.soft_limit": soft_limit, } + if soft_limit and increase_uri is not None: + additional_fields["org.matrix.msc4335.increase_uri"] = increase_uri + super().__init__( code, msg, diff --git a/synapse/config/repository.py b/synapse/config/repository.py index a667733ed7..e3591a44cf 100644 --- a/synapse/config/repository.py +++ b/synapse/config/repository.py @@ -134,7 +134,13 @@ class MediaUploadLimit: time_period_ms: int """The time period in milliseconds.""" - msc4335_info_url: Optional[str] = None + msc4335_info_uri: Optional[str] = None + """Used for experimental MSC4335 error code feature""" + + msc4335_soft_limit: Optional[bool] = None + """Used for experimental MSC4335 error code feature""" + + msc4335_increase_uri: Optional[str] = None """Used for experimental MSC4335 error code feature""" @@ -305,10 +311,33 @@ class ContentRepositoryConfig(Config): for limit_config in config.get("media_upload_limits", []): time_period_ms = self.parse_duration(limit_config["time_period"]) max_bytes = self.parse_size(limit_config["max_size"]) - msc4335_info_url = limit_config.get("msc4335_info_url", None) + msc4335_info_uri = limit_config.get("msc4335_info_uri", None) + msc4335_soft_limit = limit_config.get("msc4335_soft_limit", None) + msc4335_increase_uri = limit_config.get("msc4335_increase_uri", None) + + if ( + msc4335_info_uri is not None + or msc4335_soft_limit is not None + or msc4335_increase_uri is not None + ) and (not (msc4335_info_uri and msc4335_soft_limit is not None)): + raise ConfigError( + "If any of msc4335_info_uri, msc4335_soft_limit or " + "msc4335_increase_uri are set, then both msc4335_info_uri and " + "msc4335_soft_limit must be set." + ) + if msc4335_soft_limit and not msc4335_increase_uri: + raise ConfigError( + "msc4335_increase_uri must be set if msc4335_soft_limit is true." + ) self.media_upload_limits.append( - MediaUploadLimit(max_bytes, time_period_ms, msc4335_info_url) + MediaUploadLimit( + max_bytes, + time_period_ms, + msc4335_info_uri, + msc4335_soft_limit, + msc4335_increase_uri, + ) ) def generate_config_section(self, data_dir_path: str, **kwargs: Any) -> str: diff --git a/synapse/media/media_repository.py b/synapse/media/media_repository.py index e278308b1d..04072505ab 100644 --- a/synapse/media/media_repository.py +++ b/synapse/media/media_repository.py @@ -382,13 +382,21 @@ class MediaRepository: attempted_bytes=content_length, ) # If the MSC4335 experimental feature is enabled and the media limit - # has the info_url configured then we raise the MSC4335 error + # has the info_uri configured then we raise the MSC4335 error msc4335_enabled = await self.store.is_feature_enabled( auth_user.to_string(), ExperimentalFeature.MSC4335 ) - if msc4335_enabled and limit.msc4335_info_url: + if ( + msc4335_enabled + and limit.msc4335_info_uri + and limit.msc4335_soft_limit is not None + ): raise MSC4335UserLimitExceededError( - 403, "Media upload limit exceeded", limit.msc4335_info_url + 403, + "Media upload limit exceeded", + limit.msc4335_info_uri, + limit.msc4335_soft_limit, + limit.msc4335_increase_uri, ) # Otherwise we use the current behaviour albeit not spec compliant # See: https://github.com/element-hq/synapse/issues/18749 diff --git a/tests/rest/client/test_media.py b/tests/rest/client/test_media.py index e5ba148258..d5c2654849 100644 --- a/tests/rest/client/test_media.py +++ b/tests/rest/client/test_media.py @@ -46,7 +46,9 @@ from twisted.web.resource import Resource from synapse.api.errors import Codes, HttpResponseException from synapse.api.ratelimiting import Ratelimiter +from synapse.config import ConfigError from synapse.config._base import Config +from synapse.config.homeserver import HomeServerConfig from synapse.config.oembed import OEmbedEndpointConfig from synapse.http.client import MultipartResponse from synapse.http.types import QueryParams @@ -75,6 +77,7 @@ from tests.media.test_media_storage import ( from tests.server import FakeChannel, FakeTransport, ThreadedMemoryReactorClock from tests.test_utils import SMALL_PNG from tests.unittest import override_config +from tests.utils import default_config try: import lxml @@ -2971,6 +2974,93 @@ class MediaUploadLimits(unittest.HomeserverTestCase): channel = self.upload_media(900) self.assertEqual(channel.code, 200) + def test_msc4335_requires_config(self) -> None: + config_dict = default_config("test") + + # msc4335_info_uri and msc4335_soft_limit are required + # msc4335_increase_uri is required if msc4335_soft_limit is true + + with self.assertRaises(ConfigError): + HomeServerConfig().parse_config_dict( + { + "media_upload_limits": [ + { + "time_period": "1d", + "max_size": "1K", + "msc4335_info_uri": "https://example.com", + } + ], + **config_dict, + }, + "", + "", + ) + + with self.assertRaises(ConfigError): + HomeServerConfig().parse_config_dict( + { + "media_upload_limits": [ + { + "time_period": "1d", + "max_size": "1K", + "msc4335_info_uri": "https://example.com", + "msc4335_soft_limit": True, + } + ], + **config_dict, + }, + "", + "", + ) + + with self.assertRaises(ConfigError): + HomeServerConfig().parse_config_dict( + { + "media_upload_limits": [ + { + "time_period": "1d", + "max_size": "1K", + "msc4335_soft_limit": False, + } + ], + **config_dict, + }, + "", + "", + ) + + with self.assertRaises(ConfigError): + HomeServerConfig().parse_config_dict( + { + "media_upload_limits": [ + { + "time_period": "1d", + "max_size": "1K", + "msc4335_soft_limit": True, + } + ], + **config_dict, + }, + "", + "", + ) + + with self.assertRaises(ConfigError): + HomeServerConfig().parse_config_dict( + { + "media_upload_limits": [ + { + "time_period": "1d", + "max_size": "1K", + "msc4335_increase_uri": "https://example.com/increase", + } + ], + **config_dict, + }, + "", + "", + ) + @override_config( { "experimental_features": {"msc4335_enabled": True}, @@ -2978,13 +3068,14 @@ class MediaUploadLimits(unittest.HomeserverTestCase): { "time_period": "1d", "max_size": "1K", - "msc4335_info_url": "https://example.com", + "msc4335_info_uri": "https://example.com", + "msc4335_soft_limit": False, } ], } ) - def test_msc4335_returns_user_limit_exceeded(self) -> None: - """Test that the MSC4335 error is returned when experimental feature is enabled.""" + def test_msc4335_returns_hard_user_limit_exceeded(self) -> None: + """Test that the MSC4335 error is returned with soft_limit False when experimental feature is enabled.""" channel = self.upload_media(500) self.assertEqual(channel.code, 200) @@ -2994,7 +3085,41 @@ class MediaUploadLimits(unittest.HomeserverTestCase): channel.json_body["errcode"], "ORG.MATRIX.MSC4335_USER_LIMIT_EXCEEDED" ) self.assertEqual( - channel.json_body["org.matrix.msc4335.info_url"], "https://example.com" + channel.json_body["org.matrix.msc4335.info_uri"], "https://example.com" + ) + self.assertEqual(channel.json_body["org.matrix.msc4335.soft_limit"], False) + + @override_config( + { + "experimental_features": {"msc4335_enabled": True}, + "media_upload_limits": [ + { + "time_period": "1d", + "max_size": "1K", + "msc4335_info_uri": "https://example.com", + "msc4335_soft_limit": True, + "msc4335_increase_uri": "https://example.com/increase", + } + ], + } + ) + def test_msc4335_returns_soft_user_limit_exceeded(self) -> None: + """Test that the MSC4335 error is returned with soft_limit True when experimental feature is enabled.""" + channel = self.upload_media(500) + self.assertEqual(channel.code, 200) + + channel = self.upload_media(800) + self.assertEqual(channel.code, 403) + self.assertEqual( + channel.json_body["errcode"], "ORG.MATRIX.MSC4335_USER_LIMIT_EXCEEDED" + ) + self.assertEqual( + channel.json_body["org.matrix.msc4335.info_uri"], "https://example.com" + ) + self.assertEqual(channel.json_body["org.matrix.msc4335.soft_limit"], True) + self.assertEqual( + channel.json_body["org.matrix.msc4335.increase_uri"], + "https://example.com/increase", ) @override_config( @@ -3008,8 +3133,8 @@ class MediaUploadLimits(unittest.HomeserverTestCase): ], } ) - def test_msc4335_requires_info_url(self) -> None: - """Test that the MSC4335 error is not used if info_url is not provided.""" + def test_msc4335_requires_info_uri(self) -> None: + """Test that the MSC4335 error is not used if info_uri is not provided.""" channel = self.upload_media(500) self.assertEqual(channel.code, 200) @@ -3212,7 +3337,8 @@ class MediaUploadLimitsModuleOverrides(unittest.HomeserverTestCase): { "time_period": "1d", "max_size": "1K", - "msc4335_info_url": "https://example.com", + "msc4335_info_uri": "https://example.com", + "msc4335_soft_limit": False, }, ] }