Replace the raw ~uint32_t sentinel with a named constant
Download::default_resume_flags that masks out the open_enable_fallocate
bit. This prevents flag_fallocate from being set on all files when
open_throw() reads resume_flags() before explicit flags are configured.
The sentinel value (~uint32_t & ~open_enable_fallocate) retains the
full range as a 'not set' marker while being safe to pass through
Download::open() without unintended fallocate.
Use Download::hash_error_message() to get a descriptive error string when
hash check fails due to I/O error, instead of relying on errno which could
be zero.
Also fix Manager::receive_hashing_changed() where set_hash_failed(true) was
called without setting d.message when catching local_error during hash
check (e.g. 'too many open files').
Commit 6488131 ("Fix RPC/SCGI security and crash bugs by @sirus20x6")
replaced std::vector<std::unique_ptr<const char>> storage with
std::vector<std::string> and returned back().c_str() to the xmlrpc-c
registry as the per-method server_info pointer.
This is unsafe for any method name short enough to be SSO-stored
(<= 15 chars on libstdc++): such a string keeps its buffer inside the
std::string object itself. When a later push_back reallocates the
vector and move-constructs the existing elements into a new buffer,
the previously returned c_str() pointers — captured by xmlrpc-c at
registration time — dangle into freed memory.
Because xmlrpc-c does not dereference server_info until a call
dispatches, the failure surfaces later as nondeterministic garbage
in fault strings, e.g.
faultString: Command "thod." does not exist. (load.start, log.xmlrpc, log.execute)
faultString: Command "in_rate" does not exist. (log.add_output)
faultString: Command "" does not exist. (log.open_file)
faultString: Command "+U" does not exist. (method.set_key)
Long-named methods (e.g. system.client_version at 21 chars) are
heap-allocated above the SSO threshold and escape the bug because
the heap buffer's address is preserved across the vector move.
Switch the storage to std::deque<std::string>: per [deque.modifiers]
push_back does not invalidate references to existing elements, so
the std::string objects do not move and the c_str() pointers handed
to xmlrpc-c remain valid for the program's lifetime. The body of
store_command_name is unchanged.
Fixes the use-after-free; preserves the std::string-based storage
the original commit aimed for.
When an SCGI client closes the connection before rtorrent finishes sending
the response, send() in SCgiTask::event_write() returns -1 with errno EPIPE.
EPIPE was grouped with EAGAIN/EINTR as a non-fatal retry-later condition, so
the task was not closed and its descriptor stayed registered for EPOLLOUT. A
broken socket is reported writable immediately, so epoll_wait() returns it on
every iteration and the SCGI thread spins at 100% CPU on one core
indefinitely. The dead connection fd is also leaked (stays ESTAB).
EPIPE is terminal here, not retryable: the peer is gone and the response can
never be delivered. Close the task on EPIPE, matching event_read(), which
already closes on any recv() error other than EAGAIN/EINTR.
Reproduction: open the SCGI socket, send a complete RPC request, then
shutdown(SHUT_RDWR)/close before reading the reply. Stock: the rtorrent-scgi
thread goes to 100% CPU and the connection leaks. With this change: CPU stays
at 0% and the descriptor is closed.
- Add TORRENT_WITHOUT_NCURSES macro in scripts/checks.m4
- Clear CURSES_LIBS/CFLAGS in the macro instead of if/else in configure.ac
- Restore simple LIBS/CFLAGS lines in configure.ac
- Add missing set_escdelay stub
Add a dummy curses stub header that provides all ncurses types, macros,
and no-op function stubs. When configured with --without-ncurses, the
build uses this stub instead of linking to the real ncurses library.
This allows rtorrent to be built for daemon-only usage (e.g. with
ruTorrent or Flood) without requiring ncurses to be installed.
Closes#1613
detect_content_type() peeked at m_buffer[m_body] to infer JSON vs XML
when no CONTENT_TYPE header was provided. When the TCP header segment
arrives without any body bytes, m_body equals m_position and the peek
reads the null terminator padding byte — not the actual '{' or '[' —
causing JSON requests to be incorrectly classified as XML and fail.
Fix:
- Remove the body peek from detect_content_type(); defer it to after
the full body is confirmed present in event_read().
- Add a m_content_type_set flag to distinguish header-provided type
from auto-detected type.
Previously d.save_full_session was inside the 1_prepare handler for
event.download.inserted_new. Since 0.16.6 moved session saving to a
separate thread, save_full_download() snapshots the bencode synchronously
when called and queues the prebuilt streams for async write.
If user-registered handlers (e.g. seedingtime plugin's addtime setter
running d.set_custom=addtime) sort lexicographically AFTER 1_prepare, they
modify the bencode AFTER the snapshot has already been taken. The first
on-disk .rtorrent then lacks those custom fields. The next periodic
resume save catches up, but if rtorrent restarts before that, the data
is lost — manifesting as blank Finished/SeedingTime columns in ruTorrent.
Fix: split the inserted_new key into two — 1_prepare keeps view
visibility setup, ~_save_full runs d.save_full_session last (~ prefix
is ASCII 0x7E, sorts after all alphanumerics, matching the existing
~_delete_tied precedent on event.download.erased). The snapshot then
includes any custom fields written by user handlers.
SCgiTask objects are pre-allocated in a pool (scgi.cc) and reused across
SCGI connections. SCgiTask::open() did not reset m_trusted, so when a
task that had handled an untrusted connection (m_trusted=false) was
reused for a new connection, m_trusted stayed false unless the new
connection explicitly sent UNTRUSTED_CONNECTION=1.
The header parser only set m_trusted=false on value 1 and was a no-op
on value 0 (the comment said "default is trusted, so do nothing") —
which is wrong for a reused task that is no longer in default state.
This caused intermittent rejection of trusted commands (e.g. ruTorrent
calling execute.capture for UID detection) with "Command X is not allowed
for untrusted connections", producing cascading plugin failures and
"ruTorrent cannot determine the UID of rTorrent user" in the web UI.
Fix:
- SCgiTask::open() resets m_trusted=true to default.
- parse_headers explicitly sets m_trusted=true on UNTRUSTED_CONNECTION=0,
so the value sent on the wire is authoritative regardless of pool
reuse semantics.
Verified on gb4 with rtorrent 0.16.11 + this fix: 30/30 trusted calls
succeed, 30/30 untrusted correctly blocked, 30/30 trusted-after-untrusted
batch all succeed (previously 70%+ would fail in the same scenario).
Jari Sundell