Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions lib/app/app.dart
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import 'package:rutorrentflutter/services/api/http_client_service.dart';
import 'package:rutorrentflutter/services/api/i_api_service.dart';
import 'package:rutorrentflutter/services/api/prod_api_service.dart';
import 'package:rutorrentflutter/services/functional_services/authentication_service.dart';
import 'package:rutorrentflutter/services/functional_services/discovery_service.dart';
import 'package:rutorrentflutter/services/functional_services/disk_space_service.dart';
import 'package:rutorrentflutter/services/functional_services/internet_service.dart';
import 'package:rutorrentflutter/services/functional_services/notification_service.dart';
Expand Down Expand Up @@ -71,6 +72,7 @@ import '../ui/views/splash/splash_view.dart';
LazySingleton(classType: FilePickerService),
LazySingleton(classType: TorrentService),
LazySingleton(classType: PackageInfoService),
LazySingleton(classType: DiscoveryService),
LazySingleton(classType: DiskFileService),
],
)
Expand Down
2 changes: 2 additions & 0 deletions lib/app/app.locator.dart

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

293 changes: 293 additions & 0 deletions lib/services/functional_services/discovery_service.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,293 @@
import 'dart:async';
import 'dart:io';
import 'package:flutter/foundation.dart';
import 'package:logger/logger.dart';
import 'package:rutorrentflutter/app/app.logger.dart';

Logger _log = getLogger("DiscoveryService");

/// Represents a discovered ruTorrent server on the local network.
class DiscoveredServer {
final String host;
final int port;
final String protocol;
final String? name;

DiscoveredServer({
required this.host,
required this.port,
this.protocol = 'http',
this.name,
});

/// Constructs the full URL for connecting to this server.
String get url {
if (port == 80 && protocol == 'http') {
return '$protocol://$host';
} else if (port == 443 && protocol == 'https') {
return '$protocol://$host';
}
return '$protocol://$host:$port';
}

@override
String toString() => name != null ? '$name ($url)' : url;
}

