From 998463222bdd25182db5b4a91a9f7be2f7e2e888 Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Fri, 29 Aug 2025 18:29:04 +0100 Subject: [PATCH 1/4] Support for experimental MSC4335 - Make it available behind experimental feature flag - return it for media upload limits --- changelog.d/18876.feature | 1 + .../configuration/config_documentation.md | 9 ++ schema/synapse-config.schema.yaml | 28 ++++-- synapse/api/errors.py | 18 ++++ synapse/config/experimental.py | 3 + synapse/config/repository.py | 10 ++- synapse/media/media_repository.py | 13 +++ synapse/rest/admin/experimental_features.py | 3 + tests/rest/client/test_media.py | 89 ++++++++++++++++--- 9 files changed, 153 insertions(+), 21 deletions(-) create mode 100644 changelog.d/18876.feature diff --git a/changelog.d/18876.feature b/changelog.d/18876.feature new file mode 100644 index 00000000000..a976987a56b --- /dev/null +++ b/changelog.d/18876.feature @@ -0,0 +1 @@ +Add support for experimental [MSC4335](https://github.com/matrix-org/matrix-spec-proposals/pull/4335) M_USER_LIMIT_EXCEEDED error code for media upload limits. diff --git a/docs/usage/configuration/config_documentation.md b/docs/usage/configuration/config_documentation.md index 3c401d569bb..4571141ab36 100644 --- a/docs/usage/configuration/config_documentation.md +++ b/docs/usage/configuration/config_documentation.md @@ -2176,6 +2176,14 @@ These settings can be overridden using the `get_media_upload_limits_for_user` mo Defaults to `[]`. +Options for each entry include: + +* `time_period` (duration): The time period over which the limit applies. Required. + +* `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. + Example configuration: ```yaml media_upload_limits: @@ -2183,6 +2191,7 @@ media_upload_limits: max_size: 100M - time_period: 1w max_size: 500M + msc4335_info_url: https://example.com/quota ``` --- ### `max_image_pixels` diff --git a/schema/synapse-config.schema.yaml b/schema/synapse-config.schema.yaml index 2a7f94a7002..0a658979d9a 100644 --- a/schema/synapse-config.schema.yaml +++ b/schema/synapse-config.schema.yaml @@ -2426,20 +2426,30 @@ properties: module API [callback](../../modules/media_repository_callbacks.md#get_media_upload_limits_for_user). default: [] items: - time_period: - type: "#/$defs/duration" - description: >- - The time period over which the limit applies. Required. - max_size: - type: "#/$defs/bytes" - description: >- - Amount of data that can be uploaded in the time period by the user. - Required. + type: object + required: + - time_period + - max_size + properties: + time_period: + $ref: "#/$defs/duration" + description: >- + The time period over which the limit applies. Required. + max_size: + $ref: "#/$defs/bytes" + description: >- + Amount of data that can be uploaded in the time period by the user. + Required. + msc4335_info_url: + type: string + description: >- + Experimental MSC4335 URL to a page with more information about the upload limit. Optional. examples: - - time_period: 1h max_size: 100M - time_period: 1w max_size: 500M + msc4335_info_url: https://example.com/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 fb6721c0eea..ec477ec8800 100644 --- a/synapse/api/errors.py +++ b/synapse/api/errors.py @@ -513,6 +513,24 @@ def error_dict(self, config: Optional["HomeServerConfig"]) -> "JsonDict": ) +class MSC4335UserLimitExceededError(SynapseError): + """ + Experimental implementation of MSC4335 M_USER_LIMIT_EXCEEDED error + """ + + def __init__( + self, + code: int, + msg: str, + info_url: str, + ): + additional_fields = { + "org.matrix.msc4335.info_url": info_url, + "org.matrix.msc4335.errcode": "M_USER_LIMIT_EXCEEDED", + } + super().__init__(code, msg, Codes.UNKNOWN, additional_fields=additional_fields) + + class EventSizeError(SynapseError): """An error raised when an event is too big.""" diff --git a/synapse/config/experimental.py b/synapse/config/experimental.py index d7a3d675583..f5369017a50 100644 --- a/synapse/config/experimental.py +++ b/synapse/config/experimental.py @@ -595,3 +595,6 @@ def read_config( # MSC4306: Thread Subscriptions # (and MSC4308: Thread Subscriptions extension to Sliding Sync) self.msc4306_enabled: bool = experimental.get("msc4306_enabled", False) + + # MSC4335: M_USER_LIMIT_EXCEEDED error + self.msc4335_enabled: bool = experimental.get("msc4335_enabled", False) diff --git a/synapse/config/repository.py b/synapse/config/repository.py index e7d23740f9e..a667733ed74 100644 --- a/synapse/config/repository.py +++ b/synapse/config/repository.py @@ -21,7 +21,7 @@ import logging import os -from typing import Any, Dict, List, Tuple +from typing import Any, Dict, List, Optional, Tuple import attr @@ -134,6 +134,9 @@ class MediaUploadLimit: time_period_ms: int """The time period in milliseconds.""" + msc4335_info_url: Optional[str] = None + """Used for experimental MSC4335 error code feature""" + class ContentRepositoryConfig(Config): section = "media" @@ -302,8 +305,11 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None: 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) - self.media_upload_limits.append(MediaUploadLimit(max_bytes, time_period_ms)) + self.media_upload_limits.append( + MediaUploadLimit(max_bytes, time_period_ms, msc4335_info_url) + ) def generate_config_section(self, data_dir_path: str, **kwargs: Any) -> str: assert data_dir_path is not None diff --git a/synapse/media/media_repository.py b/synapse/media/media_repository.py index a3c0b3036e5..6a0f276672b 100644 --- a/synapse/media/media_repository.py +++ b/synapse/media/media_repository.py @@ -37,6 +37,7 @@ Codes, FederationDeniedError, HttpResponseException, + MSC4335UserLimitExceededError, NotFoundError, RequestSendFailed, SynapseError, @@ -68,6 +69,7 @@ from synapse.media.thumbnailer import Thumbnailer, ThumbnailError from synapse.media.url_previewer import UrlPreviewer from synapse.metrics.background_process_metrics import run_as_background_process +from synapse.rest.admin.experimental_features import ExperimentalFeature from synapse.storage.databases.main.media_repository import LocalMedia, RemoteMedia from synapse.types import UserID from synapse.util.async_helpers import Linearizer @@ -382,6 +384,17 @@ async def create_or_update_content( sent_bytes=uploaded_media_size, 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 + msc4335_enabled = await self.store.is_feature_enabled( + auth_user.to_string(), ExperimentalFeature.MSC4335 + ) + if msc4335_enabled and limit.msc4335_info_url: + raise MSC4335UserLimitExceededError( + 403, "Media upload limit exceeded", limit.msc4335_info_url + ) + # Otherwise we use the current behaviour albeit not spec compliant + # See: https://github.com/element-hq/synapse/issues/18749 raise SynapseError( 400, "Media upload limit exceeded", Codes.RESOURCE_LIMIT_EXCEEDED ) diff --git a/synapse/rest/admin/experimental_features.py b/synapse/rest/admin/experimental_features.py index 3d3015cef77..e6063e9701c 100644 --- a/synapse/rest/admin/experimental_features.py +++ b/synapse/rest/admin/experimental_features.py @@ -44,6 +44,7 @@ class ExperimentalFeature(str, Enum): MSC3881 = "msc3881" MSC3575 = "msc3575" MSC4222 = "msc4222" + MSC4335 = "msc4335" def is_globally_enabled(self, config: "HomeServerConfig") -> bool: if self is ExperimentalFeature.MSC3881: @@ -52,6 +53,8 @@ def is_globally_enabled(self, config: "HomeServerConfig") -> bool: return config.experimental.msc3575_enabled if self is ExperimentalFeature.MSC4222: return config.experimental.msc4222_enabled + if self is ExperimentalFeature.MSC4335: + return config.experimental.msc4335_enabled assert_never(self) diff --git a/tests/rest/client/test_media.py b/tests/rest/client/test_media.py index 91bf94b672d..22e7bcdfb30 100644 --- a/tests/rest/client/test_media.py +++ b/tests/rest/client/test_media.py @@ -44,7 +44,7 @@ from twisted.web.iweb import UNKNOWN_LENGTH, IResponse from twisted.web.resource import Resource -from synapse.api.errors import HttpResponseException +from synapse.api.errors import Codes, HttpResponseException from synapse.api.ratelimiting import Ratelimiter from synapse.config._base import Config from synapse.config.oembed import OEmbedEndpointConfig @@ -2880,11 +2880,12 @@ def make_homeserver(self, reactor: MemoryReactor, clock: Clock) -> HomeServer: config["media_storage_providers"] = [provider_config] - # These are the limits that we are testing - config["media_upload_limits"] = [ - {"time_period": "1d", "max_size": "1K"}, - {"time_period": "1w", "max_size": "3K"}, - ] + # These are the limits that we are testing unless overridden + if config.get("media_upload_limits") is None: + config["media_upload_limits"] = [ + {"time_period": "1d", "max_size": "1K"}, + {"time_period": "1w", "max_size": "3K"}, + ] return self.setup_test_homeserver(config=config) @@ -3002,10 +3003,11 @@ def make_homeserver(self, reactor: MemoryReactor, clock: Clock) -> HomeServer: config["media_storage_providers"] = [provider_config] # default limits to use - config["media_upload_limits"] = [ - {"time_period": "1d", "max_size": "1K"}, - {"time_period": "1w", "max_size": "3K"}, - ] + if config.get("media_upload_limits") is None: + config["media_upload_limits"] = [ + {"time_period": "1d", "max_size": "1K"}, + {"time_period": "1w", "max_size": "3K"}, + ] return self.setup_test_homeserver(config=config) @@ -3158,3 +3160,70 @@ def test_uses_defaults(self) -> None: ) self.assertEqual(self.last_media_upload_limit_exceeded["sent_bytes"], 500) self.assertEqual(self.last_media_upload_limit_exceeded["attempted_bytes"], 800) + + @override_config( + { + "media_upload_limits": [ + { + "time_period": "1d", + "max_size": "1K", + "msc4335_info_url": "https://example.com", + }, + ] + } + ) + def test_msc4335_defaults_disabled(self) -> None: + """Test that the MSC4335 is not used unless experimental feature is enabled.""" + channel = self.upload_media(500, self.tok3) + self.assertEqual(channel.code, 200) + + channel = self.upload_media(800, self.tok3) + # n.b. this response is not spec compliant as described at: https://github.com/element-hq/synapse/issues/18749 + self.assertEqual(channel.code, 400) + self.assertEqual(channel.json_body["errcode"], Codes.RESOURCE_LIMIT_EXCEEDED) + + @override_config( + { + "experimental_features": {"msc4335_enabled": True}, + "media_upload_limits": [ + { + "time_period": "1d", + "max_size": "1K", + "msc4335_info_url": "https://example.com", + } + ], + } + ) + def test_msc4335_returns_user_limit_exceeded(self) -> None: + """Test that the MSC4335 error is returned when experimental feature is enabled.""" + channel = self.upload_media(500, self.tok3) + self.assertEqual(channel.code, 200) + + channel = self.upload_media(800, self.tok3) + self.assertEqual(channel.code, 403) + self.assertEqual(channel.json_body["errcode"], Codes.UNKNOWN) + self.assertEqual( + channel.json_body["org.matrix.msc4335.errcode"], "M_USER_LIMIT_EXCEEDED" + ) + self.assertEqual( + channel.json_body["org.matrix.msc4335.info_url"], "https://example.com" + ) + + @override_config( + { + "experimental_features": {"msc4335_enabled": True}, + "media_upload_limits": [ + { + "time_period": "1d", + "max_size": "1K", + } + ], + } + ) + def test_msc4335_requires_info_url(self) -> None: + """Test that the MSC4335 error is not used if info_url is not provided.""" + channel = self.upload_media(500, self.tok3) + self.assertEqual(channel.code, 200) + + channel = self.upload_media(800, self.tok3) + self.assertEqual(channel.code, 400) From 9ad30bcaa13e8842c9e801f9cf30af2f62322c61 Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Fri, 3 Oct 2025 11:03:58 +0100 Subject: [PATCH 2/4] Update to latest unstable codes --- synapse/api/errors.py | 10 +++- tests/rest/client/test_media.py | 91 ++++++++++++++++----------------- 2 files changed, 53 insertions(+), 48 deletions(-) diff --git a/synapse/api/errors.py b/synapse/api/errors.py index ec477ec8800..1ca20b4c27f 100644 --- a/synapse/api/errors.py +++ b/synapse/api/errors.py @@ -152,6 +152,8 @@ class Codes(str, Enum): # Part of MSC4326 UNKNOWN_DEVICE = "ORG.MATRIX.MSC4326.M_UNKNOWN_DEVICE" + MSC4335_USER_LIMIT_EXCEEDED = "ORG.MATRIX.MSC4335_USER_LIMIT_EXCEEDED" + class CodeMessageException(RuntimeError): """An exception with integer code, a message string attributes and optional headers. @@ -526,9 +528,13 @@ def __init__( ): additional_fields = { "org.matrix.msc4335.info_url": info_url, - "org.matrix.msc4335.errcode": "M_USER_LIMIT_EXCEEDED", } - super().__init__(code, msg, Codes.UNKNOWN, additional_fields=additional_fields) + super().__init__( + code, + msg, + Codes.MSC4335_USER_LIMIT_EXCEEDED, + additional_fields=additional_fields, + ) class EventSizeError(SynapseError): diff --git a/tests/rest/client/test_media.py b/tests/rest/client/test_media.py index 22e7bcdfb30..e5ba1482581 100644 --- a/tests/rest/client/test_media.py +++ b/tests/rest/client/test_media.py @@ -2971,6 +2971,51 @@ def test_over_weekly_limit(self) -> None: channel = self.upload_media(900) self.assertEqual(channel.code, 200) + @override_config( + { + "experimental_features": {"msc4335_enabled": True}, + "media_upload_limits": [ + { + "time_period": "1d", + "max_size": "1K", + "msc4335_info_url": "https://example.com", + } + ], + } + ) + def test_msc4335_returns_user_limit_exceeded(self) -> None: + """Test that the MSC4335 error is returned 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_url"], "https://example.com" + ) + + @override_config( + { + "experimental_features": {"msc4335_enabled": True}, + "media_upload_limits": [ + { + "time_period": "1d", + "max_size": "1K", + } + ], + } + ) + def test_msc4335_requires_info_url(self) -> None: + """Test that the MSC4335 error is not used if info_url is not provided.""" + channel = self.upload_media(500) + self.assertEqual(channel.code, 200) + + channel = self.upload_media(800) + self.assertEqual(channel.code, 400) + class MediaUploadLimitsModuleOverrides(unittest.HomeserverTestCase): """ @@ -3181,49 +3226,3 @@ def test_msc4335_defaults_disabled(self) -> None: # n.b. this response is not spec compliant as described at: https://github.com/element-hq/synapse/issues/18749 self.assertEqual(channel.code, 400) self.assertEqual(channel.json_body["errcode"], Codes.RESOURCE_LIMIT_EXCEEDED) - - @override_config( - { - "experimental_features": {"msc4335_enabled": True}, - "media_upload_limits": [ - { - "time_period": "1d", - "max_size": "1K", - "msc4335_info_url": "https://example.com", - } - ], - } - ) - def test_msc4335_returns_user_limit_exceeded(self) -> None: - """Test that the MSC4335 error is returned when experimental feature is enabled.""" - channel = self.upload_media(500, self.tok3) - self.assertEqual(channel.code, 200) - - channel = self.upload_media(800, self.tok3) - self.assertEqual(channel.code, 403) - self.assertEqual(channel.json_body["errcode"], Codes.UNKNOWN) - self.assertEqual( - channel.json_body["org.matrix.msc4335.errcode"], "M_USER_LIMIT_EXCEEDED" - ) - self.assertEqual( - channel.json_body["org.matrix.msc4335.info_url"], "https://example.com" - ) - - @override_config( - { - "experimental_features": {"msc4335_enabled": True}, - "media_upload_limits": [ - { - "time_period": "1d", - "max_size": "1K", - } - ], - } - ) - def test_msc4335_requires_info_url(self) -> None: - """Test that the MSC4335 error is not used if info_url is not provided.""" - channel = self.upload_media(500, self.tok3) - self.assertEqual(channel.code, 200) - - channel = self.upload_media(800, self.tok3) - self.assertEqual(channel.code, 400) From 459ede696680b277de8eeed06f8fc01b7ee90b22 Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Fri, 10 Oct 2025 11:51:12 +0100 Subject: [PATCH 3/4] 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 5b490859736..0e64f99282e 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 2d74fee1cbd..60340570ae4 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 1ca20b4c27f..b400149023b 100644 --- a/synapse/api/errors.py +++ b/synapse/api/errors.py @@ -524,11 +524,20 @@ def __init__( 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 a667733ed74..e3591a44cfa 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 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None: 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 e278308b1d9..04072505abd 100644 --- a/synapse/media/media_repository.py +++ b/synapse/media/media_repository.py @@ -382,13 +382,21 @@ async def create_or_update_content( 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 e5ba1482581..d5c26548496 100644 --- a/tests/rest/client/test_media.py +++ b/tests/rest/client/test_media.py @@ -46,7 +46,9 @@ 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.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,121 @@ def test_over_weekly_limit(self) -> None: 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}, + "media_upload_limits": [ + { + "time_period": "1d", + "max_size": "1K", + "msc4335_info_uri": "https://example.com", + "msc4335_soft_limit": False, + } + ], + } + ) + 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) + + 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"], False) + @override_config( { "experimental_features": {"msc4335_enabled": True}, @@ -2978,13 +3096,15 @@ def test_over_weekly_limit(self) -> None: { "time_period": "1d", "max_size": "1K", - "msc4335_info_url": "https://example.com", + "msc4335_info_uri": "https://example.com", + "msc4335_soft_limit": True, + "msc4335_increase_uri": "https://example.com/increase", } ], } ) - 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_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) @@ -2994,7 +3114,12 @@ def test_msc4335_returns_user_limit_exceeded(self) -> None: 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"], True) + self.assertEqual( + channel.json_body["org.matrix.msc4335.increase_uri"], + "https://example.com/increase", ) @override_config( @@ -3008,8 +3133,8 @@ def test_msc4335_returns_user_limit_exceeded(self) -> None: ], } ) - 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 @@ def test_uses_defaults(self) -> None: { "time_period": "1d", "max_size": "1K", - "msc4335_info_url": "https://example.com", + "msc4335_info_uri": "https://example.com", + "msc4335_soft_limit": False, }, ] } From 5c08e049858228f37f05315d6d203938e7da6315 Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Mon, 13 Oct 2025 11:50:50 +0100 Subject: [PATCH 4/4] Make soft_limit optional --- synapse/api/errors.py | 7 +++++-- tests/rest/client/test_media.py | 2 +- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/synapse/api/errors.py b/synapse/api/errors.py index b400149023b..caab6c4d6d1 100644 --- a/synapse/api/errors.py +++ b/synapse/api/errors.py @@ -525,7 +525,7 @@ def __init__( code: int, msg: str, info_uri: str, - soft_limit: bool, + soft_limit: bool = False, increase_uri: Optional[str] = None, ): if soft_limit and increase_uri is None: @@ -533,8 +533,11 @@ def __init__( additional_fields: dict[str, Union[str, bool]] = { "org.matrix.msc4335.info_uri": info_uri, - "org.matrix.msc4335.soft_limit": soft_limit, } + + if soft_limit: + additional_fields["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 diff --git a/tests/rest/client/test_media.py b/tests/rest/client/test_media.py index d5c26548496..51d03daeff9 100644 --- a/tests/rest/client/test_media.py +++ b/tests/rest/client/test_media.py @@ -3087,7 +3087,7 @@ def test_msc4335_returns_hard_user_limit_exceeded(self) -> None: self.assertEqual( channel.json_body["org.matrix.msc4335.info_uri"], "https://example.com" ) - self.assertEqual(channel.json_body["org.matrix.msc4335.soft_limit"], False) + self.assertEquals(hasattr(channel.json_body, "org.matrix.msc4335.increase_uri"), False) @override_config( {