Add unit tests for osv validation (#6884)

app/lib/service/security_advisories/backend.dart

@@ -44,45 +44,8 @@ class SecurityAdvisoryBackend {
     return _db.lookupOrNull<SecurityAdvisory>(key);
   }
 
-  // TODO(zarah): add unit test for this.
-  List<String> _validateAdvisoryErrors(OSV osv) {
-    final errors = <String>[];
-
-    if (DateTime.parse(osv.modified)
-        .isAfter(clock.now().add(Duration(hours: 1)))) {
-      errors.add('Invalid modified date, cannot be a future date.');
-    }
-
-    if (osv.id.length > 255) {
-      errors.add('Invalid id, id too long (over 255 characters).');
-    }
-
-    final invalids = <int>[];
-    // Check that [osv.id] consists of printable ASCII.
-    osv.id.runes.forEach((element) {
-      if (element < 32 || element > 126) {
-        invalids.add(element);
-      }
-    });
-
-    if (invalids.isNotEmpty) {
-      errors.add('Invalid id, the "id" property must be printable ASCII.');
-    }
-
-    if (json.encode(osv.toJson()).length > 1024 * 500) {
-      errors.add('OSV too large (larger than 500 kB)');
-    }
-
-    return errors;
-  }
-
-  /// Sanity checks to ensure that we can store, lookup and update the advisory.
-  ///
-  /// We don't validate all fields, instead we aim to simply ensure that the
-  /// advisory is sufficiently sound that we can store it, look it up and update
-  /// it in the future.
   bool _isValidAdvisory(OSV osv) {
-    final errors = _validateAdvisoryErrors(osv);
+    final errors = sanityCheckOSV(osv);
     errors.forEach((error) => _logger.shout('[advisory-malformed]: $error'));
     return errors.isEmpty;
   }
@@ -131,3 +94,39 @@ class SecurityAdvisoryBackend {
     });
   }
 }
+
+/// Sanity checks to ensure that we can store, lookup and update the advisory.
+///
+/// We don't validate all fields, instead we aim to simply ensure that the
+/// advisory is sufficiently sound that we can store it, look it up and update
+/// it in the future.
+List<String> sanityCheckOSV(OSV osv) {
+  final errors = <String>[];
+
+  if (DateTime.parse(osv.modified)
+      .isAfter(clock.now().add(Duration(hours: 1)))) {
+    errors.add('Invalid modified date, cannot be a future date.');
+  }
+
+  if (osv.id.length > 255) {
+    errors.add('Invalid id, id too long (over 255 characters).');
+  }
+
+  final invalids = <int>[];
+  // Check that [osv.id] consists of printable ASCII.
+  osv.id.runes.forEach((element) {
+    if (element < 32 || element > 126) {
+      invalids.add(element);
+    }
+  });
+
+  if (invalids.isNotEmpty) {
+    errors.add('Invalid id, the "id" property must be printable ASCII.');
+  }
+
+  if (json.encode(osv.toJson()).length > 1024 * 500) {
+    errors.add('OSV too large (larger than 500 kB)');
+  }
+
+  return errors;
+}

app/test/security_advisory/security_advisory_test.dart

@@ -2,6 +2,7 @@
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
+import 'package:clock/clock.dart';
 import 'package:pub_dev/service/security_advisories/backend.dart';
 import 'package:pub_dev/service/security_advisories/models.dart';
 import 'package:test/test.dart';
@@ -79,4 +80,77 @@ void main() {
     expect(list3.length, 1);
     expect(list3.first.id, id);
   });
+  group('Validate osv', () {
+    test('Modified date should not be in the future', () async {
+      final firstTime = DateTime(2022).toIso8601String();
+      final futureTime = clock.now().add(Duration(days: 1)).toIso8601String();
+      final id = '123';
+      final osv = OSV(
+        schemaVersion: '1.2.3',
+        id: id,
+        modified: futureTime,
+        published: firstTime,
+        affected: [],
+      );
+
+      final errors = sanityCheckOSV(osv);
+      expect(errors, isNotEmpty);
+      expect(errors.first, contains('Invalid modified date'));
+    });
+
+    test('Id should be less than 255 characters', () async {
+      final firstTime = DateTime(2022).toIso8601String();
+      final longid =
+          '0123456789012345678901234567890123456789012345678901234567890123456789'
+          '0123456789012345678901234567890123456789012345678901234567890123456789'
+          '0123456789012345678901234567890123456789012345678901234567890123456789'
+          '01234567890123456789012345678901234567890123456789';
+      final osv2 = OSV(
+        schemaVersion: '1.2.3',
+        id: longid,
+        modified: firstTime,
+        published: firstTime,
+        affected: [],
+      );
+      final errors = sanityCheckOSV(osv2);
+      expect(errors, isNotEmpty);
+      expect(errors.first, contains('Invalid id'));
+    });
+
+    test('Id should be printable ASCII', () async {
+      final firstTime = DateTime(2022).toIso8601String();
+      final invalidId = '\n';
+      final osv3 = OSV(
+        schemaVersion: '1.2.3',
+        id: invalidId,
+        modified: firstTime,
+        published: firstTime,
+        affected: [],
+      );
+      final errors = sanityCheckOSV(osv3);
+      expect(errors, isNotEmpty);
+      expect(errors.first, contains('Invalid id'));
+    });
+
+    test('OSV size should be less than 500 kB', () async {
+      final firstTime = DateTime(2022).toIso8601String();
+      final id = '123';
+      final largeMap = <String, String>{};
+      for (int i = 0; i < 35000; i++) {
+        largeMap['$i'] = '$i';
+      }
+      final osv4 = OSV(
+        schemaVersion: '1.2.3',
+        id: id,
+        modified: firstTime,
+        published: firstTime,
+        affected: [],
+        databaseSpecific: largeMap,
+      );
+
+      final errors = sanityCheckOSV(osv4);
+      expect(errors, isNotEmpty);
+      expect(errors.first, contains('OSV too large'));
+    });
+  });
 }