Skip to content

Commit bf31389

Browse files
authored
fix: correctness fixes across gotrue, postgrest, storage, supabase and supabase_flutter (#1445)
## Summary A batch of correctness fixes found during a code review sweep. Each fix is in its own per-package commit and ships with a regression test that was verified to fail on the current `main` and pass with the fix. ### gotrue - `verifyOTP` sent the captcha token under `captchaToken` inside `gotrue_meta_security`, but the server reads `captcha_token`, so the token was silently dropped. Now uses `captcha_token` like every other call. - `resend` checked `containsKey(['message_id'])` with a list argument (always false), so `ResendResponse.messageId` was never populated. Now uses the string key. ### postgrest - `.maybeSingle().count()` with zero matching rows and no converter returned `null as T` where `T` is the non-nullable `PostgrestResponse`, throwing a `TypeError`. Now returns a response with null data and `count: 0`. ### storage_client - A single `MultipartFile` instance was reused across retry attempts, so a retry re-finalized an already finalized file and threw a `StateError` instead of retrying. The request factory now builds a fresh `MultipartFile` per attempt. - Binary signed url uploads derived the content type from the full url including the `?token=` query string, defeating mime lookup and falling back to `application/octet-stream`. Now uses the url path. - `_removeEmptyFolders` used JavaScript regex literals (`/.../g`) that never matched, making it a no-op. Now uses valid Dart patterns so signed url paths are normalized. ### supabase - The `headers` setter rebuilt the realtime client headers from the client headers only, dropping the `apikey` entry present at construction. Now includes the `apikey`. Also removes a redundant self-spread when merging rest headers before an `rpc`. ### supabase_flutter - When initialized with a custom `accessToken`, `_restoreSessionCancellableOperation` was never assigned, so `dispose()` threw a `LateInitializationError`. The field is now nullable and cancelled with a null-aware call. ## Test plan - New and updated tests in each package, all mock based and runnable without a backend. - Each test was confirmed to fail on the unpatched `main` and pass with the fix. - `dart analyze` and `dart format` are clean across all changed files.
1 parent 9dffae1 commit bf31389

12 files changed

Lines changed: 236 additions & 29 deletions

File tree

packages/gotrue/lib/src/gotrue_client.dart

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -595,7 +595,7 @@ class GoTrueClient {
595595
if (token != null) 'token': token,
596596
'type': type.snakeCase,
597597
'redirect_to': redirectTo,
598-
'gotrue_meta_security': {'captchaToken': captchaToken},
598+
'gotrue_meta_security': {'captcha_token': captchaToken},
599599
if (tokenHash != null) 'token_hash': tokenHash,
600600
};
601601
final fetchOptions = GotrueRequestOptions(headers: _headers, body: body);
@@ -765,7 +765,7 @@ class GoTrueClient {
765765
options: options,
766766
);
767767

768-
if ((response as Map).containsKey(['message_id'])) {
768+
if ((response as Map).containsKey('message_id')) {
769769
return ResendResponse(messageId: response['message_id']);
770770
}
771771
return ResendResponse();

packages/gotrue/test/otp_mock_test.dart

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -244,6 +244,15 @@ void main() {
244244
expect(response, isA<ResendResponse>());
245245
});
246246

247+
test('resend() parses the message id from the response', () async {
248+
final response = await client.resend(
249+
phone: testPhone,
250+
type: OtpType.sms,
251+
);
252+
253+
expect(response.messageId, 'mock-message-id-resend');
254+
});
255+
247256
test('resend() with email type', () async {
248257
final response = await client.resend(
249258
email: testEmail,
@@ -610,5 +619,19 @@ void main() {
610619
);
611620
expect(mockClient.lastRequestBody?['type'], 'email_change');
612621
});
622+
623+
test('verifyOTP() sends the captcha token under the captcha_token key',
624+
() async {
625+
await client.verifyOTP(
626+
phone: testPhone,
627+
token: '123456',
628+
type: OtpType.sms,
629+
captchaToken: 'captcha-token-123',
630+
);
631+
632+
final security = mockClient.lastRequestBody?['gotrue_meta_security']
633+
as Map<String, dynamic>?;
634+
expect(security?['captcha_token'], 'captcha-token-123');
635+
});
613636
});
614637
}

