Skip to content

Commit 0e804ed

Browse files
elshelsh
authored
Validate module alias input values (#4215)
Add valid identifier checks for module aliases rdar://89836778
1 parent 799db76 commit 0e804ed

File tree

3 files changed

+115
-2
lines changed

3 files changed

+115
-2
lines changed

‎Sources/Basics/String+Extensions.swift

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,4 +25,9 @@ extension String {
2525
}
2626
return self
2727
}
28+
29+
public var isValidIdentifier: Bool {
30+
guard !isEmpty else { return false }
31+
return allSatisfy { $0.isLetter || $0.isNumber || $0 == "_" }
32+
}
2833
}

‎Sources/PackageLoading/PackageBuilder.swift

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,9 @@ public enum ModuleError: Swift.Error {
3333
/// The artifact for the binary target could not be found.
3434
case artifactNotFound(targetName: String, expectedArtifactName: String)
3535

36+
/// Invalid module alias.
37+
case invalidModuleAlias(originalName: String, newName: String)
38+
3639
/// Invalid custom path.
3740
case invalidCustomPath(target: String, path: String)
3841

@@ -84,6 +87,8 @@ extension ModuleError: CustomStringConvertible {
8487
return "Source files for target \(target) should be located under '\(folderName)/\(target)', or a custom sources path can be set with the 'path' property in Package.swift"
8588
case .artifactNotFound(let targetName, let expectedArtifactName):
8689
return "binary target '\(targetName)' could not be mapped to an artifact with expected name '\(expectedArtifactName)'"
90+
case .invalidModuleAlias(let originalName, let newName):
91+
return "empty or invalid module alias; ['\(originalName)': '\(newName)']"
8792
case .invalidLayout(let type):
8893
return "package has unsupported layout; \(type)"
8994
case .invalidManifestConfig(let package, let message):
@@ -645,8 +650,8 @@ public final class PackageBuilder {
645650
let manifestTarget = manifest.targetMap[potentialModule.name]
646651

647652
// Get the dependencies of this target.
648-
let dependencies: [Target.Dependency] = manifestTarget.map {
649-
$0.dependencies.compactMap { dependency in
653+
let dependencies: [Target.Dependency] = try manifestTarget.map {
654+
try $0.dependencies.compactMap { dependency in
650655
switch dependency {
651656
case .target(let name, let condition):
652657
// We don't create an object for targets which have no sources.
@@ -655,6 +660,7 @@ public final class PackageBuilder {
655660
return .target(target, conditions: buildConditions(from: condition))
656661

657662
case .product(let name, let package, let moduleAliases, let condition):
663+
try validateModuleAliases(moduleAliases)
658664
return .product(
659665
.init(name: name, package: package, moduleAliases: moduleAliases),
660666
conditions: buildConditions(from: condition)
@@ -720,6 +726,18 @@ public final class PackageBuilder {
720726
}
721727
}
722728

729+
/// Validates module alias key and value pairs and throws an error if empty or contains invalid characters.
730+
private func validateModuleAliases(_ aliases: [String: String]?) throws {
731+
guard let aliases = aliases else { return }
732+
for (aliasKey, aliasValue) in aliases {
733+
if !aliasKey.isValidIdentifier ||
734+
!aliasValue.isValidIdentifier ||
735+
aliasKey == aliasValue {
736+
throw ModuleError.invalidModuleAlias(originalName: aliasKey, newName: aliasValue)
737+
}
738+
}
739+
}
740+
723741
/// Private function that constructs a single Target object for the potential target.
724742
private func createTarget(
725743
potentialModule: PotentialModule,

‎Tests/BuildTests/BuildPlanTests.swift

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1685,6 +1685,96 @@ final class BuildPlanTests: XCTestCase {
16851685
}
16861686
}
16871687

1688+
func testModuleAliasingEmptyAlias() throws {
1689+
let fs = InMemoryFileSystem(emptyFiles:
1690+
"/thisPkg/Sources/exe/main.swift",
1691+
"/thisPkg/Sources/Logging/file.swift",
1692+
"/fooPkg/Sources/Logging/fileLogging.swift"
1693+
)
1694+
1695+
let observability = ObservabilitySystem.makeForTesting()
1696+
let _ = try loadPackageGraph(
1697+
fileSystem: fs,
1698+
manifests: [
1699+
Manifest.createRootManifest(
1700+
name: "fooPkg",
1701+
path: .init("/fooPkg"),
1702+
products: [
1703+
ProductDescription(name: "Foo", type: .library(.automatic), targets: ["Logging"]),
1704+
],
1705+
targets: [
1706+
TargetDescription(name: "Logging", dependencies: []),
1707+
]),
1708+
Manifest.createRootManifest(
1709+
name: "thisPkg",
1710+
path: .init("/thisPkg"),
1711+
dependencies: [
1712+
.localSourceControl(path: .init("/fooPkg"), requirement: .upToNextMajor(from: "1.0.0")),
1713+
],
1714+
targets: [
1715+
TargetDescription(name: "exe",
1716+
dependencies: ["Logging",
1717+
.product(name: "Foo",
1718+
package: "fooPkg",
1719+
moduleAliases: ["Logging": ""]
1720+
),
1721+
]),
1722+
TargetDescription(name: "Logging", dependencies: []),
1723+
]),
1724+
],
1725+
observabilityScope: observability.topScope
1726+
)
1727+
1728+
testDiagnostics(observability.diagnostics) { result in
1729+
result.check(diagnostic: .contains("empty or invalid module alias; ['Logging': '']"), severity: .error)
1730+
}
1731+
}
1732+
1733+
func testModuleAliasingInvalidIdentifierAlias() throws {
1734+
let fs = InMemoryFileSystem(emptyFiles:
1735+
"/thisPkg/Sources/exe/main.swift",
1736+
"/thisPkg/Sources/Logging/file.swift",
1737+
"/fooPkg/Sources/Logging/fileLogging.swift"
1738+
)
1739+
1740+
let observability = ObservabilitySystem.makeForTesting()
1741+
let _ = try loadPackageGraph(
1742+
fileSystem: fs,
1743+
manifests: [
1744+
Manifest.createRootManifest(
1745+
name: "fooPkg",
1746+
path: .init("/fooPkg"),
1747+
products: [
1748+
ProductDescription(name: "Foo", type: .library(.automatic), targets: ["Logging"]),
1749+
],
1750+
targets: [
1751+
TargetDescription(name: "Logging", dependencies: []),
1752+
]),
1753+
Manifest.createRootManifest(
1754+
name: "thisPkg",
1755+
path: .init("/thisPkg"),
1756+
dependencies: [
1757+
.localSourceControl(path: .init("/fooPkg"), requirement: .upToNextMajor(from: "1.0.0")),
1758+
],
1759+
targets: [
1760+
TargetDescription(name: "exe",
1761+
dependencies: ["Logging",
1762+
.product(name: "Foo",
1763+
package: "fooPkg",
1764+
moduleAliases: ["Logging": "P$0%^#@"]
1765+
),
1766+
]),
1767+
TargetDescription(name: "Logging", dependencies: []),
1768+
]),
1769+
],
1770+
observabilityScope: observability.topScope
1771+
)
1772+
1773+
testDiagnostics(observability.diagnostics) { result in
1774+
result.check(diagnostic: .contains("empty or invalid module alias; ['Logging': 'P$0%^#@']"), severity: .error)
1775+
}
1776+
}
1777+
16881778
func testModuleAliasingDirectDeps() throws {
16891779
let fs = InMemoryFileSystem(emptyFiles:
16901780
"/thisPkg/Sources/exe/main.swift",

0 commit comments

Comments
 (0)