Skip to content

Commit e7c69b2

Browse files
yyodsyyods
committed
[red] Prevent GitHub import resource and retry bypasses
Signed-off-by: Viwat Vchirawongkwin <viwat.v@chula.ac.th>
1 parent 519b459 commit e7c69b2

2 files changed

Lines changed: 227 additions & 0 deletions

File tree

‎app/test/unit/github_repository_client_test.dart‎

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -407,6 +407,51 @@ void main() {
407407
);
408408
}
409409
});
410+
411+
test('accepts 512 direct entries and rejects a 513th', () async {
412+
Map<String, Object?> treeWithEntries(int count) => <String, Object?>{
413+
'sha': _rootTreeSha,
414+
'truncated': false,
415+
'tree': <Object?>[
416+
for (int index = 0; index < count; index += 1)
417+
<String, Object?>{
418+
'path': 'example_$index.py',
419+
'mode': '100644',
420+
'type': 'blob',
421+
'sha': index.toRadixString(16).padLeft(40, '0'),
422+
'size': 0,
423+
},
424+
],
425+
};
426+
427+
final GithubRepositoryClient atLimit = GithubRepositoryClient(
428+
httpClient: MockClient(
429+
(http.Request request) async =>
430+
http.Response(jsonEncode(treeWithEntries(512)), 200),
431+
),
432+
);
433+
final GithubDirectory accepted = await atLimit.listDirectory(
434+
_pinned(),
435+
treeSha: _rootTreeSha,
436+
remotePath: '',
437+
);
438+
expect(accepted.entries, hasLength(512));
439+
440+
final GithubRepositoryClient aboveLimit = GithubRepositoryClient(
441+
httpClient: MockClient(
442+
(http.Request request) async =>
443+
http.Response(jsonEncode(treeWithEntries(513)), 200),
444+
),
445+
);
446+
await expectLater(
447+
aboveLimit.listDirectory(
448+
_pinned(),
449+
treeSha: _rootTreeSha,
450+
remotePath: '',
451+
),
452+
throwsA(_githubFailure(GithubFailureKind.malformedResponse)),
453+
);
454+
});
410455
});
411456

