Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions buildSrc/src/main/kotlin/Versions.kt
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ class Versions {

internal var _gradleVersion: String? = null

const val agpVersion = "4.2.0-alpha13"
const val kotlinVersion = "1.3.72"
const val agpVersion = "8.3.0"
const val kotlinVersion = "1.7.20"
const val pluginVersion = "0.2"
const val pluginArtifactId = "project-replicator"
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ import org.gradle.api.file.ConfigurableFileCollection
import org.gradle.api.file.FileVisitDetails
import org.gradle.api.file.RegularFileProperty
import org.gradle.api.file.ReproducibleFileVisitor
import org.gradle.api.plugins.JavaPluginConvention
import org.gradle.api.plugins.JavaPluginExtension
import org.gradle.api.provider.ListProperty
import org.gradle.api.provider.Property
import org.gradle.api.tasks.*
Expand Down Expand Up @@ -279,9 +279,10 @@ abstract class GatherModuleInfoTask : DefaultTask() {
task.androidInputs.disallowChanges()

if (appliedPlugins.containsJava()) {
val javaConvention = project.convention.findPlugin(JavaPluginConvention::class.java)
if (javaConvention != null) {
val mainSrcSet = javaConvention.sourceSets.findByName(SourceSet.MAIN_SOURCE_SET_NAME)
// Use the modern JavaPluginExtension instead of deprecated Convention
val javaExtension = project.extensions.findByType(JavaPluginExtension::class.java)
if (javaExtension != null) {
val mainSrcSet = javaExtension.sourceSets.findByName(SourceSet.MAIN_SOURCE_SET_NAME)
// Java source files
mainSrcSet?.allSource?.srcDirs?.let {
task.javaSourceSets.from(it)
Expand Down Expand Up @@ -324,11 +325,28 @@ private fun gatherDependencies(config: Configuration): List<DependenciesInfoInpu

for (dependency in config.dependencies) {
val dep = when (dependency) {
is ProjectDependency -> DependenciesInfoInputs(
type = DependencyType.MODULE,
dependency = dependency.dependencyProject.path,
scope = config.name
)
is ProjectDependency -> {
// Reconstruct the project path from group and name
// The group uses dots (e.g., "slack-android-ng.apps") but paths use colons (e.g., ":apps")
val group = dependency.group
val name = dependency.name
val projectPath = if (group != null && group != name) {
// Extract path from group by converting dots to colons and removing root project name
val pathFromGroup = group.substringAfter('.', "").replace('.', ':')
if (pathFromGroup.isNotEmpty()) {
":$pathFromGroup:$name"
} else {
":$name"
}
} else {
":$name"
}
Comment on lines +333 to +343

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This logic for reconstructing the project path is a bit complex and contains some redundant code due to the nested if-else structure and repeated else blocks. It can be refactored to be more linear and easier to read, which will improve maintainability.

                val projectPath = (if (group != null && group != name) {
                    // Extract path from group by converting dots to colons and removing root project name
                    group.substringAfter('.', "").replace('.', ':')
                } else {
                    ""
                }).let { pathFromGroup ->
                    if (pathFromGroup.isNotEmpty()) {
                        ":$pathFromGroup:$name"
                    } else {
                        ":$name"
                    }
                }

DependenciesInfoInputs(
type = DependencyType.MODULE,
dependency = projectPath,
scope = config.name
)
}
is ModuleDependency -> DependenciesInfoInputs(
type = DependencyType.EXTERNAL_LIBRARY,
dependency = "${dependency.group}:${dependency.name}:${dependency.version}",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
package com.android.gradle.replicator

import com.android.gradle.replicator.model.internal.filedata.*
import com.android.utils.XmlUtils
import org.xml.sax.Attributes
import org.xml.sax.InputSource
import org.xml.sax.Locator
import org.xml.sax.SAXParseException
import org.xml.sax.helpers.DefaultHandler
import java.io.File
import java.io.StringReader
import javax.xml.XMLConstants
import javax.xml.parsers.SAXParserFactory

// Method to create the correct data class. Need to make this so it scans the files appropriately
Expand Down Expand Up @@ -114,10 +114,19 @@ fun parseValuesFile(valuesFile: File): ValuesMap {
}
}

val factory = SAXParserFactory.newInstance()
// Parse the input
XmlUtils.configureSaxFactory(factory, false, false)
val saxParser = XmlUtils.createSaxParser(factory)
val factory = SAXParserFactory.newInstance().apply {
// Configure safely without external entity processing
isNamespaceAware = false
isValidating = false
try {
setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true)
setFeature("http://xml.org/sax/features/external-general-entities", false)
setFeature("http://xml.org/sax/features/external-parameter-entities", false)
} catch (e: Exception) {
// Ignore if features not supported
}
Comment on lines +125 to +127

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Catching a generic Exception and silently ignoring it can hide potential configuration issues. While the intention is to ignore unsupported features, this could also swallow other unexpected errors. It would be better to log the exception to provide visibility into any problems during SAX parser configuration, especially since this is security-related configuration.

        } catch (e: Exception) {
            // Ignore if features not supported, but log a warning.
            println("Warning: Failed to set security features on SAX parser: ${e.message}")
        }

}
val saxParser = factory.newSAXParser()
try {
saxParser.parse(InputSource(StringReader(valuesFile.readText())), handler)
} catch (e: SAXParseException) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import com.android.build.api.dsl.DynamicFeatureBuildFeatures
import com.android.build.api.dsl.LibraryBuildFeatures
import com.android.build.api.dsl.TestBuildFeatures
import com.android.build.gradle.BaseExtension
import com.android.build.gradle.internal.plugins.BasePlugin
import com.android.gradle.replicator.model.internal.DefaultAndroidInfo
import com.android.gradle.replicator.model.internal.DefaultBuildFeaturesInfo
import org.gradle.api.Project
Expand All @@ -39,14 +38,25 @@ interface AndroidCollector {
fun collectInfo(project: Project) : AndroidInfoInputs?
}

// Android plugin IDs to check for
private val ANDROID_PLUGIN_IDS = listOf(
"com.android.application",
"com.android.library",
"com.android.dynamic-feature",
"com.android.test"
)

/**
* Basic implementation of collector to start with.
*/
class DefaultAndroidCollector : AndroidCollector {
private lateinit var project: Project
override fun collectInfo(project: Project): AndroidInfoInputs? {
// load the BasePlugin by reflection in case it does not exist so that we dont fail with ClassNotFoundException
return project.plugins.withType(BasePlugin::class.java).firstOrNull()?.let {
// Check for Android plugins by ID instead of using internal BasePlugin class
val hasAndroidPlugin = ANDROID_PLUGIN_IDS.any { project.plugins.hasPlugin(it) }
if (!hasAndroidPlugin) return null

return run {
this.project = project

// if we are here there is indeed an Android plugin applied
Expand Down