Bug: read-only flag is set on KRPC responses
Summary
KrpcSocket::response_message() currently sets the BEP 43 read-only flag on
responses when the local socket is not in server mode:
read_only: !self.server_mode,
This is suspicious and likely incorrect. BEP 43 defines ro as a flag that a
read-only DHT node adds to outgoing query messages so responders know not to add
the requester to their routing tables. It does not define ro as a response
field.
Why this is wrong
BEP 43 says:
In each outgoing query message the read-only DHT node places a ro key in the
top-level message dictionary and sets its value to 1.
Spec link:
This repository already documents the same assumption in Message:
src/common/messages.rs: read_only is described as a BEP 43 request flag.
- The comment says it "should only be set on requests" and that behavior is
undefined when set on a response.
However, KrpcSocket::response_message() currently sets read_only in
responses:
src/rpc/socket.rs: response_message() sets read_only: !self.server_mode.
That means a read-only local node can send response packets with ro=1, even
though the flag is only meaningful for query/request packets.
Expected behavior
ro=1 should be emitted only on outgoing KRPC query/request messages when the
local node is read-only.
KRPC responses should not set the BEP 43 read-only flag. In practice,
KrpcSocket::response_message() should probably set:
Proposed fix
- Change
KrpcSocket::response_message() to always set read_only: false.
- Keep
KrpcSocket::request_message() setting read_only: !self.server_mode.
- Add a regression test that a read-only socket sends
ro=1 on requests but
not on responses.
- Consider removing or narrowing response-side
read_only handling in
Rpc::handle_response(), because valid responses should not use that flag.
Bug: read-only flag is set on KRPC responses
Summary
KrpcSocket::response_message()currently sets the BEP 43 read-only flag onresponses when the local socket is not in server mode:
This is suspicious and likely incorrect. BEP 43 defines
roas a flag that aread-only DHT node adds to outgoing query messages so responders know not to add
the requester to their routing tables. It does not define
roas a responsefield.
Why this is wrong
BEP 43 says:
Spec link:
This repository already documents the same assumption in
Message:src/common/messages.rs:read_onlyis described as a BEP 43 request flag.undefined when set on a response.
However,
KrpcSocket::response_message()currently setsread_onlyinresponses:
src/rpc/socket.rs:response_message()setsread_only: !self.server_mode.That means a read-only local node can send response packets with
ro=1, eventhough the flag is only meaningful for query/request packets.
Expected behavior
ro=1should be emitted only on outgoing KRPC query/request messages when thelocal node is read-only.
KRPC responses should not set the BEP 43 read-only flag. In practice,
KrpcSocket::response_message()should probably set:Proposed fix
KrpcSocket::response_message()to always setread_only: false.KrpcSocket::request_message()settingread_only: !self.server_mode.ro=1on requests butnot on responses.
read_onlyhandling inRpc::handle_response(), because valid responses should not use that flag.