412457
group('GithubRepositoryClient blob reads', () {
@@ -505,6 +550,65 @@ void main() {
505550
});
506551

507552
group('GithubRepositoryClient failures', () {
553+
test(
554+
'cancels response bodies rejected by status or declared length',
555+
() async {
556+
final List<({int status, int? length, GithubFailureKind kind})> cases =
557+
<({int status, int? length, GithubFailureKind kind})>[
558+
(
559+
status: HttpStatus.notFound,
560+
length: null,
561+
kind: GithubFailureKind.notFound,
562+
),
563+
(
564+
status: HttpStatus.ok,
565+
length: 1024 * 1024 + 1,
566+
kind: GithubFailureKind.malformedResponse,
567+
),
568+
];
569+
570+
for (final variant in cases) {
571+
final int status = variant.status;
572+
final int? length = variant.length;
573+
final GithubFailureKind kind = variant.kind;
574+
bool bodyCancelled = false;
575+
final StreamController<List<int>> body = StreamController<List<int>>(
576+
onCancel: () {
577+
bodyCancelled = true;
578+
},
579+
);
580+
final MockClient httpClient = MockClient.streaming((
581+
http.BaseRequest request,
582+
http.ByteStream requestBody,
583+
) async {
584+
return http.StreamedResponse(
585+
body.stream,
586+
status,
587+
contentLength: length,
588+
);
589+
});
590+
final GithubRepositoryClient api = GithubRepositoryClient(
591+
httpClient: httpClient,
592+
);
593+
594+
try {
595+
await expectLater(
596+
api.resolve(_locator(), ref: 'main'),
597+
throwsA(_githubFailure(kind)),
598+
);
599+
expect(
600+
bodyCancelled,
601+
isTrue,
602+
reason: 'HTTP $status with Content-Length $length',
603+
);
604+
} finally {
605+
unawaited(body.close());
606+
httpClient.close();
607+
}
608+
}
609+
},
610+
);
611+
508612
test('maps HTTP status classes to stable failure kinds', () async {
509613
const Map<int, GithubFailureKind> cases = <int, GithubFailureKind>{
510614
404: GithubFailureKind.notFound,

‎app/test/widget/github_import_view_test.dart‎

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,20 @@ final class _HoldingPutConnection extends RecordingConnection {
195195
}
196196
}
197197

198+
final class _HoldingReviewConnection extends RecordingConnection {
199+
_HoldingReviewConnection() : super(initial: ConnState.ready);
200+
201+
final Completer<void> listStarted = Completer<void>();
202+
final Completer<void> releaseList = Completer<void>();
203+
204+
@override
205+
Future<DirectoryListing> listDirWithMetadata(String path) async {
206+
if (!listStarted.isCompleted) listStarted.complete();
207+
await releaseList.future;
208+
return super.listDirWithMetadata(path);
209+
}
210+
}
211+
198212
Key _entryKey(String remotePath) => ValueKey<String>('githubEntry_$remotePath');
199213

200214
Key _selectionKey(String remotePath) =>
@@ -753,6 +767,115 @@ void main() {
753767
);
754768
});
755769

770+
testWidgets('review cancellation cannot bypass a blob retry deadline', (
771+
WidgetTester tester,
772+
) async {
773+
final RecordingConnection connection = RecordingConnection(
774+
initial: ConnState.ready,
775+
);
776+
final _FakeGithubApi api = _FakeGithubApi(
777+
entries: const <GithubEntry>[_helloEntry],
778+
fetchFailuresRemaining: 1,
779+
fetchFailure: const GithubFailure(
780+
GithubFailureKind.rateLimited,
781+
retryAfter: Duration(seconds: 60),
782+
),
783+
);
784+
addTearDown(connection.dispose);
785+
786+
await _openImport(tester, connection, api);
787+
await _browse(tester);
788+
await tester.tap(find.byKey(_selectionKey('hello.py')));
789+
await tester.pump();
790+
await tester.tap(find.byKey(kGithubReviewButtonKey));
791+
await tester.pumpAndSettle();
792+
await tester.tap(find.byKey(kGithubCommitButtonKey));
793+
await tester.pumpAndSettle();
794+
expect(api.fetchedRemotePaths, <String>['hello.py']);
795+
796+
await tester.tap(
797+
find.widgetWithText(TextButton, l10nOf(tester).commonCancel),
798+
);
799+
await tester.pumpAndSettle();
800+
await tester.tap(find.byKey(kGithubReviewButtonKey));
801+
await tester.pumpAndSettle();
802+
803+
expect(
804+
tester
805+
.widget<FilledButton>(find.byKey(kGithubCommitButtonKey))
806+
.onPressed,
807+
isNull,
808+
);
809+
expect(api.fetchedRemotePaths, <String>['hello.py']);
810+
811+
await tester.pump(const Duration(seconds: 60));
812+
expect(
813+
tester
814+
.widget<FilledButton>(find.byKey(kGithubCommitButtonKey))
815+
.onPressed,
816+
isNotNull,
817+
);
818+
});
819+
820+
testWidgets('folder navigation cannot bypass a retry deadline', (
821+
WidgetTester tester,
822+
) async {
823+
final RecordingConnection connection = RecordingConnection(
824+
initial: ConnState.ready,
825+
);
826+
final _FakeGithubApi api = _FakeGithubApi(
827+
childListFailure: const GithubFailure(
828+
GithubFailureKind.rateLimited,
829+
retryAfter: Duration(seconds: 60),
830+
),
831+
);
832+
addTearDown(connection.dispose);
833+
834+
await _openImport(tester, connection, api);
835+
await _browse(tester);
836+
await tester.tap(find.byKey(_entryKey('nested')));
837+
await tester.pumpAndSettle();
838+
expect(api.listedRemotePaths, <String>['', 'nested']);
839+
840+
await tester.tap(find.byKey(_entryKey('nested')));
841+
await tester.pumpAndSettle();
842+
expect(api.listedRemotePaths, <String>['', 'nested']);
843+
844+
await tester.pump(const Duration(seconds: 60));
845+
await tester.tap(find.byKey(_entryKey('nested')));
846+
await tester.pumpAndSettle();
847+
expect(api.listedRemotePaths, <String>['', 'nested', 'nested']);
848+
});
849+
850+
testWidgets('board target review has a distinct visible phase', (
851+
WidgetTester tester,
852+
) async {
853+
final _HoldingReviewConnection connection = _HoldingReviewConnection();
854+
final _FakeGithubApi api = _FakeGithubApi(
855+
entries: const <GithubEntry>[_helloEntry],
856+
);
857+
addTearDown(() async {
858+
if (!connection.releaseList.isCompleted) {
859+
connection.releaseList.complete();
860+
}
861+
await connection.dispose();
862+
});
863+
864+
await _openImport(tester, connection, api);
865+
await _browse(tester);
866+
await tester.tap(find.byKey(_selectionKey('hello.py')));
867+
await tester.pump();
868+
await tester.tap(find.byKey(kGithubReviewButtonKey));
869+
await connection.listStarted.future;
870+
await tester.pump();
871+
872+
expect(find.text('Checking board targets…'), findsOneWidget);
873+
874+
connection.releaseList.complete();
875+
await tester.pumpAndSettle();
876+
expect(find.byKey(kGithubCommitButtonKey), findsOneWidget);
877+
});
878+
756879
testWidgets('review marks an existing regular-file target before consent', (
757880
WidgetTester tester,
758881
) async {

0 commit comments

Comments
 (0)