Partially revert "Reject setting null profile field values" (#20422)

As per https://github.com/element-hq/synapse/issues/20419 this change
broke Element Web clearing user status, which basically means the client
is stuck with any status, and users cannot set a new one.

EW should move to use DELETE to clear user status, but until then we
should ensure the feature is not broken on existing clients in the wild.

Should we set a clear deprecation time for rejecting nulls? There might
be other clients using profile fields and doing PUT to clear the field.


Discussed in
[`#element-backend-internal:matrix.org`](https://matrix.to/#/!SGNQGPGUwtcPBUotTL:matrix.org/$b7zid3lLGdFDC8pwy1IvSPv3zl9y1fXEpnnBte9vcQU?via=jki.re&via=element.io&via=matrix.org)
[[1]](https://matrix.to/#/!SGNQGPGUwtcPBUotTL:matrix.org/$jTEcHlDBIvKRAKtV-ESXabkPkLia9B7C4JQVkoOxzj0?via=jki.re&via=element.io&via=matrix.org)

Reverts a part of https://github.com/element-hq/synapse/pull/20153 by
only rejecting null fields when the profile update stream is configured
(ie user status requirement).

Future work to unrevert tracked by
https://github.com/element-hq/synapse/issues/20424

changelog.d/20422.bugfix

@@ -0,0 +1,4 @@
+Fix clearing user status for clients that send a `null` profile field value.
+
+When `include_profile_updates_in_sync` is enabled, Synapse will continue to accept `null` custom profile field values
+until clients have had a sufficient time to change their behaviour. This special casing is tracked in [this issue](https://github.com/element-hq/synapse/issues/20424).

synapse/rest/client/profile.py

@@ -138,6 +138,9 @@ class ProfileFieldRestServlet(RestServlet):
                     r"^/_matrix/client/unstable/uk\.tcpip\.msc4133/profile/(?P<user_id>[^/]*)/(?P<field_name>[^/]*)$"
                 )
             )
+        self._include_profile_updates_in_sync = (
+            hs.config.server.include_profile_updates_in_sync
+        )
 
     async def on_GET(
         self, request: SynapseRequest, user_id: str, field_name: str
@@ -232,13 +235,20 @@ class ProfileFieldRestServlet(RestServlet):
                 errcode=Codes.MISSING_PARAM,
             )
 
-        if new_value is None:
+        if new_value is None and not self._include_profile_updates_in_sync:
             # > Servers MAY reject null values.
             # — https://spec.matrix.org/v1.19/client-server-api/#put_matrixclientv3profileuseridkeyname
             #
             # Although we can safely handle nulls internally, for interoperability reasons it is preferable if we
             # reject them.
             # That way, clients don't accidentally rely on being able to send them.
+            #
+            # The exception is if we have the profile update stream enabled, ie send
+            # down profile updates via sync streams, we will still accept null values
+            # for some time, as Element Web depends on removing User Status via
+            # a PUT and `null` value.
+            # https://github.com/element-hq/synapse/issues/20424 tracks removing
+            # this special case once enough clients have updated.
             raise SynapseError(
                 HTTPStatus.BAD_REQUEST,
                 f"'{field_name}' cannot be `null`.",

tests/rest/client/test_profile.py

@@ -40,6 +40,7 @@ from synapse.types import UserID
 from synapse.util.clock import Clock
 
 from tests import unittest
+from tests.unittest import override_config
 from tests.utils import USE_POSTGRES_FOR_TESTS
 
 
@@ -722,6 +723,24 @@ class ProfileTestCase(unittest.HomeserverTestCase):
         self.assertEqual(channel.code, HTTPStatus.BAD_REQUEST, channel.result)
         self.assertEqual(channel.json_body["errcode"], Codes.INVALID_PARAM)
 
+    @override_config({"include_profile_updates_in_sync": True})
+    def test_null_field_is_not_rejected_if_profile_update_stream_enabled(self) -> None:
+        """
+        Tests that setting a field to `null` is not rejected if profile update
+        stream is configured active.
+
+        See https://github.com/element-hq/synapse/issues/20424 for removing
+        this special case.
+        """
+        channel = self.make_request(
+            "PUT",
+            f"/_matrix/client/v3/profile/{self.owner}/org.example.custom_field",
+            content={"org.example.custom_field": None},
+            access_token=self.owner_tok,
+        )
+        self.assertEqual(channel.code, HTTPStatus.OK, channel.result)
+        self.assertFalse("errcode" in channel.json_body.keys())
+
     def test_set_custom_field_noauth(self) -> None:
         channel = self.make_request(
             "PUT",