Skip to content

Commit 40f9df2

Browse files
authored
Improve diagnostics for file URLs with hostnames (#3630)
* Improved diagnostic for file URLs with hostnames * Add more tests for file URLs with hostnames
1 parent 7d02c35 commit 40f9df2

File tree

2 files changed

+46
-18
lines changed

2 files changed

+46
-18
lines changed

Sources/PackageLoading/ManifestJSONParser.swift

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -217,8 +217,16 @@ enum ManifestJSONParser {
217217
// FIXME: SwiftPM can't handle file locations with file:// scheme so we need to
218218
// strip that. We need to design a Location data structure for SwiftPM.
219219
let location = String(dependencyLocation.dropFirst(filePrefix.count))
220-
if location.first != "/" {
221-
throw ManifestParseError.invalidManifestFormat("file:// URLs cannot be relative, did you mean to use `.package(path:)`?", diagnosticFile: nil)
220+
let hostnameComponent = location.prefix(while: { $0 != "/" })
221+
guard hostnameComponent.isEmpty else {
222+
if hostnameComponent == ".." {
223+
throw ManifestParseError.invalidManifestFormat(
224+
"file:// URLs cannot be relative, did you mean to use '.package(path:)'?", diagnosticFile: nil
225+
)
226+
}
227+
throw ManifestParseError.invalidManifestFormat(
228+
"file:// URLs with hostnames are not supported, are you missing a '/'?", diagnosticFile: nil
229+
)
222230
}
223231
return AbsolutePath(location).pathString
224232
} else if URL.scheme(dependencyLocation) == nil {

Tests/PackageLoadingTests/PD_4_2_LoadingTests.swift

Lines changed: 36 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -497,23 +497,43 @@ class PackageDescription4_2LoadingTests: PackageDescriptionLoadingTests {
497497
}
498498
}
499499

500-
func testURLContainsNotAbsolutePath() throws {
501-
let manifest = """
502-
import PackageDescription
503-
let package = Package(
504-
name: "Trivial",
505-
dependencies: [
506-
.package(url: "file://../best", from: "1.0.0"),
507-
],
508-
targets: [
509-
.target(
510-
name: "foo",
511-
dependencies: []),
512-
]
513-
)
514-
"""
500+
func testFileURLsWithHostnames() throws {
501+
enum ExpectedError {
502+
case relativePath
503+
case unsupportedHostname
504+
505+
var manifestError: ManifestParseError {
506+
switch self {
507+
case .relativePath:
508+
return .invalidManifestFormat("file:// URLs cannot be relative, did you mean to use '.package(path:)'?", diagnosticFile: nil)
509+
case .unsupportedHostname:
510+
return .invalidManifestFormat("file:// URLs with hostnames are not supported, are you missing a '/'?", diagnosticFile: nil)
511+
}
512+
}
513+
}
515514

516-
XCTAssertManifestLoadThrows(ManifestParseError.invalidManifestFormat("file:// URLs cannot be relative, did you mean to use `.package(path:)`?", diagnosticFile: nil), manifest)
515+
let urls: [(String, ExpectedError)] = [
516+
("file://../best", .relativePath), // Possible attempt at a relative path.
517+
("file://somehost/bar", .unsupportedHostname), // Obviously non-local.
518+
("file://localhost/bar", .unsupportedHostname), // Local but non-trivial (e.g. on Windows, this is a UNC path).
519+
]
520+
for (url, expectedError) in urls {
521+
let manifest = """
522+
import PackageDescription
523+
let package = Package(
524+
name: "Trivial",
525+
dependencies: [
526+
.package(url: "\(url)", from: "1.0.0"),
527+
],
528+
targets: [
529+
.target(
530+
name: "foo",
531+
dependencies: []),
532+
]
533+
)
534+
"""
535+
XCTAssertManifestLoadThrows(expectedError.manifestError, manifest)
536+
}
517537
}
518538

519539
func testCacheInvalidationOnEnv() throws {

0 commit comments

Comments
 (0)