packages/postgrest/lib/src/postgrest_builder.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -366,7 +366,7 @@ class PostgrestBuilder<T, S, R> implements Future<T> {
366366
return PostgrestResponse<S>(data: _converter(null as R), count: 0)
367367
as T;
368368
}
369-
return null as T;
369+
return PostgrestResponse<S>(data: null as S, count: 0) as T;
370370
}
371371
if (_converter != null) {
372372
return _converter(null as R) as T;
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
import 'dart:convert';
2+
3+
import 'package:http/http.dart';
4+
import 'package:postgrest/postgrest.dart';
5+
import 'package:test/test.dart';
6+
7+
/// Mimics PostgREST returning the "0 rows" error that `maybeSingle()` treats as
8+
/// an empty result rather than a failure.
9+
class ZeroRowsHttpClient extends BaseClient {
10+
@override
11+
Future<StreamedResponse> send(BaseRequest request) async {
12+
return StreamedResponse(
13+
Stream.value(utf8.encode(jsonEncode({
14+
'code': 'PGRST116',
15+
'details': 'Results contain 0 rows',
16+
'hint': null,
17+
'message': 'JSON object requested, multiple (or no) rows returned',
18+
}))),
19+
406,
20+
request: request,
21+
);
22+
}
23+
}
24+
25+
void main() {
26+
test('maybeSingle().count() returns null data and count 0 when no rows match',
27+
() async {
28+
final postgrest = PostgrestClient(
29+
'https://example.com',
30+
httpClient: ZeroRowsHttpClient(),
31+
);
32+
33+
final response = await postgrest
34+
.from('users')
35+
.update({'name': 'x'})
36+
.select()
37+
.maybeSingle()
38+
.count();
39+
40+
expect(response.data, isNull);
41+
expect(response.count, 0);
42+
});
43+
}

packages/storage_client/lib/src/fetch.dart

Lines changed: 17 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -107,16 +107,16 @@ class Fetch {
107107
final contentType = fileOptions.contentType != null
108108
? MediaType.parse(fileOptions.contentType!)
109109
: _parseMediaType(file.path);
110-
final multipartFile = http.MultipartFile.fromBytes(
111-
'',
112-
file.readAsBytesSync(),
113-
filename: file.path,
114-
contentType: contentType,
115-
);
110+
final bytes = file.readAsBytesSync();
116111
return _handleMultipartRequest(
117112
method,
118113
url,
119-
multipartFile,
114+
() => http.MultipartFile.fromBytes(
115+
'',
116+
bytes,
117+
filename: file.path,
118+
contentType: contentType,
119+
),
120120
fileOptions,
121121
options,
122122
retryAttempts,
@@ -135,18 +135,17 @@ class Fetch {
135135
) {
136136
final contentType = fileOptions.contentType != null
137137
? MediaType.parse(fileOptions.contentType!)
138-
: _parseMediaType(url);
139-
final multipartFile = http.MultipartFile.fromBytes(
140-
'',
141-
data,
142-
// request fails with null filename so set it empty instead.
143-
filename: '',
144-
contentType: contentType,
145-
);
138+
: _parseMediaType(Uri.parse(url).path);
146139
return _handleMultipartRequest(
147140
method,
148141
url,
149-
multipartFile,
142+
() => http.MultipartFile.fromBytes(
143+
'',
144+
data,
145+
// request fails with null filename so set it empty instead.
146+
filename: '',
147+
contentType: contentType,
148+
),
150149
fileOptions,
151150
options,
152151
retryAttempts,
@@ -157,7 +156,7 @@ class Fetch {
157156
Future<dynamic> _handleMultipartRequest(
158157
String method,
159158
String url,
160-
MultipartFile multipartFile,
159+
MultipartFile Function() createMultipartFile,
161160
FileOptions fileOptions,
162161
FetchOptions? options,
163162
int retryAttempts,
@@ -169,7 +168,7 @@ class Fetch {
169168
http.MultipartRequest createRequest() {
170169
final request = http.MultipartRequest(method, Uri.parse(url))
171170
..headers.addAll(headers)
172-
..files.add(multipartFile)
171+
..files.add(createMultipartFile())
173172
..fields['cacheControl'] = fileOptions.cacheControl
174173
..headers['x-upsert'] = fileOptions.upsert.toString();
175174
if (fileOptions.metadata != null) {

packages/storage_client/lib/src/storage_file_api.dart

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,9 +41,7 @@ class StorageFileApi {
4141
}
4242

4343
String _removeEmptyFolders(String path) {
44-
return path
45-
.replaceAll(RegExp(r'/^\/|\/$/g'), '')
46-
.replaceAll(RegExp(r'/\/+/g'), '/');
44+
return path.replaceAll(RegExp(r'^/|/$'), '').replaceAll(RegExp(r'/+'), '/');
4745
}
4846

4947
/// Uploads a file to an existing bucket.
Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,104 @@
1+
import 'dart:convert';
2+
import 'dart:typed_data';
3+
4+
import 'package:http/http.dart';
5+
import 'package:storage_client/storage_client.dart';
6+
import 'package:test/test.dart';
7+
8+
import 'custom_http_client.dart';
9+
10+
const storageUrl = 'http://localhost/storage/v1';
11+
const headers = {'Authorization': 'Bearer token'};
12+
13+
/// Client that finalizes (reads) the request body before failing, mimicking a
14+
/// real HTTP client. This exercises [MultipartFile] finalization on every retry
15+
/// attempt, which previously crashed because the same file instance was reused.
16+
class FinalizingRetryHttpClient extends BaseClient {
17+
FinalizingRetryHttpClient({this.failuresBeforeSuccess = 1});
18+
19+
final int failuresBeforeSuccess;
20+
int attempts = 0;
21+
22+
@override
23+
Future<StreamedResponse> send(BaseRequest request) async {
24+
attempts++;
25+
await request.finalize().drain<void>();
26+
if (attempts <= failuresBeforeSuccess) {
27+
throw ClientException('Offline');
28+
}
29+
return StreamedResponse(
30+
Stream.value(utf8.encode(jsonEncode({'Key': 'public/a.txt'}))),
31+
201,
32+
request: request,
33+
);
34+
}
35+
}
36+
37+
void main() {
38+
group('multipart uploads', () {
39+
test('retries a binary upload after a failure that finalized the request',
40+
() async {
41+
final retryClient = FinalizingRetryHttpClient(failuresBeforeSuccess: 1);
42+
final client = SupabaseStorageClient(
43+
storageUrl,
44+
headers,
45+
httpClient: retryClient,
46+
retryAttempts: 3,
47+
);
48+
49+
final result = await client
50+
.from('bucket')
51+
.uploadBinary('folder/file.png', Uint8List.fromList([1, 2, 3]));
52+
53+
expect(result, 'public/a.txt');
54+
expect(retryClient.attempts, 2);
55+
});
56+
57+
test('detects content type from the path of a binary signed url upload',
58+
() async {
59+
final mockClient = CustomHttpClient();
60+
mockClient.response = <String, dynamic>{};
61+
mockClient.statusCode = 200;
62+
final client = SupabaseStorageClient(
63+
storageUrl,
64+
headers,
65+
httpClient: mockClient,
66+
);
67+
68+
await client.from('bucket').uploadBinaryToSignedUrl(
69+
'folder/image.png',
70+
'signed-token',
71+
Uint8List.fromList([1, 2, 3]),
72+
);
73+
74+
final request = mockClient.receivedRequests.single as MultipartRequest;
75+
expect(request.files.single.contentType.mimeType, 'image/png');
76+
});
77+
});
78+
79+
group('path normalization', () {
80+
test('removes leading, trailing and duplicate slashes from the path',
81+
() async {
82+
final mockClient = CustomHttpClient();
83+
mockClient.response = <String, dynamic>{};
84+
mockClient.statusCode = 200;
85+
final client = SupabaseStorageClient(
86+
storageUrl,
87+
headers,
88+
httpClient: mockClient,
89+
);
90+
91+
final cleanPath = await client.from('bucket').uploadBinaryToSignedUrl(
92+
'/folder//image.png/',
93+
'signed-token',
94+
Uint8List.fromList([1]),
95+
);
96+
97+
expect(cleanPath, 'folder/image.png');
98+
99+
final requestPath = mockClient.receivedRequests.single.url.path;
100+
expect(requestPath, endsWith('/bucket/folder/image.png'));
101+
expect(requestPath.contains('//'), isFalse);
102+
});
103+
});
104+
}

packages/supabase/lib/src/supabase_client.dart

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,10 @@ class SupabaseClient {
113113
// manually unsubscribe and resubscribe to all channels.
114114
realtime.headers
115115
..clear()
116-
..addAll(_headers);
116+
..addAll({
117+
'apikey': _supabaseKey,
118+
..._headers,
119+
});
117120
}
118121

119122
/// {@macro supabase_client}
@@ -213,7 +216,7 @@ class SupabaseClient {
213216
Map<String, dynamic>? params,
214217
get = false,
215218
}) {
216-
rest.headers.addAll({...rest.headers, ...headers});
219+
rest.headers.addAll(headers);
217220
return rest.rpc(fn, params: params, get: get);
218221
}
219222

packages/supabase/lib/src/supabase_query_schema.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ class SupabaseQuerySchema {
5454
Map<String, dynamic>? params,
5555
bool get = false,
5656
}) {
57-
_rest.headers.addAll({..._rest.headers, ..._headers});
57+
_rest.headers.addAll(_headers);
5858
return _rest.rpc(
5959
fn,
6060
params: params,

packages/supabase/test/client_test.dart

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -305,6 +305,13 @@ void main() {
305305
expect(supabase.headers['X-Client-Info'], startsWith('supabase-dart/'));
306306
});
307307

308+
test('should preserve apikey on realtime headers when setting headers',
309+
() {
310+
supabase.headers = {'Custom-Header': 'custom-value'};
311+
312+
expect(supabase.realtime.headers['apikey'], supabaseKey);
313+
});
314+
308315
test('should not update auth headers when using custom access token', () {
309316
final customTokenClient = SupabaseClient(
310317
supabaseUrl,

0 commit comments

Comments
 (0)