From 2d2ca32a7b9c1a5fbac27120d289f8bb23d9f88e Mon Sep 17 00:00:00 2001 From: Tony Chen Date: Mon, 14 Sep 2026 21:58:33 +1000 Subject: [PATCH 1/4] Introduce HTTP connection reuse, remove redundant GET requests, and batch encryption-key updates --- lib/src/solid/api/http_client.dart | 141 ++++++++++++++++ lib/src/solid/api/rest_api.dart | 128 +++++++++++---- lib/src/solid/read_pod.dart | 30 ++-- .../solid/utils/individual_key_manager.dart | 153 ++++++++++++++---- lib/src/solid/utils/key_manager.dart | 22 +++ lib/src/solid/utils/session.dart | 8 + lib/src/solid/write_pod.dart | 115 ++++++++++--- pubspec.yaml | 8 +- 8 files changed, 501 insertions(+), 104 deletions(-) create mode 100644 lib/src/solid/api/http_client.dart diff --git a/lib/src/solid/api/http_client.dart b/lib/src/solid/api/http_client.dart new file mode 100644 index 00000000..ed9678c3 --- /dev/null +++ b/lib/src/solid/api/http_client.dart @@ -0,0 +1,141 @@ +/// A shared, connection-pooling HTTP client for all POD requests. +/// +/// Copyright (C) 2026, Software Innovation Institute, ANU. +/// +/// Licensed under the MIT License (the "License"). +/// +/// License: https://choosealicense.com/licenses/mit/. +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +// SOFTWARE. +/// +/// Authors: Tony Chen + +library; + +import 'dart:async'; + +import 'package:flutter/foundation.dart' show debugPrint; + +import 'package:http/http.dart' as http; + +/// The process-wide client used for every request to a POD server. +/// +/// The top-level helpers in `package:http` (`http.get()`, `http.put()`, ...) +/// build a brand new [http.Client] for each call and close it again as soon as +/// the response arrives. On `dart:io` platforms that means a new `HttpClient` +/// with its own, immediately discarded, connection pool: every request pays a +/// fresh TCP handshake plus a full TLS handshake, so a single write to a POD +/// (which issues several requests) costs several extra round trips. The cost +/// scales with the network distance to the POD server and with how fast the +/// platform's TLS stack is, which is why the same operation can feel instant +/// against a nearby server and painfully slow against a distant one. +/// +/// Sharing one client keeps the underlying connections alive (the `Connection: +/// keep-alive` header the API already sends finally means something), so only +/// the first request to a host pays for the handshakes. +/// +/// On the web `http.Client()` is a `BrowserClient` and connection reuse is the +/// browser's business; sharing the instance is still correct and avoids +/// allocating a client per request. + +http.Client get podHttpClient => + _podHttpClient ??= _RetryIdleConnectionClient(http.Client()); + +http.Client? _podHttpClient; + +/// Close and drop the shared client. +/// +/// Call on logout so no authenticated connection is kept open. The next +/// request transparently creates a new client. + +void closePodHttpClient() { + _podHttpClient?.close(); + _podHttpClient = null; +} + +/// Replace the shared client, closing any existing one. +/// +/// Intended for tests, which can inject a `MockClient` here. + +void setPodHttpClient(http.Client client) { + _podHttpClient?.close(); + _podHttpClient = client; +} + +/// Retries a request once when the connection dies before any response +/// arrives. +/// +/// This is the cost of keeping connections alive: a server (or an intervening +/// proxy) may close an idle connection at the very moment the client picks it +/// up for the next request, which surfaces as a [http.ClientException] such as +/// "Connection closed before full header was received". Nothing was served, so +/// re-sending on a fresh connection is safe and invisible to the caller. +/// +/// Only methods that can be repeated without changing the outcome are retried. +/// POST and PATCH are left alone: a POST that did reach the server would +/// create a second resource, and a retry of a partly applied PATCH is not +/// equivalent to the original. + +class _RetryIdleConnectionClient extends http.BaseClient { + _RetryIdleConnectionClient(this._inner); + + final http.Client _inner; + + static const _retryableMethods = {'GET', 'HEAD', 'PUT', 'DELETE'}; + + @override + Future send(http.BaseRequest request) async { + // A request can only be sent once, so a retry needs a fresh copy. Only + // in-memory requests can be copied; a streamed body cannot be replayed. + + final copy = _retryableMethods.contains(request.method.toUpperCase()) + ? _copyOf(request) + : null; + + try { + return await _inner.send(request); + } on http.ClientException catch (e) { + if (copy == null) rethrow; + + debugPrint( + 'Retrying ${request.method} ${request.url} after connection error: ' + '${e.message}', + ); + + return await _inner.send(copy); + } + } + + /// A resendable duplicate of [request], or null if its body cannot be + /// replayed. + + static http.BaseRequest? _copyOf(http.BaseRequest request) { + if (request is! http.Request) return null; + + return http.Request(request.method, request.url) + ..headers.addAll(request.headers) + ..followRedirects = request.followRedirects + ..maxRedirects = request.maxRedirects + ..persistentConnection = request.persistentConnection + ..bodyBytes = request.bodyBytes; + } + + @override + void close() => _inner.close(); +} diff --git a/lib/src/solid/api/rest_api.dart b/lib/src/solid/api/rest_api.dart index a24b9d9c..cbc59aee 100644 --- a/lib/src/solid/api/rest_api.dart +++ b/lib/src/solid/api/rest_api.dart @@ -37,11 +37,11 @@ import 'dart:typed_data' show Uint8List; import 'package:flutter/foundation.dart' show debugPrint; -import 'package:http/http.dart' as http; import 'package:intl/intl.dart'; import 'package:mime/mime.dart' as mime; import 'package:rdf/rdf.dart'; +import 'package:solidpod/src/solid/api/http_client.dart'; import 'package:solidpod/src/solid/constants/common.dart'; import 'package:solidpod/src/solid/utils/authdata_manager.dart'; import 'package:solidpod/src/solid/utils/exceptions.dart'; @@ -194,8 +194,13 @@ Future> initialStructureTest( /// on a server using HTTP requests: /// - PUT request: create or replace a resource if exists (e.g. an ACL file) /// - POST request: create a resource (e.g. a TTL file or a directory) +/// +/// Returns the HTTP status code of the successful response. A 201 means the +/// server created a new resource, 200/205 that it replaced an existing one. +/// Callers can use that to skip a separate existence check (see [writePod], +/// which only probes for an ACL when the data file already existed). -Future createResource( +Future createResource( String resourceUrl, { dynamic content = '', bool isFile = true, @@ -213,7 +218,7 @@ Future createResource( // Use PUT request for creating and replacing a file if it already exists final put = (isFile && replaceIfExist) ? true : false; - final httpMethod = put ? http.put : http.post; + final httpMethod = put ? podHttpClient.put : podHttpClient.post; // Get the name and parent container URL of the resource to be created for // POST request @@ -259,7 +264,7 @@ Future createResource( ); if ([200, 201, 205].contains(response.statusCode)) { - return; + return response.statusCode; } else if (response.statusCode == 403) { // No write permission at this location. Surface a typed, actionable error // so callers (e.g. writing to another user's POD) can distinguish a @@ -289,7 +294,7 @@ Future deleteResource( 'DELETE', ); - final response = await http.delete( + final response = await podHttpClient.delete( Uri.parse(resourceUrl), headers: { 'Accept': '*/*', @@ -320,55 +325,100 @@ Future deleteResource( /// Asynchronously checks whether a given resource exists on the server. /// -/// This function makes an HTTP GET request to the specified resource URL to determine if the resource exists. +/// This function makes an HTTP request to the specified resource URL to determine if the resource exists. /// It handles both files and directories (containers) by setting appropriate headers based on the [isFile]. +/// +/// Set [useHead] when only the existence of the resource matters. A GET +/// downloads the whole resource just to look at its status code, which is +/// wasteful on the write path where the body is discarded. HEAD returns the +/// same status with no body. Servers that do not implement HEAD for a +/// resource (405/501, or anything else unexpected) fall back to the GET, so +/// enabling it never changes the answer — only how much is transferred. Future checkResourceStatus( String resUrl, { bool isFile = true, + bool useHead = false, }) async { if (!isFile) { assert(resUrl.endsWith('/')); } else { assert(!resUrl.endsWith('/')); } + + final headers = { + 'Content-Type': isFile + ? ResourceContentType.any.value + : ResourceContentType.directory.value, + 'Link': isFile ? fileTypeLink : dirTypeLink, + ...noHttpCacheHeaders, + }; + + if (useHead) { + final (:accessToken, :dPopToken) = + await getTokensForResource(resUrl, 'HEAD'); + final headResponse = await podHttpClient.head( + Uri.parse(resUrl), + headers: { + ...headers, + 'Authorization': 'DPoP $accessToken', + 'DPoP': dPopToken, + }, + ); + + final status = _statusFromCode(headResponse.statusCode); + if (status != null) { + return status; + } + + // Unexpected code (e.g. a server without HEAD support): fall through to + // the GET below rather than reporting ResourceStatus.unknown. + + debugPrint( + 'HEAD $resUrl returned ${headResponse.statusCode}, retrying with GET.', + ); + } + final (:accessToken, :dPopToken) = await getTokensForResource(resUrl, 'GET'); - final response = await http.get( + final response = await podHttpClient.get( Uri.parse(resUrl), headers: { - 'Content-Type': isFile - ? ResourceContentType.any.value - : ResourceContentType.directory.value, + ...headers, 'Authorization': 'DPoP $accessToken', - 'Link': isFile ? fileTypeLink : dirTypeLink, 'DPoP': dPopToken, - ...noHttpCacheHeaders, }, ); - if (response.statusCode == 200 || response.statusCode == 204) { - return ResourceStatus.exist; - } else if (response.statusCode == 403) { - return ResourceStatus.forbidden; - } else if (response.statusCode == 404) { - return ResourceStatus.notExist; - } else { - debugPrint( - 'Failed to check resource status.\n' - 'URL: $resUrl\n' - 'ERR: ${response.body}', - ); - return ResourceStatus.unknown; + final status = _statusFromCode(response.statusCode); + if (status != null) { + return status; } + + debugPrint( + 'Failed to check resource status.\n' + 'URL: $resUrl\n' + 'ERR: ${response.body}', + ); + return ResourceStatus.unknown; } +/// Map an HTTP status code onto a [ResourceStatus], or null when the code +/// says nothing about whether the resource exists. + +ResourceStatus? _statusFromCode(int code) => switch (code) { + 200 || 204 => ResourceStatus.exist, + 403 => ResourceStatus.forbidden, + 404 => ResourceStatus.notExist, + _ => null, + }; + /// Asynchronously checks whether a given webId exists. /// /// This function makes an HTTP GET request to a public resource URL to determine if the webId exists. Future checkWebIdExists(String webIdUrl) async { try { - final response = await http.get( + final response = await podHttpClient.get( Uri.parse(webIdUrl), headers: { 'Content-Type': ResourceContentType.any.value, @@ -436,7 +486,7 @@ Future checkWebIdProfile(String webIdUrl) async { // keeps the request URL and any debug logs honest. final uri = Uri.parse(webIdUrl).removeFragment(); - final response = await http.get( + final response = await podHttpClient.get( uri, headers: const { // Solid servers content-negotiate on `Accept`. List the common RDF @@ -513,7 +563,7 @@ Future updateFileByQuery(String fileUrl, String query) async { fileUrl, 'PATCH', ); - final editResponse = await http.patch( + final editResponse = await podHttpClient.patch( Uri.parse(fileUrl), headers: { 'Accept': '*/*', @@ -553,7 +603,7 @@ Future initialProfileUpdate(String profBody) async { final (:accessToken, :dPopToken) = await getTokensForResource(profUrl, 'PUT'); // The PUT request will create the acl item in the server - final updateResponse = await http.put( + final updateResponse = await podHttpClient.put( Uri.parse(profUrl), headers: { 'Accept': '*/*', @@ -576,13 +626,17 @@ Future initialProfileUpdate(String profBody) async { /// If [resourceUrl] ends with '/', i.e., a container / directory, /// This function returns the bytes of a turtle string representing /// the list of resources in the container / directory. +/// +/// Throws [ResourceNotExistException] on 404 and [AccessForbiddenException] +/// on 403 so that callers can tell the two apart without first issuing a +/// separate existence check (which would download the resource twice). Future getResource(String resourceUrl) async { final (:accessToken, :dPopToken) = await getTokensForResource( resourceUrl, 'GET', ); - final response = await http.get( + final response = await podHttpClient.get( Uri.parse(resourceUrl), headers: { 'Accept': '*/*', @@ -595,8 +649,14 @@ Future getResource(String resourceUrl) async { if (response.statusCode == 200) { return response.bodyBytes; + } else if (response.statusCode == 404) { + throw ResourceNotExistException('$resourceUrl does not exist'); + } else if (response.statusCode == 403) { + throw AccessForbiddenException('Access to $resourceUrl is not allowed'); } else { - throw Exception('Failed to get resource $resourceUrl'); + throw Exception( + 'Failed to get resource $resourceUrl (HTTP ${response.statusCode})', + ); } } @@ -611,7 +671,7 @@ Future<({List subDirs, List files})> getResourcesInContainer( final (:accessToken, :dPopToken) = await getTokensForResource(url, 'GET'); - final profResponse = await http.get( + final profResponse = await podHttpClient.get( Uri.parse(url), headers: { 'Accept': '*/*', @@ -686,7 +746,7 @@ Future getResourceMetadata(String resourceUrl) async { 'HEAD', ); - final response = await http.head( + final response = await podHttpClient.head( Uri.parse(resourceUrl), headers: { 'Accept': '*/*', @@ -742,7 +802,7 @@ Future updateAclFileContent( ); // http request to update the acl file on the server - final editResponse = await http.put( + final editResponse = await podHttpClient.put( Uri.parse(resourceAclUrl), headers: { 'Accept': '*/*', diff --git a/lib/src/solid/read_pod.dart b/lib/src/solid/read_pod.dart index f27dc81a..694d2463 100644 --- a/lib/src/solid/read_pod.dart +++ b/lib/src/solid/read_pod.dart @@ -92,23 +92,14 @@ Future readPod( pathType: pathType, ); - final fileStatus = await checkResourceStatus(fileUrl); - - if (fileStatus != ResourceStatus.exist) { - switch (fileStatus) { - case ResourceStatus.notExist: - throw ResourceNotExistException('$fileUrl does not exist'); - case ResourceStatus.forbidden: - throw AccessForbiddenException('Access to $fileUrl is not allowed'); - case ResourceStatus.unknown: - throw Exception('Unknown error.'); - default: - {} - } - } - try { - // Retrieve raw content + // Retrieve raw content. + // + // No separate existence check first: that was a second GET of the very + // same resource, so every read downloaded the file twice and paid two + // round trips instead of one. getResource() reports a missing or + // forbidden resource through ResourceNotExistException / + // AccessForbiddenException, which is what the check used to raise. final fileContent = utf8.decode(await getResource(fileUrl)); @@ -158,6 +149,13 @@ Future readPod( } return decryptData(encDataStr, encKey, IV.fromBase64(ivStr)); + } on ResourceNotExistException { + // A missing resource is an ordinary outcome callers handle themselves; + // do not dump a stack trace for it. + + rethrow; + } on AccessForbiddenException { + rethrow; } on Object catch (e, trace) { debugPrint(e.toString()); debugPrint(trace.toString()); diff --git a/lib/src/solid/utils/individual_key_manager.dart b/lib/src/solid/utils/individual_key_manager.dart index bf72c714..db8e0b25 100644 --- a/lib/src/solid/utils/individual_key_manager.dart +++ b/lib/src/solid/utils/individual_key_manager.dart @@ -28,6 +28,8 @@ library; +import 'dart:async' show Completer; + import 'package:flutter/foundation.dart' show debugPrint; import 'package:encrypter_plus/encrypter_plus.dart'; @@ -50,11 +52,26 @@ class IndividualKeyManager { static Map? _indKeyMap; + // Serialises changes to the key file. Every change rewrites the file in + // full, so two overlapping changes would race: the later PUT could land + // first and be overwritten by an earlier, smaller snapshot of the map, + // losing a key and with it the ability to decrypt the file it belongs to. + // Holding this chain across both the in-memory update and the upload keeps + // each upload a superset of the one before. + + static Future _keyFileLock = Future.value(); + + // The load in progress, if any, so that concurrent readers share one + // request for the key file rather than each fetching their own copy. + + static Future? _loadInProgress; + /// Clear all cached individual keys. static void clear() { _indKeyUrl = null; _indKeyMap = null; + _loadInProgress = null; } /// Load encrypted individual keys. @@ -64,7 +81,34 @@ class IndividualKeyManager { return; } - _indKeyMap = await readIndKeyFile(); + final pending = _loadInProgress; + if (pending != null && !forceReload) { + return pending; + } + + final load = readIndKeyFile().then((map) { + _indKeyMap = map; + }); + + _loadInProgress = load; + + try { + await load; + } finally { + if (identical(_loadInProgress, load)) { + _loadInProgress = null; + } + } + } + + /// Run [action] with exclusive access to the key file. + + static Future _withKeyFileLock(Future Function() action) { + final done = Completer(); + final previous = _keyFileLock; + _keyFileLock = done.future; + + return previous.then((_) => action()).whenComplete(done.complete); } /// Generate the content of indKeyFile and save it (on server). @@ -88,7 +132,7 @@ class IndividualKeyManager { String resourceUrl, Key masterKey, ) async { - if (_indKeyMap == null || _indKeyMap!.isEmpty) { + if (_indKeyMap == null) { await loadIndividualKeys(); } @@ -123,26 +167,70 @@ class IndividualKeyManager { required Key masterKey, bool isFile = true, }) async { - final resourceUrl = await (isFile ? getFileUrl : getDirUrl)(resourcePath); + await addIndividualKeys( + indKeys: {resourcePath: indKey}, + masterKey: masterKey, + isFile: isFile, + ); + } - if (_indKeyMap == null) { - await loadIndividualKeys(); + /// Add the (encrypted) individual keys for several resources at once, + /// persisting the key file a single time. + /// + /// Every individual key lives in one file, `encryption/ind-keys.ttl`, which + /// is rewritten in full on each change (a SPARQL PATCH is not usable here: + /// CSS servers may answer 200 without persisting the triples). Registering + /// keys one at a time therefore re-uploads the whole file per resource, and + /// the file grows by one entry per encrypted file in the POD — so writing N + /// new files costs O(N^2) bytes and N round trips of pure overhead. Adding + /// the whole batch in one go reduces that to a single upload. + + static Future addIndividualKeys({ + required Map indKeys, + required Key masterKey, + bool isFile = true, + }) async { + if (indKeys.isEmpty) { + return; } - assert(_indKeyMap != null); - final iv = genRandIV(); - final encIndKey = encryptData(indKey.base64, masterKey, iv); + await _withKeyFileLock(() async { + if (_indKeyMap == null) { + await loadIndividualKeys(); + } + assert(_indKeyMap != null); - final record = IndKeyRecord( - resourcePath: resourcePath, - encKeyBase64: encIndKey, - ivBase64: iv.base64, - ); - _indKeyMap![resourceUrl] = record; + for (final entry in indKeys.entries) { + final resourcePath = entry.key; + final resourceUrl = + await (isFile ? getFileUrl : getDirUrl)(resourcePath); - // Use full PUT overwrite instead of SPARQL PATCH — CSS servers may accept - // the PATCH request (HTTP 200) without actually persisting the triples. - await saveIndividualKeys(_indKeyMap); + final iv = genRandIV(); + final encIndKey = encryptData(entry.value.base64, masterKey, iv); + + _indKeyMap![resourceUrl] = IndKeyRecord( + resourcePath: resourcePath, + encKeyBase64: encIndKey, + ivBase64: iv.base64, + )..key = entry.value; + } + + // Use full PUT overwrite instead of SPARQL PATCH — CSS servers may + // accept the PATCH request (HTTP 200) without actually persisting the + // triples. + await saveIndividualKeys(_indKeyMap); + }); + } + + /// Whether an individual key is already registered for [resourceUrl]. + /// + /// Loads the key file if it has not been read yet, but never writes. + + static Future hasIndividualKey(String resourceUrl) async { + if (_indKeyMap == null) { + await loadIndividualKeys(); + } + return _indKeyMap!.containsKey(resourceUrl); } /// Remove the (encrypted) individual key for file. @@ -152,23 +240,26 @@ class IndividualKeyManager { bool isFile = true, }) async { final resourceUrl = await (isFile ? getFileUrl : getDirUrl)(resourcePath); - if (_indKeyMap == null) { - await loadIndividualKeys(); - } - assert(_indKeyMap != null); - if (_indKeyMap!.containsKey(resourceUrl)) { - _indKeyMap!.remove(resourceUrl); + await _withKeyFileLock(() async { + if (_indKeyMap == null) { + await loadIndividualKeys(); + } + assert(_indKeyMap != null); - // Use full PUT overwrite instead of SPARQL PATCH. - await saveIndividualKeys(_indKeyMap); + if (_indKeyMap!.containsKey(resourceUrl)) { + _indKeyMap!.remove(resourceUrl); - debugPrint('Deleted individual key for $resourcePath'); - } else { - debugPrint( - 'Individual key for "$resourcePath" does not exist, do nothing.', - ); - } + // Use full PUT overwrite instead of SPARQL PATCH. + await saveIndividualKeys(_indKeyMap); + + debugPrint('Deleted individual key for $resourcePath'); + } else { + debugPrint( + 'Individual key for "$resourcePath" does not exist, do nothing.', + ); + } + }); } /// Re-encrypt all individual keys with a new master key. diff --git a/lib/src/solid/utils/key_manager.dart b/lib/src/solid/utils/key_manager.dart index b42ea5ff..3783d3c7 100644 --- a/lib/src/solid/utils/key_manager.dart +++ b/lib/src/solid/utils/key_manager.dart @@ -564,6 +564,28 @@ class KeyManager { ); } + /// Add the (encrypted) individual keys for several resources, saving the + /// key file once for the whole batch. + /// + /// See [IndividualKeyManager.addIndividualKeys] for why this matters when + /// many files are written in one go. + + static Future addIndividualKeys({ + required Map indKeys, + bool isFile = true, + }) async { + await IndividualKeyManager.addIndividualKeys( + indKeys: indKeys, + masterKey: await getMasterKey(), + isFile: isFile, + ); + } + + /// Whether an individual key is already registered for [resourceUrl]. + + static Future hasIndividualKey(String resourceUrl) async => + IndividualKeyManager.hasIndividualKey(resourceUrl); + /// Remove the (encrypted) individual key for file. static Future removeIndividualKey({ diff --git a/lib/src/solid/utils/session.dart b/lib/src/solid/utils/session.dart index d54f7c1c..e13fbac4 100644 --- a/lib/src/solid/utils/session.dart +++ b/lib/src/solid/utils/session.dart @@ -37,6 +37,7 @@ import 'package:http/http.dart' as http; import 'package:jwt_decoder/jwt_decoder.dart'; import 'package:solid_auth/solid_auth.dart' show DpopTokenGenerator; +import 'package:solidpod/src/solid/api/http_client.dart'; import 'package:solidpod/src/solid/constants/common.dart'; import 'package:solidpod/src/solid/utils/app_info.dart'; import 'package:solidpod/src/solid/utils/authdata_manager.dart'; @@ -192,6 +193,11 @@ Future logoutPod() async { } final authDataRemoved = await AuthDataManager.removeAuthData(); + + // Drop the pooled connections so no authenticated socket is kept open. + + closePodHttpClient(); + return authDataRemoved; } on Object catch (e) { debugPrint('logoutPod() CRITICAL ERROR: $e'); @@ -245,6 +251,8 @@ Future silentLogout() async { } } + closePodHttpClient(); + return authDataRemoved; } on Object catch (e) { debugPrint('silentLogout() CRITICAL: $e'); diff --git a/lib/src/solid/write_pod.dart b/lib/src/solid/write_pod.dart index e2efbd88..872b9fef 100644 --- a/lib/src/solid/write_pod.dart +++ b/lib/src/solid/write_pod.dart @@ -28,8 +28,6 @@ library; -import 'package:flutter/foundation.dart' show debugPrint; - import 'package:encrypter_plus/encrypter_plus.dart' show Key; import 'package:mime/mime.dart' as mime; @@ -38,7 +36,10 @@ import 'package:solidpod/src/solid/constants/common.dart'; import 'package:solidpod/src/solid/constants/path_type.dart'; import 'package:solidpod/src/solid/utils/exceptions.dart'; import 'package:solidpod/src/solid/utils/io_helper.dart'; +import 'package:solidpod/src/solid/utils/key_helper.dart' + show genRandIndividualKey; import 'package:solidpod/src/solid/utils/key_inheritance.dart'; +import 'package:solidpod/src/solid/utils/key_manager.dart'; import 'package:solidpod/src/solid/utils/misc.dart'; import 'package:solidpod/src/solid/utils/permission.dart' show genAclTurtle; import 'package:solidpod/src/solid/write_external_pod.dart' @@ -154,30 +155,37 @@ Future writePod( encKey = await configureEncKey(fileUrl, inheritKeyUrl: inheritKeyUrl); } - switch (await checkResourceStatus(fileUrl)) { - case ResourceStatus.exist: - if (overwrite) { - debugPrint('NOTE: Overwriting existing file "$filePath"'); - } else { + // Whether the file is known not to exist yet. Only meaningful when we + // actually probed for it below; null means "not checked". + + bool? fileExisted; + + // With overwrite=true the PUT below replaces whatever is there, so probing + // first only costs a round trip (and, before checkResourceStatus grew a + // HEAD path, a full download of the file we are about to replace). A 403 + // still surfaces as AccessForbiddenException, raised by createResource(). + + if (!overwrite) { + switch (await checkResourceStatus(fileUrl, useHead: true)) { + case ResourceStatus.exist: throw Exception( 'File "$filePath" already exists and ' 'overwrite=$overwrite, writePod() aborted', ); - } - case ResourceStatus.unknown: - throw Exception( - 'Unable to determine if file "$fileUrl" exists, writePod() aborted', - ); + case ResourceStatus.unknown: + throw Exception( + 'Unable to determine if file "$fileUrl" exists, writePod() aborted', + ); - case ResourceStatus.forbidden: - throw AccessForbiddenException( - 'Access to file "$fileUrl" is forbidden, writePod() aborted', - ); + case ResourceStatus.forbidden: + throw AccessForbiddenException( + 'Access to file "$fileUrl" is forbidden, writePod() aborted', + ); - case ResourceStatus.notExist: // Empty case falls through. - // debugPrint('File "$fileUrl" does not exist'); - {} + case ResourceStatus.notExist: + fileExisted = false; + } } final content = encKey == null @@ -193,7 +201,7 @@ Future writePod( // Create file on server - await createResource( + final writeStatus = await createResource( fileUrl, content: content, contentType: encKey == null @@ -201,12 +209,75 @@ Future writePod( : ResourceContentType.turtleText, ); - // Create the ACL file for the data file if necessary + // Create the ACL file for the data file if necessary. if (createAcl) { final aclFileUrl = '$fileUrl.acl'; - if (await checkResourceStatus(aclFileUrl) == ResourceStatus.notExist) { + + // When the data file did not exist a moment ago, neither does its ACL, so + // the probe can be skipped. We know that either because we checked above + // (overwrite=false) or because the server answered the PUT with 201 + // Created. Servers that answer 200 for a newly created resource simply + // fall back to the probe, which is correct, just one round trip slower. + + final isNewFile = fileExisted == false || writeStatus == 201; + + if (isNewFile || + await checkResourceStatus(aclFileUrl, useHead: true) == + ResourceStatus.notExist) { await createResource(aclFileUrl, content: await genAclTurtle(fileUrl)); } } } + +/// Register encryption keys for [filePaths] up front, in a single request. +/// +/// [writePod] gives every encrypted file its own individual key and stores all +/// of them in one file, `encryption/ind-keys.ttl`, which has to be rewritten +/// in full whenever a key is added. Writing a batch of files one by one +/// therefore re-uploads that file once per file, and it carries an entry for +/// every encrypted file in the POD — so importing N records uploads O(N^2) +/// bytes of key material on top of the data itself, and gets slower the more +/// data the POD already holds. +/// +/// Calling this first adds all the missing keys in one go. The subsequent +/// [writePod] calls then find their key already registered and skip the key +/// file entirely, which also makes them independent of each other and safe to +/// run concurrently. +/// +/// Paths are interpreted exactly as [writePod] interprets its `filePath`, so +/// pass the same values (and the same [pathType]). Paths that already have a +/// key are skipped. A key registered here for a file that is never written is +/// harmless: it is simply unused. + +Future prepareEncryptionKeys( + List filePaths, { + PathType pathType = PathType.relativeToData, +}) async { + if (!await isUserLoggedIn()) { + throw NotLoggedInException('User must be logged in to write to POD'); + } + + final newKeys = {}; + + for (final filePath in filePaths) { + final fileUrl = await generateResourceUrlFromPath( + resourcePath: filePath, + pathType: pathType, + ); + + if (await KeyManager.hasIndividualKey(fileUrl)) { + continue; + } + + final resourcePath = await extractResourcePathFromUrl(fileUrl); + + // Guard against duplicates within [filePaths] itself: two entries for the + // same file would otherwise generate two keys, the second of which would + // not match content encrypted with the first. + + newKeys.putIfAbsent(resourcePath, genRandIndividualKey); + } + + await KeyManager.addIndividualKeys(indKeys: newKeys); +} diff --git a/pubspec.yaml b/pubspec.yaml index 64a77108..5b91cebb 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -35,7 +35,7 @@ dependencies: petitparser: ^7.0.2 pointycastle: ^4.0.0 rdf: ^1.0.0 - solid_auth: ^1.0.9 + solid_auth: ^1.0.10 universal_io: ^2.3.1 dev_dependencies: @@ -45,6 +45,12 @@ dev_dependencies: flutter_lints: ^6.0.0 import_order_lint: ^0.2.3 +dependency_overrides: + solid_auth: + git: + url: https://github.com/anusii/solid_auth.git + ref: tony/553_upload_performance + flutter: config: enable-swift-package-manager: false From 303b929513d5c653e3d11c928939d4e72e5f1da2 Mon Sep 17 00:00:00 2001 From: Tony Chen Date: Tue, 15 Sep 2026 23:17:09 +1000 Subject: [PATCH 2/4] Restore the readPod and writePod fns --- lib/src/solid/api/rest_api.dart | 9 +---- lib/src/solid/read_pod.dart | 39 +++++++++++------- lib/src/solid/write_pod.dart | 71 +++++++++++++++------------------ pubspec.yaml | 2 +- 4 files changed, 61 insertions(+), 60 deletions(-) diff --git a/lib/src/solid/api/rest_api.dart b/lib/src/solid/api/rest_api.dart index cbc59aee..d3cd65e6 100644 --- a/lib/src/solid/api/rest_api.dart +++ b/lib/src/solid/api/rest_api.dart @@ -194,13 +194,8 @@ Future> initialStructureTest( /// on a server using HTTP requests: /// - PUT request: create or replace a resource if exists (e.g. an ACL file) /// - POST request: create a resource (e.g. a TTL file or a directory) -/// -/// Returns the HTTP status code of the successful response. A 201 means the -/// server created a new resource, 200/205 that it replaced an existing one. -/// Callers can use that to skip a separate existence check (see [writePod], -/// which only probes for an ACL when the data file already existed). -Future createResource( +Future createResource( String resourceUrl, { dynamic content = '', bool isFile = true, @@ -264,7 +259,7 @@ Future createResource( ); if ([200, 201, 205].contains(response.statusCode)) { - return response.statusCode; + return; } else if (response.statusCode == 403) { // No write permission at this location. Surface a typed, actionable error // so callers (e.g. writing to another user's POD) can distinguish a diff --git a/lib/src/solid/read_pod.dart b/lib/src/solid/read_pod.dart index 694d2463..b3a34185 100644 --- a/lib/src/solid/read_pod.dart +++ b/lib/src/solid/read_pod.dart @@ -92,14 +92,32 @@ Future readPod( pathType: pathType, ); + // Check the resource is there and readable before fetching it, so that a + // missing or forbidden resource is reported as such rather than as whatever + // the fetch happens to fail with. + // + // The probe uses HEAD, not GET: it only needs the status code, and a GET + // downloaded the entire resource a second time purely to discard it. + // checkResourceStatus() falls back to GET on any server that does not + // answer HEAD, so the outcome is unchanged. + + final fileStatus = await checkResourceStatus(fileUrl, useHead: true); + + if (fileStatus != ResourceStatus.exist) { + switch (fileStatus) { + case ResourceStatus.notExist: + throw ResourceNotExistException('$fileUrl does not exist'); + case ResourceStatus.forbidden: + throw AccessForbiddenException('Access to $fileUrl is not allowed'); + case ResourceStatus.unknown: + throw Exception('Unknown error.'); + default: + {} + } + } + try { - // Retrieve raw content. - // - // No separate existence check first: that was a second GET of the very - // same resource, so every read downloaded the file twice and paid two - // round trips instead of one. getResource() reports a missing or - // forbidden resource through ResourceNotExistException / - // AccessForbiddenException, which is what the check used to raise. + // Retrieve raw content final fileContent = utf8.decode(await getResource(fileUrl)); @@ -149,13 +167,6 @@ Future readPod( } return decryptData(encDataStr, encKey, IV.fromBase64(ivStr)); - } on ResourceNotExistException { - // A missing resource is an ordinary outcome callers handle themselves; - // do not dump a stack trace for it. - - rethrow; - } on AccessForbiddenException { - rethrow; } on Object catch (e, trace) { debugPrint(e.toString()); debugPrint(trace.toString()); diff --git a/lib/src/solid/write_pod.dart b/lib/src/solid/write_pod.dart index 872b9fef..4b060562 100644 --- a/lib/src/solid/write_pod.dart +++ b/lib/src/solid/write_pod.dart @@ -28,6 +28,8 @@ library; +import 'package:flutter/foundation.dart' show debugPrint; + import 'package:encrypter_plus/encrypter_plus.dart' show Key; import 'package:mime/mime.dart' as mime; @@ -155,37 +157,40 @@ Future writePod( encKey = await configureEncKey(fileUrl, inheritKeyUrl: inheritKeyUrl); } - // Whether the file is known not to exist yet. Only meaningful when we - // actually probed for it below; null means "not checked". - - bool? fileExisted; - - // With overwrite=true the PUT below replaces whatever is there, so probing - // first only costs a round trip (and, before checkResourceStatus grew a - // HEAD path, a full download of the file we are about to replace). A 403 - // still surfaces as AccessForbiddenException, raised by createResource(). - - if (!overwrite) { - switch (await checkResourceStatus(fileUrl, useHead: true)) { - case ResourceStatus.exist: + // Check what is already at the target before uploading anything, so that an + // existing file is never silently replaced when overwrite is false and a + // forbidden or indeterminate target is reported before the upload rather + // than through whatever the upload happens to fail with. + // + // The probe uses HEAD, not GET: only the status code is wanted, and a GET + // downloaded the whole file that was about to be replaced. On a server that + // does not answer HEAD, checkResourceStatus() falls back to GET, so the + // outcome is unchanged. + + switch (await checkResourceStatus(fileUrl, useHead: true)) { + case ResourceStatus.exist: + if (overwrite) { + debugPrint('NOTE: Overwriting existing file "$filePath"'); + } else { throw Exception( 'File "$filePath" already exists and ' 'overwrite=$overwrite, writePod() aborted', ); + } - case ResourceStatus.unknown: - throw Exception( - 'Unable to determine if file "$fileUrl" exists, writePod() aborted', - ); + case ResourceStatus.unknown: + throw Exception( + 'Unable to determine if file "$fileUrl" exists, writePod() aborted', + ); - case ResourceStatus.forbidden: - throw AccessForbiddenException( - 'Access to file "$fileUrl" is forbidden, writePod() aborted', - ); + case ResourceStatus.forbidden: + throw AccessForbiddenException( + 'Access to file "$fileUrl" is forbidden, writePod() aborted', + ); - case ResourceStatus.notExist: - fileExisted = false; - } + case ResourceStatus.notExist: // Empty case falls through. + // debugPrint('File "$fileUrl" does not exist'); + {} } final content = encKey == null @@ -201,7 +206,7 @@ Future writePod( // Create file on server - final writeStatus = await createResource( + await createResource( fileUrl, content: content, contentType: encKey == null @@ -209,22 +214,12 @@ Future writePod( : ResourceContentType.turtleText, ); - // Create the ACL file for the data file if necessary. + // Create the ACL file for the data file if necessary if (createAcl) { final aclFileUrl = '$fileUrl.acl'; - - // When the data file did not exist a moment ago, neither does its ACL, so - // the probe can be skipped. We know that either because we checked above - // (overwrite=false) or because the server answered the PUT with 201 - // Created. Servers that answer 200 for a newly created resource simply - // fall back to the probe, which is correct, just one round trip slower. - - final isNewFile = fileExisted == false || writeStatus == 201; - - if (isNewFile || - await checkResourceStatus(aclFileUrl, useHead: true) == - ResourceStatus.notExist) { + if (await checkResourceStatus(aclFileUrl, useHead: true) == + ResourceStatus.notExist) { await createResource(aclFileUrl, content: await genAclTurtle(fileUrl)); } } diff --git a/pubspec.yaml b/pubspec.yaml index 5b91cebb..80fb6365 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -35,7 +35,7 @@ dependencies: petitparser: ^7.0.2 pointycastle: ^4.0.0 rdf: ^1.0.0 - solid_auth: ^1.0.10 + solid_auth: ^1.0.9 universal_io: ^2.3.1 dev_dependencies: From 41e4369ad9f787009cdda5589faad7f5f8941576 Mon Sep 17 00:00:00 2001 From: Tony Chen Date: Tue, 15 Sep 2026 23:19:13 +1000 Subject: [PATCH 3/4] Update pubspec.yaml --- pubspec.yaml | 6 ------ 1 file changed, 6 deletions(-) diff --git a/pubspec.yaml b/pubspec.yaml index 80fb6365..64a77108 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -45,12 +45,6 @@ dev_dependencies: flutter_lints: ^6.0.0 import_order_lint: ^0.2.3 -dependency_overrides: - solid_auth: - git: - url: https://github.com/anusii/solid_auth.git - ref: tony/553_upload_performance - flutter: config: enable-swift-package-manager: false From d7b10f35cb9884185d03262c2a86820d20e52ddb Mon Sep 17 00:00:00 2001 From: Tony Chen Date: Sun, 20 Sep 2026 00:42:30 +1000 Subject: [PATCH 4/4] Lint --- lib/src/solid/api/http_client.dart | 9 --------- 1 file changed, 9 deletions(-) diff --git a/lib/src/solid/api/http_client.dart b/lib/src/solid/api/http_client.dart index ed9678c3..05adbe33 100644 --- a/lib/src/solid/api/http_client.dart +++ b/lib/src/solid/api/http_client.dart @@ -69,15 +69,6 @@ void closePodHttpClient() { _podHttpClient = null; } -/// Replace the shared client, closing any existing one. -/// -/// Intended for tests, which can inject a `MockClient` here. - -void setPodHttpClient(http.Client client) { - _podHttpClient?.close(); - _podHttpClient = client; -} - /// Retries a request once when the connection dies before any response /// arrives. ///