search

qutebrowser / qutebrowser

A keyboard-driven, vim-like browser based on Python and Qt.
https://www.qutebrowser.org/
GNU General Public License v3.0
9.75k stars 1.01k forks source link

AssertionError with invalid data sent to IPC socket #6817

Open The-Compiler opened 3 years ago

The-Compiler commented 3 years ago

As reported by @foxxx0:

passed an URL with a trailing newline to the ipc socket

Traceback (most recent call last):
  File "/usr/lib/python3.9/site-packages/qutebrowser/misc/ipc.py", line 322, in _handle_data
    json_data = json.loads(decoded)
  File "/usr/lib/python3.9/json/__init__.py", line 346, in loads
    return _default_decoder.decode(s)
  File "/usr/lib/python3.9/json/decoder.py", line 340, in decode
    raise JSONDecodeError("Extra data", s, end)
json.decoder.JSONDecodeError: Extra data: line 1 column 6 (char 5)

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/usr/lib/python3.9/site-packages/qutebrowser/misc/ipc.py", line 394, in on_ready_read
    self._handle_data(data)
  File "/usr/lib/python3.9/site-packages/qutebrowser/misc/ipc.py", line 325, in _handle_data
    self._handle_invalid_data()
  File "/usr/lib/python3.9/site-packages/qutebrowser/misc/ipc.py", line 304, in _handle_invalid_data
    assert self._socket is not None
AssertionError

on qutebrowser v2.4.0 with Qt 5.15.2 on Archlinux.

Report (private): https://crashes.qutebrowser.org/view/b1d0d28e

foxxx0 commented 3 years ago

After taking a quick look into misc/ipc.py it seems to me that the client is maybe getting prematurely disconnected from the socket before handling all of their (broken) data:

12:12:33 ERROR    ipc        ipc:_handle_invalid_data:305 Ignoring invalid IPC data from socket 0x7f3b04179790.
12:12:33 DEBUG    ipc        ipc:on_disconnected:292 Client disconnected from socket 0x7f3b04179790.
12:12:33 DEBUG    ipc        ipc:handle_connection:267 No new connection to handle.
12:12:33 DEBUG    ipc        ipc:_get_socket:372 In _get_socket with None socket!
12:12:33 DEBUG    ipc        ipc:on_ready_read:392 Read from socket 0x7f3b04179790: b'"], "target_arg": null, "version": "2.4.0", "protocol_version": 1, "cwd": "/tmp/qb_temp"}\n'
12:12:33 DEBUG    ipc        ipc:_handle_data:320 Processing: "], "target_arg": null, "version": "2.4.0", "protocol_version": 1, "cwd": "/tmp/qb_temp"}
12:12:33 ERROR    ipc        ipc:_handle_data:324 invalid json: "], "target_arg": null, "version": "2.4.0", "protocol_version": 1, "cwd": "/tmp/qb_temp"}
12:12:33 ERROR    misc       crashsignal:_handle_early_exits:240 Uncaught exception

The json.decoder.JSONDecodeError is misleading here as the original exception initiator was the assertion in _handle_invalid_data(). During the first _handle_invalid_data() the client will get disconnected and that sets self._socket to None. Now there is still data in the ipc socket ready to be read but no client connected anymore, causing the assertion error.

Not sure how you want to handle this, I would suggest checking for any leftover data in _handle_invalid_data() or on_disconnected() and purge that with a warning/error to prevent reading it later.

(Just for clarification: json.decoder.JSONDecodeError actually inherits from ValueError and that is already being caught.)