Update for MSC4335 landing in the spec

Author: hughns

changelog.d/18876.feature

@@ -1 +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.
+Return M_USER_LIMIT_EXCEEDED error code for media upload limits from [MSC4335](https://github.com/matrix-org/matrix-spec-proposals/pull/4335).

docs/usage/configuration/config_documentation.md

@@ -2215,19 +2215,20 @@ 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_uri` (string): Experimental MSC4335 URI to where the user can find information about the upload limit. Optional.
+* `info_uri` (string): URI return to the client for where the user can find information about the upload limit. Optional. If not set then a static `data:text/html` URI is returned with a simple message. It is recommended to provide an `info_uri` that points to a page with more information about the upload limit and how users can reduce their upload usage or request an upload limit increase.
 
-* `msc4335_can_upgrade` (boolean): Experimental MSC4335 value to say if the limit can be increased. Optional.
+* `can_upgrade` (boolean): Value returned to the client for whether the limit can be increased. Optional default `false`. Defaults to `false`.
 
 Example configuration:
 ```yaml
 media_upload_limits:
 - time_period: 1h
   max_size: 100M
+  info_uri: https://example.com/quota#hour
 - time_period: 1w
   max_size: 500M
-  msc4335_info_uri: https://example.com/quota
-  msc4335_can_upgrade: true
+  info_uri: https://example.com/quota
+  can_upgrade: true
 ```
 ---
 ### `max_image_pixels`

schema/synapse-config.schema.yaml

@@ -2475,21 +2475,29 @@ properties:
           description: >-
             Amount of data that can be uploaded in the time period by the user.
             Required.
-        msc4335_info_uri:
+        info_uri:
           type: string
           description: >-
-            Experimental MSC4335 URI to where the user can find information about the upload limit. Optional.
-        msc4335_can_upgrade:
+            URI return to the client for where the user can find information
+            about the upload limit. Optional.
+            If not set then a static `data:text/html` URI is returned with a
+            simple message.
+            It is recommended to provide an `info_uri` that points to a page
+            with more information about the upload limit and how users can
+            reduce their upload usage or request an upload limit increase.
+        can_upgrade:
           type: boolean
           description: >-
-            Experimental MSC4335 value to say if the limit can be increased. Optional.
+            Value returned to the client for whether the limit can be increased. Optional default `false`.
+          default: false
     examples:
       - - time_period: 1h
           max_size: 100M
+          info_uri: https://example.com/quota#hour
         - time_period: 1w
           max_size: 500M
-          msc4335_info_uri: https://example.com/quota
-          msc4335_can_upgrade: true
+          info_uri: https://example.com/quota
+          can_upgrade: true
   max_image_pixels:
     $ref: "#/$defs/bytes"
     description: Maximum number of pixels that will be thumbnailed.

synapse/api/errors.py

@@ -152,7 +152,7 @@ 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"
+    USER_LIMIT_EXCEEDED = "M_USER_LIMIT_EXCEEDED"
 
 
 class CodeMessageException(RuntimeError):
@@ -515,9 +515,9 @@ def error_dict(self, config: Optional["HomeServerConfig"]) -> "JsonDict":
         )
 
 
-class MSC4335UserLimitExceededError(SynapseError):
+class UserLimitExceededError(SynapseError):
     """
-    Experimental implementation of MSC4335 M_USER_LIMIT_EXCEEDED error
+    Implementation of M_USER_LIMIT_EXCEEDED error
     """
 
     def __init__(
@@ -528,16 +528,16 @@ def __init__(
         can_upgrade: bool = False,
     ):
         additional_fields: dict[str, Union[str, bool]] = {
-            "org.matrix.msc4335.info_uri": info_uri,
+            "info_uri": info_uri,
         }
 
         if can_upgrade:
-            additional_fields["org.matrix.msc4335.can_upgrade"] = can_upgrade
+            additional_fields["can_upgrade"] = can_upgrade
 
         super().__init__(
             code,
             msg,
-            Codes.MSC4335_USER_LIMIT_EXCEEDED,
+            Codes.USER_LIMIT_EXCEEDED,
             additional_fields=additional_fields,
         )
 

synapse/config/repository.py

@@ -21,7 +21,7 @@
 
 import logging
 import os
-from typing import Any, Optional
+from typing import Any
 
 import attr
 
@@ -134,11 +134,11 @@ class MediaUploadLimit:
     time_period_ms: int
     """The time period in milliseconds."""
 
-    msc4335_info_uri: Optional[str] = None
-    """Used for experimental MSC4335 error code feature"""
+    info_uri: str
+    """The URI to return with the M_USER_LIMIT_EXCEEDED error."""
 
-    msc4335_can_upgrade: Optional[bool] = None
-    """Used for experimental MSC4335 error code feature"""
+    can_upgrade: bool = False
+    """Whether the user can upgrade their plan to increase the limit. This is returned in the M_USER_LIMIT_EXCEEDED error."""
 
 
 class ContentRepositoryConfig(Config):
@@ -314,23 +314,21 @@ 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_uri = limit_config.get("msc4335_info_uri", None)
-            msc4335_can_upgrade = limit_config.get("msc4335_can_upgrade", None)
+            info_uri = limit_config.get("info_uri", "")
+            can_upgrade = bool(limit_config.get("can_upgrade", False))
 
-            if (msc4335_info_uri is not None or msc4335_can_upgrade is not None) and (
-                not (msc4335_info_uri and msc4335_can_upgrade is not None)
-            ):
-                raise ConfigError(
-                    "If any of msc4335_info_uri or msc4335_can_upgrade are set, then both msc4335_info_uri and "
-                    "msc4335_can_upgrade must be set."
+            if info_uri == "":
+                logger.warning(
+                    "Empty info_uri provided for media upload limit, using static fallback value instead. You should specify an info_uri that points to more information about the upload limits imposed."
                 )
+                info_uri = "data:text/html,<p>You have exceeded a media upload limit. Ask your server administrator for more information.</p>"
 
             self.media_upload_limits.append(
                 MediaUploadLimit(
                     max_bytes,
                     time_period_ms,
-                    msc4335_info_uri,
-                    msc4335_can_upgrade,
+                    info_uri,
+                    can_upgrade,
                 )
             )
 

synapse/media/media_repository.py

@@ -37,10 +37,10 @@
     Codes,
     FederationDeniedError,
     HttpResponseException,
-    MSC4335UserLimitExceededError,
     NotFoundError,
     RequestSendFailed,
     SynapseError,
+    UserLimitExceededError,
     cs_error,
 )
 from synapse.api.ratelimiting import Ratelimiter
@@ -71,7 +71,6 @@
 )
 from synapse.media.thumbnailer import Thumbnailer, ThumbnailError
 from synapse.media.url_previewer import UrlPreviewer
-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
@@ -398,26 +397,12 @@ 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_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_uri
-                    and limit.msc4335_can_upgrade is not None
-                ):
-                    raise MSC4335UserLimitExceededError(
-                        403,
-                        "Media upload limit exceeded",
-                        limit.msc4335_info_uri,
-                        limit.msc4335_can_upgrade,
-                    )
-                # 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
+
+                raise UserLimitExceededError(
+                    403,
+                    "Media upload limit exceeded",
+                    limit.info_uri,
+                    limit.can_upgrade,
                 )
 
         if is_new_media:

synapse/module_api/callbacks/media_repository_callbacks.py

@@ -150,7 +150,10 @@ async def on_media_upload_limit_exceeded(
             ):
                 # Use a copy of the data in case the module modifies it
                 limit_copy = MediaUploadLimit(
-                    max_bytes=limit.max_bytes, time_period_ms=limit.time_period_ms
+                    max_bytes=limit.max_bytes,
+                    time_period_ms=limit.time_period_ms,
+                    info_uri=limit.info_uri,
+                    can_upgrade=limit.can_upgrade,
                 )
                 await delay_cancellation(
                     callback(user_id, limit_copy, sent_bytes, attempted_bytes)

synapse/rest/admin/experimental_features.py

@@ -44,7 +44,6 @@ 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:
@@ -53,8 +52,6 @@ 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)