diff --git a/lib/src/hosted_source.dart b/lib/src/hosted_source.dart index 6fdc8e91328b745441897ab13fdceb0a8a724f4b..44f7195ea5ffe8cbd3bf098a2de3c09112fbf945 100644 --- a/lib/src/hosted_source.dart +++ b/lib/src/hosted_source.dart @@ -29,21 +29,20 @@ class HostedSource extends Source { final name = "hosted"; final shouldCache = true; - /// The URL of the default package repository. - static final defaultUrl = "https://pub.dartlang.org"; - /// Downloads a list of all versions of a package that are available from the /// site. Future<List<Version>> getVersions(String name, description) { - var parsed = _parseDescription(description); - var fullUrl = "${parsed.last}/packages/${parsed.first}.json"; + var url = _makeUrl(description, + (server, package) => "$server/packages/$package.json"); - return httpClient.read(fullUrl).then((body) { + log.io("Get versions from $url."); + return httpClient.read(url).then((body) { var doc = json.parse(body); return doc['versions'] .map((version) => new Version.parse(version)) .toList(); }).catchError((ex) { + var parsed = _parseDescription(description); _throwFriendlyError(ex, parsed.first, parsed.last); }); } @@ -51,13 +50,14 @@ class HostedSource extends Source { /// Downloads and parses the pubspec for a specific version of a package that /// is available from the site. Future<Pubspec> describe(PackageId id) { - var parsed = _parseDescription(id.description); - var fullUrl = "${parsed.last}/packages/${parsed.first}/versions/" - "${id.version}.yaml"; + var url = _makeVersionUrl(id, (server, package, version) => + "$server/packages/$package/versions/$version.yaml"); - return httpClient.read(fullUrl).then((yaml) { + log.io("Describe package at $url."); + return httpClient.read(url).then((yaml) { return new Pubspec.parse(null, yaml, systemCache.sources); }).catchError((ex) { + var parsed = _parseDescription(id.description); _throwFriendlyError(ex, id, parsed.last); }); } @@ -65,21 +65,19 @@ class HostedSource extends Source { /// Downloads a package from the site and unpacks it. Future<bool> install(PackageId id, String destPath) { return defer(() { - var parsedDescription = _parseDescription(id.description); - var name = parsedDescription.first; - var url = parsedDescription.last; - - var fullUrl = "$url/packages/$name/versions/${id.version}.tar.gz"; + var url = _makeVersionUrl(id, (server, package, version) => + "$server/packages/$package/versions/$version.tar.gz"); + log.io("Install package from $url."); log.message('Downloading $id...'); // Download and extract the archive to a temp directory. var tempDir = systemCache.createTempDir(); - return httpClient.send(new http.Request("GET", Uri.parse(fullUrl))) + return httpClient.send(new http.Request("GET", url)) .then((response) => response.stream) .then((stream) { return timeout(extractTarGz(stream, tempDir), HTTP_TIMEOUT, - 'fetching URL "$fullUrl"'); + 'fetching URL "$url"'); }).then((_) { // Now that the install has succeeded, move it to the real location in // the cache. This ensures that we don't leave half-busted ghost @@ -142,31 +140,57 @@ class HostedSource extends Source { throw asyncError; } - /// Parses the description for a package. - /// - /// If the package parses correctly, this returns a (name, url) pair. If not, - /// this throws a descriptive FormatException. - Pair<String, String> _parseDescription(description) { - if (description is String) { - return new Pair<String, String>(description, defaultUrl); - } +} - if (description is! Map) { - throw new FormatException( - "The description must be a package name or map."); - } +/// The URL of the default package repository. +final _defaultUrl = "https://pub.dartlang.org"; + +/// Parses [description] into its server and package name components, then +/// converts that to a Uri given [pattern]. Ensures the package name is +/// properly URL encoded. +Uri _makeUrl(description, String pattern(String server, String package)) { + var parsed = _parseDescription(description); + var server = parsed.last; + var package = encodeUriComponent(parsed.first); + return new Uri(pattern(server, package)); +} - if (!description.containsKey("name")) { - throw new FormatException( - "The description map must contain a 'name' key."); - } +/// Parses [id] into its server, package name, and version components, then +/// converts that to a Uri given [pattern]. Ensures the package name is +/// properly URL encoded. +Uri _makeVersionUrl(PackageId id, + String pattern(String server, String package, String version)) { + var parsed = _parseDescription(id.description); + var server = parsed.last; + var package = encodeUriComponent(parsed.first); + var version = encodeUriComponent(id.version.toString()); + return new Uri(pattern(server, package, version)); +} - var name = description["name"]; - if (name is! String) { - throw new FormatException("The 'name' key must have a string value."); - } +/// Parses the description for a package. +/// +/// If the package parses correctly, this returns a (name, url) pair. If not, +/// this throws a descriptive FormatException. +Pair<String, String> _parseDescription(description) { + if (description is String) { + return new Pair<String, String>(description, _defaultUrl); + } + + if (description is! Map) { + throw new FormatException( + "The description must be a package name or map."); + } + + if (!description.containsKey("name")) { + throw new FormatException( + "The description map must contain a 'name' key."); + } - var url = description.containsKey("url") ? description["url"] : defaultUrl; - return new Pair<String, String>(name, url); + var name = description["name"]; + if (name is! String) { + throw new FormatException("The 'name' key must have a string value."); } + + var url = description.containsKey("url") ? description["url"] : _defaultUrl; + return new Pair<String, String>(name, url); } diff --git a/lib/src/validator/name.dart b/lib/src/validator/name.dart index a0ca81ad65f03139533d42a8ecb0957071142d97..c183f9c0db927c6a6ffa128cb6439e5d5b2c22cf 100644 --- a/lib/src/validator/name.dart +++ b/lib/src/validator/name.dart @@ -65,17 +65,19 @@ class NameValidator extends Validator { } void _checkName(String name, String description, {bool isPackage}) { + // Packages names are more stringent than libraries. + var messages = isPackage ? errors : warnings; + if (name == "") { errors.add("$description may not be empty."); } else if (!new RegExp(r"^[a-zA-Z0-9_]*$").hasMatch(name)) { - warnings.add("$description may only contain letters, numbers, and " + messages.add("$description may only contain letters, numbers, and " "underscores.\n" "Using a valid Dart identifier makes the name usable in Dart code."); } else if (!new RegExp(r"^[a-zA-Z]").hasMatch(name)) { - warnings.add("$description must begin with letter.\n" + messages.add("$description must begin with letter.\n" "Using a valid Dart identifier makes the name usable in Dart code."); } else if (_RESERVED_WORDS.contains(name.toLowerCase())) { - var messages = isPackage ? errors : warnings; messages.add("$description may not be a reserved word in Dart.\n" "Using a valid Dart identifier makes the name usable in Dart code."); } else if (new RegExp(r"[A-Z]").hasMatch(name)) { diff --git a/test/install/hosted/install_test.dart b/test/install/hosted/install_test.dart index 1ed4f404e9f58878424cb6e2bb9663d3dfb45647..5bc120c3d7a9c2d4cc57e6b9e8e193bfd2adab3f 100644 --- a/test/install/hosted/install_test.dart +++ b/test/install/hosted/install_test.dart @@ -9,6 +9,7 @@ import 'dart:io'; import '../../test_pub.dart'; main() { + initConfig(); integration('installs a package from a pub server', () { servePackages([package("foo", "1.2.3")]); @@ -20,4 +21,15 @@ main() { cacheDir({"foo": "1.2.3"}).scheduleValidate(); packagesDir({"foo": "1.2.3"}).scheduleValidate(); }); + + integration('URL encodes the package name', () { + servePackages([]); + + appDir([dependency("bad name!", "1.2.3")]).scheduleCreate(); + + schedulePub(args: ['install'], + error: new RegExp('Could not find package "bad name!" at ' + 'http://localhost:'), + exitCode: 1); + }); } diff --git a/test/validator_test.dart b/test/validator_test.dart index 77d9c8652ef08bd19a26fc169f951072c3b4f1bf..4f59bbae58ffb211043970c9a05435e5d2452e92 100644 --- a/test/validator_test.dart +++ b/test/validator_test.dart @@ -284,12 +284,12 @@ main() { integration('has a package name with an invalid character', () { dir(appPath, [libPubspec("test-pkg", "1.0.0")]).scheduleCreate(); - expectValidationWarning(name); + expectValidationError(name); }); integration('has a package name that begins with a number', () { dir(appPath, [libPubspec("8ball", "1.0.0")]).scheduleCreate(); - expectValidationWarning(name); + expectValidationError(name); }); integration('has a package name that contains upper-case letters', () {