/// Service for discovering ruTorrent servers on the local network
/// using mDNS/DNS-SD (Zeroconf).
///
/// This service broadcasts mDNS queries for the `_rutorrent_mobile._tcp.local`
/// service type and listens for responses from servers that have been
/// configured to advertise via Avahi or similar mDNS responders.
///
/// ## Server Configuration
/// To make a ruTorrent server discoverable, users need to create an Avahi
/// service file on their server. Example `/etc/avahi/services/rutorrent.service`:
/// ```xml
/// <?xml version="1.0" standalone='no'?>
/// <!DOCTYPE service-group SYSTEM "avahi-service.dtd">
/// <service-group>
/// <name>My ruTorrent Server</name>
/// <service>
/// <type>_rutorrent_mobile._tcp</type>
/// <port>443</port>
/// <txt-record>protocol=https</txt-record>
/// </service>
/// </service-group>
/// ```
class DiscoveryService {
static const String _serviceType = '_rutorrent_mobile._tcp.local';
static const int _mdnsPort = 5353;
static const String _mdnsAddress = '224.0.0.251';
static const Duration _discoveryTimeout = Duration(seconds: 5);

final ValueNotifier<List<DiscoveredServer>> discoveredServers =
ValueNotifier([]);
final ValueNotifier<bool> isDiscovering = ValueNotifier(false);

/// Starts discovering ruTorrent servers on the local network.
///
/// Sends an mDNS query for the `_rutorrent_mobile._tcp.local` service type
/// and listens for responses for [timeout] duration (default 5 seconds).
///
/// Returns a list of discovered servers.
Future<List<DiscoveredServer>> discover({Duration? timeout}) async {
final effectiveTimeout = timeout ?? _discoveryTimeout;
isDiscovering.value = true;
discoveredServers.value = [];

_log.i('Starting mDNS discovery for $_serviceType');

try {
// Bind to the mDNS multicast address
final socket = await RawDatagramSocket.bind(
InternetAddress.anyIPv4,
0, // Let OS pick a port
);

// Join the multicast group
socket.joinMulticast(InternetAddress(_mdnsAddress));
socket.broadcastEnabled = true;
socket.readEventsEnabled = true;

// Build and send the mDNS query
final query = _buildMdnsQuery(_serviceType);
socket.send(
query,
InternetAddress(_mdnsAddress),
_mdnsPort,
);

_log.i('mDNS query sent, listening for responses...');

// Collect responses
final List<DiscoveredServer> servers = [];
final completer = Completer<List<DiscoveredServer>>();

// Set up timeout
Timer(effectiveTimeout, () {
if (!completer.isCompleted) {
socket.close();
completer.complete(servers);
}
});

// Listen for responses
socket.listen((event) {
if (event == RawSocketEvent.read) {
final datagram = socket.receive();
if (datagram != null) {
try {
final server = _parseMdnsResponse(datagram);
if (server != null &&
!servers.any((s) => s.host == server.host && s.port == server.port)) {
servers.add(server);
_log.i('Discovered server: ${server.url}');
discoveredServers.value = List.from(servers);
}
} catch (e) {
_log.w('Error parsing mDNS response: $e');
}
}
}
});
Comment on lines +109 to +134

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Socket subscription not explicitly cancelled; potential resource leak on repeated calls.

The socket.listen() subscription is not stored or cancelled when the timeout fires. While socket.close() should eventually trigger the stream to end, explicitly cancelling the subscription is safer and clearer. Additionally, if discover() is called while a previous discovery is still in progress, both will run concurrently with conflicting state updates.

🛠️ Proposed fix: Store and cancel subscription, guard against concurrent calls
  Future<List<DiscoveredServer>> discover({Duration? timeout}) async {
    final effectiveTimeout = timeout ?? _discoveryTimeout;
+   
+   // Prevent concurrent discovery runs
+   if (isDiscovering.value) {
+     _log.w('Discovery already in progress');
+     return discoveredServers.value;
+   }
+   
    isDiscovering.value = true;
    discoveredServers.value = [];

And for the listener:

      // Listen for responses
-     socket.listen((event) {
+     final subscription = socket.listen((event) {
        if (event == RawSocketEvent.read) {
          // ... existing code ...
        }
      });

      // Set up timeout
      Timer(effectiveTimeout, () {
        if (!completer.isCompleted) {
+         subscription.cancel();
          socket.close();
          completer.complete(servers);
        }
      });
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/services/functional_services/discovery_service.dart` around lines 109 -
134, The socket.listen subscription in discover() (where Timer(effectiveTimeout,
...), socket.listen(...), completer, and discoveredServers are used) must be
stored and explicitly cancelled to avoid resource leaks and conflicting
concurrent runs: create a StreamSubscription variable for the listen call and
call subscription.cancel() both inside the timeout handler and when the
completer completes (after socket.close()), and add a simple concurrency guard
(e.g., an _isDiscovering flag or checking an existing non-completed completer)
at the start of discover() to prevent starting a new discovery while one is in
progress; ensure you still close the socket and complete the completer after
cancelling the subscription and update discoveredServers as before.


final result = await completer.future;

_log.i('Discovery complete. Found ${result.length} server(s).');

discoveredServers.value = result;
isDiscovering.value = false;
return result;
} catch (e) {
_log.e('Discovery failed: $e');
isDiscovering.value = false;
return [];
}
}

/// Builds a minimal mDNS query packet for the given service type.
List<int> _buildMdnsQuery(String name) {
final List<int> packet = [];

// Transaction ID (2 bytes)
packet.addAll([0x00, 0x00]);
// Flags: standard query (2 bytes)
packet.addAll([0x00, 0x00]);
// Questions: 1 (2 bytes)
packet.addAll([0x00, 0x01]);
// Answer RRs: 0 (2 bytes)
packet.addAll([0x00, 0x00]);
// Authority RRs: 0 (2 bytes)
packet.addAll([0x00, 0x00]);
// Additional RRs: 0 (2 bytes)
packet.addAll([0x00, 0x00]);

// Query name
final labels = name.split('.');
for (final label in labels) {
packet.add(label.length);
packet.addAll(label.codeUnits);
}
packet.add(0x00); // Null terminator

// Type: PTR (12)
packet.addAll([0x00, 0x0C]);
// Class: IN (1) with unicast-response bit
packet.addAll([0x00, 0x01]);

return packet;
}

/// Attempts to parse a discovered server from an mDNS response packet.
/// Returns null if the response doesn't contain valid server information.
DiscoveredServer? _parseMdnsResponse(Datagram datagram) {
final data = datagram.data;

// Basic validation - minimum DNS header size
if (data.length < 12) return null;

// Check if this is a response (bit 15 of flags should be 1)
if ((data[2] & 0x80) == 0) return null;

// Use the sender's address as the host
final host = datagram.address.address;
int port = 80;
String protocol = 'http';
String? name;

// Try to extract SRV record (port) and TXT record (protocol)
// by scanning the response data
int offset = 12;
try {
// Skip questions section
final qdCount = (data[4] << 8) | data[5];
for (int i = 0; i < qdCount && offset < data.length; i++) {
offset = _skipName(data, offset);
offset += 4; // Skip QTYPE and QCLASS
}

// Parse answer/additional sections for SRV and TXT records
final anCount = (data[6] << 8) | data[7];
final nsCount = (data[8] << 8) | data[9];
final arCount = (data[10] << 8) | data[11];
final totalRecords = anCount + nsCount + arCount;

for (int i = 0; i < totalRecords && offset < data.length - 2; i++) {
offset = _skipName(data, offset);
if (offset + 10 > data.length) break;

final type = (data[offset] << 8) | data[offset + 1];
final rdLength = (data[offset + 8] << 8) | data[offset + 9];
offset += 10;

if (offset + rdLength > data.length) break;

if (type == 33 && rdLength >= 6) {
// SRV record
port = (data[offset + 4] << 8) | data[offset + 5];
} else if (type == 16) {
// TXT record
final txtData = String.fromCharCodes(
data.sublist(offset, offset + rdLength));
if (txtData.contains('protocol=https')) {
protocol = 'https';
} else if (txtData.contains('protocol=http')) {
protocol = 'http';
}
} else if (type == 12) {
// PTR record - might contain the service name
try {
name = _readName(data, offset);
} catch (_) {}
}

offset += rdLength;
}
} catch (e) {
_log.w('Error parsing DNS records: $e');
}

// Determine protocol from port if not specified in TXT
if (port == 443) protocol = 'https';

return DiscoveredServer(
host: host,
port: port,
protocol: protocol,
name: name,
);
}
Comment on lines +185 to +261

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Any mDNS response from the network is returned as a "discovered server", even if unrelated.

The method returns a DiscoveredServer for any packet with the response flag set, regardless of whether it actually advertises _rutorrent_mobile._tcp. If another device on the network responds to a different mDNS query around the same time, it could be incorrectly listed as a ruTorrent server with default port 80.

Consider validating that at least one record in the response matches the expected service type before returning a server.

🔧 Suggested approach

Add a flag to track whether a relevant record was found:

  DiscoveredServer? _parseMdnsResponse(Datagram datagram) {
    final data = datagram.data;

    // Basic validation - minimum DNS header size
    if (data.length < 12) return null;

    // Check if this is a response (bit 15 of flags should be 1)
    if ((data[2] & 0x80) == 0) return null;

    // Use the sender's address as the host
    final host = datagram.address.address;
    int port = 80;
    String protocol = 'http';
    String? name;
+   bool foundRelevantRecord = false;

    // Try to extract SRV record (port) and TXT record (protocol)
    // ... parsing logic ...
    
+       if (type == 33 && rdLength >= 6) {
+         // SRV record
+         port = (data[offset + 4] << 8) | data[offset + 5];
+         foundRelevantRecord = true;
+       }
    // ... rest of parsing ...

+   if (!foundRelevantRecord) return null;
+
    // Determine protocol from port if not specified in TXT
    if (port == 443) protocol = 'https';
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/services/functional_services/discovery_service.dart` around lines 185 -
261, _parseMdnsResponse currently returns a DiscoveredServer for any DNS
response; add a boolean (e.g., foundService) initialized false and set it to
true when you encounter a PTR (type 12) or SRV (type 33) record whose service
name matches "_rutorrent_mobile._tcp" (use _readName or the PTR target text),
and only construct/return DiscoveredServer if foundService is true (otherwise
return null). Also ensure you only apply SRV/TXT-derived port/protocol values
when foundService is true so unrelated packets don't default to port 80;
reference symbols: _parseMdnsResponse, _skipName, _readName, type 12 (PTR), type
33 (SRV), and type 16 (TXT).


/// Skips a DNS name in the packet, handling compression pointers.
int _skipName(List<int> data, int offset) {
while (offset < data.length) {
final len = data[offset];
if (len == 0) return offset + 1;
if ((len & 0xC0) == 0xC0) return offset + 2; // Compression pointer
offset += len + 1;
}
return offset;
}

/// Reads a DNS name from the packet at the given offset.
String _readName(List<int> data, int offset) {
final parts = <String>[];
int maxJumps = 10;
while (offset < data.length && maxJumps > 0) {
final len = data[offset];
if (len == 0) break;
if ((len & 0xC0) == 0xC0) {
offset = ((len & 0x3F) << 8) | data[offset + 1];
maxJumps--;
continue;
}
offset++;
if (offset + len > data.length) break;
parts.add(String.fromCharCodes(data.sublist(offset, offset + len)));
offset += len;
}
return parts.join('.');
}
}
Loading