Add a Pub validator for READMEs that are invalid utf-8.

BUG=7045

Review URL: https://codereview.chromium.org//12090081

git-svn-id: https://dart.googlecode.com/svn/branches/bleeding_edge/dart@17948 260f80e4-7a28-3924-810f-c04153c831b5

utils/pub/io.dart

@@ -85,6 +85,16 @@ Future<String> readTextFile(file) {
       });
 }
 
+/// Reads the contents of the binary file [file], which can either be a [String]
+/// or a [File].
+List<int> readBinaryFile(file) {
+  var path = _getPath(file);
+  log.io("Reading binary file $path.");
+  var contents = new File(path).readAsBytesSync();
+  log.io("Read ${contents.length} bytes from $path.");
+  return contents;
+}
+
 /// Creates [file] (which can either be a [String] or a [File]), and writes
 /// [contents] to it. Completes when the file is written and closed.
 ///
@@ -109,6 +119,20 @@ Future<File> writeTextFile(file, String contents, {dontLogContents: false}) {
   });
 }
 
+/// Creates [file] (which can either be a [String] or a [File]), and writes
+/// [contents] to it.
+File writeBinaryFile(file, List<int> contents) {
+  var path = _getPath(file);
+  file = new File(path);
+
+  log.io("Writing ${contents.length} bytes to binary file $path.");
+  file.openSync(FileMode.WRITE)
+      ..writeListSync(contents, 0, contents.length)
+      ..closeSync();
+  log.fine("Wrote text file $path.");
+  return file;
+}
+
 /// Asynchronously deletes [file], which can be a [String] or a [File]. Returns
 /// a [Future] that completes when the deletion is done.
 Future<File> deleteFile(file) {

utils/pub/package.dart

@@ -11,6 +11,8 @@ import 'source.dart';
 import 'source_registry.dart';
 import 'version.dart';
 
+final _README_REGEXP = new RegExp(r"^README($|\.)", caseSensitive: false);
+
 /// A named, versioned, unit of code and resource reuse.
 class Package {
   /// Loads the package whose root directory is [packageDir]. [name] is the
@@ -58,6 +60,26 @@ class Package {
   /// specified in the pubspec when this package depends on another.
   Collection<PackageRef> get dependencies => pubspec.dependencies;
 
+  /// Returns the path to the README file at the root of the entrypoint, or null
+  /// if no README file is found. If multiple READMEs are found, this uses the
+  /// same conventions as pub.dartlang.org for choosing the primary one: the
+  /// README with the fewest extensions that is lexically ordered first is
+  /// chosen.
+  Future<String> get readmePath {
+    return listDir(dir).then((entries) {
+      var readmes = entries.where((entry) => entry.contains(_README_REGEXP));
+      if (readmes.isEmpty) return;
+
+      return readmes.min((readme1, readme2) {
+        var extensions1 = ".".allMatches(readme1).length;
+        var extensions2 = ".".allMatches(readme2).length;
+        var comparison = extensions1.compareTo(extensions2);
+        if (comparison != 0) return comparison;
+        return readme1.compareTo(readme2);
+      });
+    });
+  }
+
   /// Constructs a package with the given pubspec. The package will have no
   /// directory associated with it.
   Package.inMemory(this.pubspec)

utils/pub/validator.dart

@@ -18,6 +18,7 @@ import 'validator/lib.dart';
 import 'validator/license.dart';
 import 'validator/name.dart';
 import 'validator/pubspec_field.dart';
+import 'validator/utf8_readme.dart';
 
 /// The base class for validators that check whether a package is fit for
 /// uploading. Each validator should override [errors], [warnings], or both to
@@ -52,7 +53,8 @@ abstract class Validator {
       new PubspecFieldValidator(entrypoint),
       new DependencyValidator(entrypoint),
       new DirectoryValidator(entrypoint),
-      new CompiledDartdocValidator(entrypoint)
+      new CompiledDartdocValidator(entrypoint),
+      new Utf8ReadmeValidator(entrypoint)
     ];
 
     return Future.wait(validators.map((validator) => validator.validate()))

utils/pub/validator/utf8_readme.dart

@@ -0,0 +1,39 @@
+// Copyright (c) 2013, the Dart project authors.  Please see the AUTHORS file
+// 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.
+
+library utf8_readme_validator;
+
+import 'dart:async';
+import 'dart:utf';
+
+import '../../../pkg/path/lib/path.dart' as path;
+
+import '../entrypoint.dart';
+import '../io.dart';
+import '../utils.dart';
+import '../validator.dart';
+
+/// Validates that a package's README is valid utf-8.
+class Utf8ReadmeValidator extends Validator {
+  Utf8ReadmeValidator(Entrypoint entrypoint)
+    : super(entrypoint);
+
+  Future validate() {
+    return entrypoint.root.readmePath.then((readme) {
+      if (readme == null) return;
+      var bytes = readBinaryFile(readme);
+      try {
+        // The second and third arguments here are the default values. The
+        // fourth tells [decodeUtf8] to throw an ArgumentError if `bytes` isn't
+        // valid utf-8.
+        decodeUtf8(bytes, 0, null, null);
+      } on ArgumentError catch (_) {
+        warnings.add("$readme contains invalid UTF-8.\n"
+            "This will cause it to be displayed incorrectly on "
+                "pub.dartlang.org.");
+      }
+    });
+  }
+}
+

utils/tests/pub/test_pub.dart

@@ -14,6 +14,7 @@ import 'dart:io';
 import 'dart:json' as json;
 import 'dart:math';
 import 'dart:uri';
+import 'dart:utf';
 
 import '../../../pkg/http/lib/testing.dart';
 import '../../../pkg/oauth2/lib/oauth2.dart' as oauth2;
@@ -49,6 +50,10 @@ initConfig() {
 FileDescriptor file(Pattern name, String contents) =>
     new FileDescriptor(name, contents);
 
+/// Creates a new [FileDescriptor] with [name] and [contents].
+FileDescriptor binaryFile(Pattern name, List<int> contents) =>
+    new FileDescriptor.bytes(name, contents);
+
 /// Creates a new [DirectoryDescriptor] with [name] and [contents].
 DirectoryDescriptor dir(Pattern name, [List<Descriptor> contents]) =>
     new DirectoryDescriptor(name, contents);
@@ -866,16 +871,20 @@ abstract class Descriptor {
 /// tree before running a test, and for validating that the file system matches
 /// some expectations after running it.
 class FileDescriptor extends Descriptor {
-  /// The text contents of the file.
-  final String contents;
+  /// The contents of the file, in bytes.
+  final List<int> contents;
 
-  FileDescriptor(Pattern name, this.contents) : super(name);
+  String get textContents => new String.fromCharCodes(contents);
+
+  FileDescriptor.bytes(Pattern name, this.contents) : super(name);
+
+  FileDescriptor(Pattern name, String contents) :
+      this.bytes(name, encodeUtf8(contents));
 
   /// Creates the file within [dir]. Returns a [Future] that is completed after
   /// the creation is done.
-  Future<File> create(dir) {
-    return writeTextFile(join(dir, _stringName), contents);
-  }
+  Future<File> create(dir) => new Future.immediate(null).then((_) =>
+      writeBinaryFile(join(dir, _stringName), contents));
 
   /// Deletes the file within [dir]. Returns a [Future] that is completed after
   /// the deletion is done.
@@ -887,10 +896,10 @@ class FileDescriptor extends Descriptor {
   Future validate(String path) {
     return _validateOneMatch(path, (file) {
       return readTextFile(file).then((text) {
-        if (text == contents) return null;
+        if (text == textContents) return null;
 
         throw new ExpectException(
-            'File $file should contain:\n\n$contents\n\n'
+            'File $file should contain:\n\n$textContents\n\n'
             'but contained:\n\n$text');
       });
     });
@@ -904,7 +913,7 @@ class FileDescriptor extends Descriptor {
     }
 
     var stream = new ListInputStream();
-    stream.write(contents.charCodes);
+    stream.write(contents);
     stream.markEndOfStream();
     return stream;
   }

utils/tests/pub/validator_test.dart

@@ -22,6 +22,7 @@ import '../../pub/validator/lib.dart';
 import '../../pub/validator/license.dart';
 import '../../pub/validator/name.dart';
 import '../../pub/validator/pubspec_field.dart';
+import '../../pub/validator/utf8_readme.dart';
 
 void expectNoValidationError(ValidatorCreator fn) {
   expectLater(schedulePackageValidation(fn), pairOf(isEmpty, isEmpty));
@@ -53,6 +54,9 @@ Validator name(Entrypoint entrypoint) => new NameValidator(entrypoint);
 Validator pubspecField(Entrypoint entrypoint) =>
   new PubspecFieldValidator(entrypoint);
 
+Validator utf8Readme(Entrypoint entrypoint) =>
+  new Utf8ReadmeValidator(entrypoint);
+
 void scheduleNormalPackage() => normalPackage.scheduleCreate();
 
 main() {
@@ -141,6 +145,14 @@ main() {
       ]).scheduleCreate();
       expectNoValidationError(compiledDartdoc);
     });
+
+    integration('has a non-primary readme with invalid utf-8', () {
+      dir(appPath, [
+        file("README", "Valid utf-8"),
+        binaryFile("README.invalid", [192])
+      ]).scheduleCreate();
+      expectNoValidationError(utf8Readme);
+    });
   });
 
   group('should consider a package invalid if it', () {
@@ -550,5 +562,12 @@ main() {
 
       expectValidationWarning(compiledDartdoc);
     });
+
+    test('has a README with invalid utf-8', () {
+      dir(appPath, [
+        binaryFile("README", [192])
+      ]).scheduleCreate();
+      expectValidationWarning(utf8Readme);
+    });
   });
 }