Repository: crossbario/autobahn-python (language: python)

Issue:
Formatting bug in new release of Autobahn
From Tahoe-LAFS test suite:

```
Traceback (most recent call last):
  File "/home/itamarst/Devel/tahoe-lafs/venv/lib64/python3.7/site-packages/twisted/web/server.py", line 227, in process
    self.render(resrc)
  File "/home/itamarst/Devel/tahoe-lafs/venv/lib64/python3.7/site-packages/twisted/web/server.py", line 292, in render
    body = resrc.render(self)
  File "/home/itamarst/Devel/tahoe-lafs/venv/lib64/python3.7/site-packages/autobahn/twisted/resource.py", line 175, in render
    protocol.dataReceived(data)
  File "/home/itamarst/Devel/tahoe-lafs/venv/lib64/python3.7/site-packages/autobahn/twisted/websocket.py", line 348, in dataReceived
    self._dataReceived(data)
  File "/home/itamarst/Devel/tahoe-lafs/venv/lib64/python3.7/site-packages/autobahn/websocket/protocol.py", line 1243, in _dataReceived
    self.consumeData()
  File "/home/itamarst/Devel/tahoe-lafs/venv/lib64/python3.7/site-packages/autobahn/websocket/protocol.py", line 1272, in consumeData
    self.processHandshake()
  File "/home/itamarst/Devel/tahoe-lafs/venv/lib64/python3.7/site-packages/autobahn/websocket/protocol.py", line 2740, in processHandshake
    self.sendServerStatus()
  File "/home/itamarst/Devel/tahoe-lafs/venv/lib64/python3.7/site-packages/autobahn/websocket/protocol.py", line 3182, in sendServerStatus
    self.sendHtml(_SERVER_STATUS_TEMPLATE.format(redirect, __version__))
builtins.KeyError: '\n            color'
```

If you look at `_SERVER_STATUS_TEMPLATE`, you will see that it is both:

1. Using CSS styles with `{ }`
2. Using `{}` with Python's `.format()` method.

This combination doesn't work. The solution, I believe, is that CSS curly brackets should be quoted so `.format()` doesn't break it, e.g.

```css
         a, a:visited, a:hover {{
            color: #fff;
         }}
```


Discussion / hints:
thanks for letting us know! fwiw, the regression was introduced in v22.4. here https://github.com/crossbario/autobahn-python/pull/1547

before it was using plain old `%s` based escaping

https://github.com/crossbario/autobahn-python/blob/5890ca3cfcd1ee3dbe830add0e2a73ad82043ae9/autobahn/websocket/protocol.py#L2476
https://github.com/crossbario/autobahn-python/blob/5890ca3cfcd1ee3dbe830add0e2a73ad82043ae9/autobahn/websocket/protocol.py#L3147

however, now it should escape the curly braces as you hint ..
on a side note, even though we do run flake8 as part of CI, we don't run mypy (not for autobahn, but we run it for crossbar). however, while mypy catches a lot more than flake8, it wouldn't caught this one:

```
cpy39_1) (base) oberstet@intel-nuci7:~/scm/crossbario/autobahn-python$ mypy autobahn/websocket/protocol.py
autobahn/websocket/protocol.py:61: error: Skipping analyzing "txaio": module is installed, but missing library stubs or py.typed marker
autobahn/websocket/protocol.py:61: note: See https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-imports
autobahn/websocket/protocol.py:2564: error: "WebSocketServerProtocol" has no attribute "log"
Found 2 errors in 1 file (checked 1 source file)
```
> those 2 reported errors are bogus, this is all good ..


I bet pylint would catch it, you could setup pylint with approved list of lints starting with this one.
if I restarted again, one of the first things would be add CI for flake8, mypy and pylint, so the code grows 100% compliant from the beginning. well. out of curiosity, I tried pinning down the problem using pylint, but it doesn't identify the issue. pylint has a looot to complain rgd code style (and fwiw, sure, I agree .. mostly), but the only error it identifies is bogus:

```
(cpy39_1) (base) oberstet@intel-nuci7:~/scm/crossbario/autobahn-python$ pylint -d C0103,C0301,E1101,W0201,C0411,R1702,R1705,C0123,R0915,C0325,R0913,C0209,W0212,R0912,R0902,R0914,R0911,R0902,W0613,R1710,W0703,R0903,R0205,C0116,R0201,R1714,W0702,R1711,W0702,W0612,R1711,R0904,R1703,W0511,C0302,C0114,C0115,W0125   autobahn/websocket/protocol.py
************* Module autobahn.websocket.protocol
autobahn/websocket/protocol.py:990:48: E0203: Access to member 'trackTimings' before its definition line 991 (access-member-before-definition)

------------------------------------------------------------------
Your code has been rated at 9.97/10 (previous run: 9.97/10, +0.00)

(cpy39_1) (base) oberstet@intel-nuci7:~/scm/crossbario/autobahn-python$ pylint -E -d E1101 autobahn/websocket/protocol.py
************* Module autobahn.websocket.protocol
autobahn/websocket/protocol.py:990:48: E0203: Access to member 'trackTimings' before its definition line 991 (access-member-before-definition)
(cpy39_1) (base) oberstet@intel-nuci7:~/scm/crossbario/autobahn-python$ 
```

pylint doesn't understand the attribute access guard:

https://github.com/crossbario/autobahn-python/blob/2e2ee5f9775ed312db699f5c55fc0488311735a5/autobahn/websocket/protocol.py#L990


Fix patch:
diff --git a/autobahn/websocket/protocol.py b/autobahn/websocket/protocol.py
index e5cde9056..e12cc05b1 100755
--- a/autobahn/websocket/protocol.py
+++ b/autobahn/websocket/protocol.py
@@ -2499,16 +2499,16 @@ def prepareMessage(self, payload, isBinary=False, doNotCompress=False):
    <head>
       {}
       <style>
-         body {
+         body {{
             color: #fff;
             background-color: #027eae;
             font-family: "Segoe UI", "Lucida Grande", "Helvetica Neue", Helvetica, Arial, sans-serif;
             font-size: 16px;
-         }
+         }}
 
-         a, a:visited, a:hover {
+         a, a:visited, a:hover {{
             color: #fff;
-         }
+         }}
       </style>
    </head>
    <body>


Test patch:
diff --git a/autobahn/websocket/test/test_websocket_protocol.py b/autobahn/websocket/test/test_websocket_protocol.py
index 370f7ff7c..c644d7cd0 100644
--- a/autobahn/websocket/test/test_websocket_protocol.py
+++ b/autobahn/websocket/test/test_websocket_protocol.py
@@ -227,6 +227,12 @@ def test_sendClose_invalid_code_value(self):
         self.assertEqual(self.transport._written, b"")
         self.assertEqual(self.protocol.state, self.protocol.STATE_OPEN)
 
+    def test_interpolate_server_status_template(self):
+        from autobahn.websocket.protocol import _SERVER_STATUS_TEMPLATE
+        s = _SERVER_STATUS_TEMPLATE.format(None, '0.0.0')
+        self.assertEqual(type(s), str)
+        self.assertTrue(len(s) > 0)
+
 
 if os.environ.get('USE_TWISTED', False):
     class TwistedProtocolTests(unittest.TestCase):
@@ -270,3 +276,6 @@ def test_loseConnection(self):
             # that here
             self.protocol._connectionLost(txaio.create_failure(RuntimeError("testing")))
             self.assertTrue(self.protocol.openHandshakeTimeoutCall is None)
+
+        def test_send_server_status(self):
+            self.protocol.sendServerStatus()