Fix http/s proxy authentication with long username/passwords

[MagicRB]

Pull Request Checklist

• Pull request is based on the develop branch
• Pull request includes a changelog file. The entry should:
  • Be a short description of your change which makes sense to users. "Fixed a bug that prevented receiving messages from other servers." instead of "Moved X method from EventStore to EventWorkerStore.".
  • Use markdown where necessary, mostly for code blocks.
  • End with either a period (.) or an exclamation mark (!).
  • Start with a capital letter.
  • Feel free to credit yourself, by adding a sentence "Contributed by @github_username." or "Contributed by [Your Name]." to the end of the entry.
• Pull request includes a sign off
• Code style is correct
  (run the linters)

[MagicRB]

I'm not sure if the changelog change should be squashed with the actual change or not, please advise.

[clokep]

I'm not sure if the changelog change should be squashed with the actual change or not, please advise.

It doesn't matter, we squash merge anyway. So don't bother.

[MagicRB]

i will apply what the lint said and force push

[MagicRB]

Took me a while, sorry about that :) it's faster than trying to get the infra working on NixOS

[DMRobertson]

Introduced in #10475

[DMRobertson]

The following patch adds a test case which reproduces the issue. Could you apply it and include it as part of this PR?

From 5fe76b9434e22bb752c252dd9c66c3c2bfb90dfc Mon Sep 17 00:00:00 2001
From: David Robertson <[email protected]>
Date: Mon, 23 Oct 2023 19:21:23 +0100
Subject: [PATCH] Add test case to detect dodgy b64 encoding

---
 tests/http/test_proxyagent.py | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/tests/http/test_proxyagent.py b/tests/http/test_proxyagent.py
index 8164b0b78e..b48c2c293a 100644
--- a/tests/http/test_proxyagent.py
+++ b/tests/http/test_proxyagent.py
@@ -217,6 +217,20 @@ def test_parse_proxy(
         )
 
 
+class TestBasicProxyCredentials(TestCase):
+    def test_long_user_pass_string_encoded_without_newlines(self) -> None:
+        """Reproduces https://github.com/matrix-org/synapse/pull/16504."""
+        creds = BasicProxyCredentials(
+            b"looooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooonguser:[email protected]:9988"
+        )
+        auth_value = creds.as_proxy_authorization_value()
+        self.assertNotIn(b"\n", auth_value)
+        self.assertEqual(
+            creds.as_proxy_authorization_value(),
+            b"Basic: bG9vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vbmd1c2VyOnBhc3M=",
+        )
+
+
 class MatrixFederationAgentTests(TestCase):
     def setUp(self) -> None:
         self.reactor = ThreadedMemoryReactorClock()
-- 
2.41.0

[clokep]

Seems reasonable to me.

[DMRobertson]

Hmm. That test is failing and I don't know why.

[DMRobertson]

Oh, it's because I passed the entire auth string into the credentials constructor, rather than just the username:password. I'll fix.

[clokep]

Thank you! 🎉