add missing shutdown_request (#1770)

This patch fixes the error handling of cases where there are runtime errors in `socketserver`.
For example, when creating a new thread (to handle a request) fails.

`get_request` handles ssl connections by replacing the new client socket by a tuple containing `(server_socket, new_client_socket)` in order to later deal with handshakes in `process_request_thread`

During the processing of a request, the socketserver `BaseServer` calls `handle_request`, calling the `_handle_request_noblock`, which is calling the following functions (https://github.com/python/cpython/blob/3.8/Lib/socketserver.py#L303):

```
request, client_addr = get_request()
verify_request(request, client_address):
process_request(request, client_address)
handle_error(request, client_address)
shutdown_request(request)
```

- `get_request` is overloaded in patroni and returns `request` as a tuple in case of ssl calls
- `verify_request` defaults to `return True` and should be fixed if used but is fine in this case
- `process_request` just calls `process_request_thread` (which is overloaded in patroni and handles tuple-style requests)
- `handle_error` is overloaded in patroni and handles tuple-style requests)
- but `shutdown_request` is not overloaded and thus missing support for tuple-style requests

This patch adds support for tuple-style requests in patroni api
This commit is contained in:
Nicolas Limage
2020-11-27 19:25:06 +01:00
committed by GitHub
parent d7f579ee61
commit e2b2daf0b3
2 changed files with 18 additions and 0 deletions
+5
View File
@@ -664,6 +664,11 @@ class RestApiServer(ThreadingMixIn, HTTPServer, Thread):
newsock = (sock, newsock)
return newsock, addr
def shutdown_request(self, request):
if isinstance(request, tuple):
_, request = request # SSLSocket
return super(RestApiServer, self).shutdown_request(request)
def reload_config(self, config):
if 'listen' not in config: # changing config in runtime
raise ValueError('Can not find "restapi.listen" config')
+13
View File
@@ -488,3 +488,16 @@ class TestRestApiServer(unittest.TestCase):
mock_accept.return_value = (newsock, '2')
self.srv.socket = Mock()
self.assertEqual(self.srv.get_request(), ((self.srv.socket, newsock), '2'))
@patch.object(MockRestApiServer, 'process_request', Mock(side_effect=RuntimeError))
def test_process_request_error(self):
mock_address = ('127.0.0.1', 55555)
mock_socket = Mock()
mock_ssl_socket = (Mock(), Mock())
for mock_request in (mock_socket, mock_ssl_socket):
with patch.object(
MockRestApiServer,
'get_request',
Mock(return_value=(mock_request, mock_address))
):
self.srv._handle_request_noblock()