Skip to content

Compat: preserve/interpolate repository URLs; improved validation messaging #11140

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Compat: preserve/interpolate repository URLs; improved validation messaging #11140

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
42a95f2
Allow repository URL interpolation with improved validation
gnodet Sep 17, 2025
b986627
Apply code formatting fixes
gnodet Oct 6, 2025
c4578a5
Fix repository URL validation to allow supported expressions
gnodet Oct 6, 2025
3921d5f
Fix repository URL interpolation to support .uri expressions
gnodet Oct 6, 2025
8ffdecd
Apply spotless formatting to integration tests
gnodet Oct 6, 2025
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
22 changes: 4 additions & 18 deletions ...li/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/CompatibilityFixStrategy.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@
import org.apache.maven.api.di.Singleton;
import org.apache.maven.cling.invoker.mvnup.UpgradeContext;
import org.jdom2.Attribute;
import org.jdom2.Comment;
import org.jdom2.Content;
import org.jdom2.Document;
import org.jdom2.Element;
Expand Down Expand Up @@ -498,25 +497,12 @@ private boolean fixRepositoryExpressions(Element repositoriesElement, Namespace
Element urlElement = repository.getChild("url", namespace);
if (urlElement != null) {
String url = urlElement.getTextTrim();
if (url.contains("${")
&& !url.contains("${project.basedir}")
&& !url.contains("${project.rootDirectory}")) {
if (url.contains("${")) {
// Allow repository URL interpolation; do not disable.
// Keep a gentle warning to help users notice unresolved placeholders at build time.
String repositoryId = getChildText(repository, "id", namespace);
context.warning("Found unsupported expression in " + elementType + " URL (id: " + repositoryId
context.info("Detected interpolated expression in " + elementType + " URL (id: " + repositoryId
+ "): " + url);
context.warning(
"Maven 4 only supports ${project.basedir} and ${project.rootDirectory} expressions in repository URLs");

// Comment out the problematic repository
Comment comment =
new Comment(" Repository disabled due to unsupported expression in URL: " + url + " ");
Element parent = repository.getParentElement();
parent.addContent(parent.indexOf(repository), comment);
removeElementWithFormatting(repository);

context.detail("Fixed: " + "Commented out " + elementType + " with unsupported URL expression (id: "
+ repositoryId + ")");
fixed = true;
}
}
}
Expand Down
79 changes: 79 additions & 0 deletions impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
import java.util.concurrent.Executor;
import java.util.concurrent.Executors;
import java.util.concurrent.atomic.AtomicReference;
import java.util.function.BiFunction;
import java.util.function.Supplier;
import java.util.function.UnaryOperator;
import java.util.stream.Collectors;
Expand All @@ -63,12 +64,15 @@
import org.apache.maven.api.model.Activation;
import org.apache.maven.api.model.Dependency;
import org.apache.maven.api.model.DependencyManagement;
import org.apache.maven.api.model.DeploymentRepository;
import org.apache.maven.api.model.DistributionManagement;
import org.apache.maven.api.model.Exclusion;
import org.apache.maven.api.model.InputLocation;
import org.apache.maven.api.model.Mixin;
import org.apache.maven.api.model.Model;
import org.apache.maven.api.model.Parent;
import org.apache.maven.api.model.Profile;
import org.apache.maven.api.model.Repository;
import org.apache.maven.api.services.BuilderProblem;
import org.apache.maven.api.services.BuilderProblem.Severity;
import org.apache.maven.api.services.Interpolator;
Expand Down Expand Up @@ -1441,6 +1445,29 @@ Model doReadFileModel() throws ModelBuilderException {
model.getParent().getVersion()))
: null)
.build();
// Interpolate repository URLs
if (model.getProjectDirectory() != null) {
String basedir = model.getProjectDirectory().toString();
String basedirUri = model.getProjectDirectory().toUri().toString();
properties.put("basedir", basedir);
properties.put("project.basedir", basedir);
properties.put("project.basedir.uri", basedirUri);
}
try {
String root = request.getSession().getRootDirectory().toString();
String rootUri =
request.getSession().getRootDirectory().toUri().toString();
properties.put("project.rootDirectory", root);
properties.put("project.rootDirectory.uri", rootUri);
} catch (IllegalStateException e) {
}
UnaryOperator<String> callback = properties::get;
model = model.with()
.repositories(interpolateRepository(model.getRepositories(), callback))
.pluginRepositories(interpolateRepository(model.getPluginRepositories(), callback))
.profiles(map(model.getProfiles(), this::interpolateRepository, callback))
.distributionManagement(interpolateRepository(model.getDistributionManagement(), callback))
.build();
// Override model properties with user properties
Map<String, String> newProps = merge(model.getProperties(), session.getUserProperties());
if (newProps != null) {
Expand Down Expand Up @@ -1471,6 +1498,41 @@ Model doReadFileModel() throws ModelBuilderException {
return model;
}

private DistributionManagement interpolateRepository(
DistributionManagement distributionManagement, UnaryOperator<String> callback) {
return distributionManagement == null
? null
: distributionManagement
.with()
.repository((DeploymentRepository)
interpolateRepository(distributionManagement.getRepository(), callback))
.snapshotRepository((DeploymentRepository)
interpolateRepository(distributionManagement.getSnapshotRepository(), callback))
.build();
}

private Profile interpolateRepository(Profile profile, UnaryOperator<String> callback) {
return profile == null
? null
: profile.with()
.repositories(interpolateRepository(profile.getRepositories(), callback))
.pluginRepositories(interpolateRepository(profile.getPluginRepositories(), callback))
.build();
}

private List<Repository> interpolateRepository(List<Repository> repositories, UnaryOperator<String> callback) {
return map(repositories, this::interpolateRepository, callback);
}

private Repository interpolateRepository(Repository repository, UnaryOperator<String> callback) {
return repository == null
? null
: repository
.with()
.url(interpolator.interpolate(repository.getUrl(), callback))
.build();
}

/**
* Merges a list of model profiles with user-defined properties.
* For each property defined in both the model and user properties, the user property value
Expand Down Expand Up @@ -2340,4 +2402,21 @@ private Object getOuterRequest(Request<?> req) {
}
return req;
}

private static <T, A> List<T> map(List<T> resources, BiFunction<T, A, T> mapper, A argument) {
List<T> newResources = null;
if (resources != null) {
for (int i = 0; i < resources.size(); i++) {
T resource = resources.get(i);
T newResource = mapper.apply(resource, argument);
if (newResource != resource) {
if (newResources == null) {
newResources = new ArrayList<>(resources);
}
newResources.set(i, newResource);
}
}
}
return newResources;
}
}
131 changes: 77 additions & 54 deletions impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelValidator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -542,16 +542,6 @@ && equals(parent.getArtifactId(), model.getArtifactId())) {
validationLevel);
}

validateRawRepositories(
problems, model.getRepositories(), "repositories.repository.", EMPTY, validationLevel);

validateRawRepositories(
problems,
model.getPluginRepositories(),
"pluginRepositories.pluginRepository.",
EMPTY,
validationLevel);

Build build = model.getBuild();
if (build != null) {
validate20RawPlugins(problems, build.getPlugins(), "build.plugins.plugin.", EMPTY, validationLevel);
Expand Down Expand Up @@ -605,16 +595,6 @@ && equals(parent.getArtifactId(), model.getArtifactId())) {
validationLevel);
}

validateRawRepositories(
problems, profile.getRepositories(), prefix, "repositories.repository.", validationLevel);

validateRawRepositories(
problems,
profile.getPluginRepositories(),
prefix,
"pluginRepositories.pluginRepository.",
validationLevel);

BuildBase buildBase = profile.getBuild();
if (buildBase != null) {
validate20RawPlugins(problems, buildBase.getPlugins(), prefix, "plugins.plugin.", validationLevel);
Expand Down Expand Up @@ -685,6 +665,44 @@ && equals(parent.getArtifactId(), model.getArtifactId())) {
parent);
}
}

if (validationLevel > VALIDATION_LEVEL_MINIMAL) {
validateRawRepositories(
problems, model.getRepositories(), "repositories.repository.", EMPTY, validationLevel);

validateRawRepositories(
problems,
model.getPluginRepositories(),
"pluginRepositories.pluginRepository.",
EMPTY,
validationLevel);

for (Profile profile : model.getProfiles()) {
String prefix = "profiles.profile[" + profile.getId() + "].";

validateRawRepositories(
problems, profile.getRepositories(), prefix, "repositories.repository.", validationLevel);

validateRawRepositories(
problems,
profile.getPluginRepositories(),
prefix,
"pluginRepositories.pluginRepository.",
validationLevel);
}

DistributionManagement distMgmt = model.getDistributionManagement();
if (distMgmt != null) {
validateRawRepository(
problems, distMgmt.getRepository(), "distributionManagement.repository.", "", true);
validateRawRepository(
problems,
distMgmt.getSnapshotRepository(),
"distributionManagement.snapshotRepository.",
"",
true);
}
}
}

private void validate30RawProfileActivation(ModelProblemCollector problems, Activation activation, String prefix) {
Expand Down Expand Up @@ -1536,40 +1554,7 @@ private void validateRawRepositories(
Map<String, Repository> index = new HashMap<>();

for (Repository repository : repositories) {
validateStringNotEmpty(
prefix, prefix2, "id", problems, Severity.ERROR, Version.V20, repository.getId(), null, repository);

if (validateStringNotEmpty(
prefix,
prefix2,
"[" + repository.getId() + "].url",
problems,
Severity.ERROR,
Version.V20,
repository.getUrl(),
null,
repository)) {
// only allow ${basedir} and ${project.basedir}
Matcher matcher = EXPRESSION_NAME_PATTERN.matcher(repository.getUrl());
while (matcher.find()) {
String expr = matcher.group(1);
if (!("basedir".equals(expr)
|| "project.basedir".equals(expr)
|| expr.startsWith("project.basedir.")
|| "project.rootDirectory".equals(expr)
|| expr.startsWith("project.rootDirectory."))) {
addViolation(
problems,
Severity.ERROR,
Version.V40,
prefix + prefix2 + "[" + repository.getId() + "].url",
null,
"contains an unsupported expression (only expressions starting with 'project.basedir' or 'project.rootDirectory' are supported).",
repository);
break;
}
}
}
validateRawRepository(problems, repository, prefix, prefix2, false);

String key = repository.getId();

Expand All @@ -1593,6 +1578,44 @@ private void validateRawRepositories(
}
}

private void validateRawRepository(
ModelProblemCollector problems,
Repository repository,
String prefix,
String prefix2,
boolean allowEmptyUrl) {
if (repository == null) {
return;
}
validateStringNotEmpty(
prefix, prefix2, "id", problems, Severity.ERROR, Version.V20, repository.getId(), null, repository);

if (!allowEmptyUrl
&& validateStringNotEmpty(
prefix,
prefix2,
"[" + repository.getId() + "].url",
problems,
Severity.ERROR,
Version.V20,
repository.getUrl(),
null,
repository)) {
// Check for uninterpolated expressions - these should have been interpolated by now
Matcher matcher = EXPRESSION_NAME_PATTERN.matcher(repository.getUrl());
if (matcher.find()) {
addViolation(
problems,
Severity.ERROR,
Version.V40,
prefix + prefix2 + "[" + repository.getId() + "].url",
null,
"contains an uninterpolated expression.",
repository);
}
}
}

private void validate20EffectiveRepository(
ModelProblemCollector problems, Repository repository, String prefix, int validationLevel) {
if (repository != null) {
Expand Down
19 changes: 13 additions & 6 deletions impl/maven-impl/src/test/java/org/apache/maven/impl/model/DefaultModelValidatorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -365,7 +365,7 @@ void testEmptyPluginVersion() throws Exception {
@Test
void testMissingRepositoryId() throws Exception {
SimpleProblemCollector result =
validateFile("missing-repository-id-pom.xml", ModelValidator.VALIDATION_LEVEL_STRICT);
validateRaw("missing-repository-id-pom.xml", ModelValidator.VALIDATION_LEVEL_STRICT);

assertViolations(result, 0, 4, 0);

Expand Down Expand Up @@ -888,16 +888,23 @@ void testParentVersionRELEASE() throws Exception {
@Test
void repositoryWithExpression() throws Exception {
SimpleProblemCollector result = validateFile("raw-model/repository-with-expression.xml");
assertViolations(result, 0, 1, 0);
assertEquals(
"'repositories.repository.[repo].url' contains an unsupported expression (only expressions starting with 'project.basedir' or 'project.rootDirectory' are supported).",
result.getErrors().get(0));
// Interpolation in repository URLs is allowed; unresolved placeholders will fail later during resolution
assertViolations(result, 0, 0, 0);
}

@Test
void repositoryWithBasedirExpression() throws Exception {
SimpleProblemCollector result = validateRaw("raw-model/repository-with-basedir-expression.xml");
assertViolations(result, 0, 0, 0);
// This test runs on raw model without interpolation, so all expressions appear uninterpolated
// In the real flow, supported expressions would be interpolated before validation
assertViolations(result, 0, 3, 0);
}

@Test
void repositoryWithUnsupportedExpression() throws Exception {
SimpleProblemCollector result = validateRaw("raw-model/repository-with-unsupported-expression.xml");
// Unsupported expressions should cause validation errors
assertViolations(result, 0, 1, 0);
}

@Test
Expand Down
Loading
Loading