From 3e38392021b1c5fe74c6ea797de8ec76a13f8524 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B3zsef=20Bart=C3=B3k?= Date: Tue, 21 Sep 2021 11:08:06 +0300 Subject: [PATCH 1/4] Fix buffer overflow error in KryoBackedDecoder Fixes #19523 --- .../internal/serialize/kryo/KryoBackedDecoder.java | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/subprojects/messaging/src/main/java/org/gradle/internal/serialize/kryo/KryoBackedDecoder.java b/subprojects/messaging/src/main/java/org/gradle/internal/serialize/kryo/KryoBackedDecoder.java index a68dbfeb7b76..71ecdb038f35 100644 --- a/subprojects/messaging/src/main/java/org/gradle/internal/serialize/kryo/KryoBackedDecoder.java +++ b/subprojects/messaging/src/main/java/org/gradle/internal/serialize/kryo/KryoBackedDecoder.java @@ -168,6 +168,8 @@ public void skipChunked() throws EOFException, IOException { public T decodeChunked(DecodeAction decodeAction) throws EOFException, Exception { if (nested == null) { nested = new KryoBackedDecoder(new InputStream() { + private int leftover = 0; + @Override public int read() throws IOException { throw new UnsupportedOperationException(); @@ -175,14 +177,21 @@ public int read() throws IOException { @Override public int read(byte[] buffer, int offset, int length) throws IOException { + if (leftover > 0) { + int count = Math.min(leftover, length); + leftover -= count; + readBytes(buffer, offset, count); + return count; + } + int count = readSmallInt(); if (count == 0) { // End of stream has been reached return -1; } if (count > length) { - // For now, assume same size buffers used to read and write - throw new UnsupportedOperationException(); + leftover = count - length; + count = length; } readBytes(buffer, offset, count); return count; From 69fa6596bad55070bba6aad6ad48b0e4d57ab3d4 Mon Sep 17 00:00:00 2001 From: Louis Jacomet Date: Wed, 16 Mar 2022 15:18:15 +0100 Subject: [PATCH 2/4] Fix nullability hints --- .../DefaultDependencyConstraintHandler.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/dsl/dependencies/DefaultDependencyConstraintHandler.java b/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/dsl/dependencies/DefaultDependencyConstraintHandler.java index 41e3e1b65f76..bdce57c84783 100644 --- a/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/dsl/dependencies/DefaultDependencyConstraintHandler.java +++ b/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/dsl/dependencies/DefaultDependencyConstraintHandler.java @@ -189,26 +189,26 @@ private DependencyConstraint doAdd(Configuration configuration, Object dependenc return dependency; } - private DependencyConstraint doAddProvider(Configuration configuration, Provider dependencyNotation, Action configureAction) { + private DependencyConstraint doAddProvider(Configuration configuration, Provider dependencyNotation, @Nullable Action configureAction) { Provider lazyConstraint = dependencyNotation.map(mapDependencyConstraintProvider(configureAction)); configuration.getDependencyConstraints().addLater(lazyConstraint); // Return a fake dependency constraint object to satisfy Kotlin DSL backwards compatibility return DUMMY_CONSTRAINT; } - private DependencyConstraint doAddListProvider(Configuration configuration, Provider dependencyNotation, Action configureAction) { + private DependencyConstraint doAddListProvider(Configuration configuration, Provider dependencyNotation, @Nullable Action configureAction) { // workaround for the fact that mapping to a list will not create a `CollectionProviderInternal` ListProperty constraints = objects.listProperty(DependencyConstraint.class); constraints.set(dependencyNotation.map(notation -> { List deps = Cast.uncheckedCast(notation); - return deps.stream().map(d -> create(d, configureAction)).collect(Collectors.toList()); + return deps.stream().map(d -> doCreate(d, configureAction)).collect(Collectors.toList()); })); configuration.getDependencyConstraints().addAllLater(constraints); return DUMMY_CONSTRAINT; } - private Transformer mapDependencyConstraintProvider(Action configurationAction) { - return lazyNotation -> create(lazyNotation, configurationAction); + private Transformer mapDependencyConstraintProvider(@Nullable Action configurationAction) { + return lazyNotation -> doCreate(lazyNotation, configurationAction); } @Override From 834fbce415d8355a3f6a38f9030596c7b9662307 Mon Sep 17 00:00:00 2001 From: Louis Jacomet Date: Wed, 16 Mar 2022 15:18:34 +0100 Subject: [PATCH 3/4] Fix typo in exception --- .../notations/DependencyConstraintNotationParser.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/subprojects/dependency-management/src/main/java/org/gradle/api/internal/notations/DependencyConstraintNotationParser.java b/subprojects/dependency-management/src/main/java/org/gradle/api/internal/notations/DependencyConstraintNotationParser.java index 3ed83d16e790..0059d4a7c5cd 100644 --- a/subprojects/dependency-management/src/main/java/org/gradle/api/internal/notations/DependencyConstraintNotationParser.java +++ b/subprojects/dependency-management/src/main/java/org/gradle/api/internal/notations/DependencyConstraintNotationParser.java @@ -98,12 +98,12 @@ private final static class UnsupportedCapabilitiesHandler implements ModuleDepen @Override public void requireCapability(Object capabilityNotation) { - throw new InvalidUserDataException("Capabilies are not supported by dependency constraints"); + throw new InvalidUserDataException("Capabilities are not supported by dependency constraints"); } @Override public void requireCapabilities(Object... capabilityNotations) { - throw new InvalidUserDataException("Capabilies are not supported by dependency constraints"); + throw new InvalidUserDataException("Capabilities are not supported by dependency constraints"); } } } From b95575c0b2821ac71f7ea48ae0662de4103b414e Mon Sep 17 00:00:00 2001 From: Louis Jacomet Date: Wed, 16 Mar 2022 15:41:54 +0100 Subject: [PATCH 4/4] Add support for dependency constraint without version Prior to this change, a published Gradle Module Metadata that contained a constraint without a version definition would result in a null VersionConstraint instead of an empty one. This caused issue during metadata serialization and potentially resolution. The field is not nullable and an empty version constraint is now used as the default value. Fixes #20189 --- ...pendenciesAttributesIntegrationTest.groovy | 59 +++++++++++++++++++ .../parser/GradleModuleMetadataParser.java | 2 +- .../GradleModuleMetadataParserTest.groovy | 4 +- 3 files changed, 63 insertions(+), 2 deletions(-) diff --git a/subprojects/dependency-management/src/integTest/groovy/org/gradle/integtests/resolve/attributes/DependenciesAttributesIntegrationTest.groovy b/subprojects/dependency-management/src/integTest/groovy/org/gradle/integtests/resolve/attributes/DependenciesAttributesIntegrationTest.groovy index ddbf42fc9d2f..9d936c340058 100644 --- a/subprojects/dependency-management/src/integTest/groovy/org/gradle/integtests/resolve/attributes/DependenciesAttributesIntegrationTest.groovy +++ b/subprojects/dependency-management/src/integTest/groovy/org/gradle/integtests/resolve/attributes/DependenciesAttributesIntegrationTest.groovy @@ -506,6 +506,65 @@ class DependenciesAttributesIntegrationTest extends AbstractModuleDependencyReso 'c1' | 'c2' | 'runtime' | ['org.gradle.status': defaultStatus(), 'org.gradle.usage': 'java-runtime', 'org.gradle.libraryelements': 'jar', 'org.gradle.category': 'library', custom: 'c2'] } + @Issue("https://github.com/gradle/gradle/issues/20182") + @RequiredFeature(feature = GradleMetadataResolveRunner.GRADLE_METADATA, value = "true") + @Unroll("Selects variant #expectedVariant using custom attribute value #dependencyValue overriding configuration attribute #configurationValue using dependency constraint without version") + def "dependency attribute value overrides configuration attribute using dependency constraint without version"() { + given: + repository { + 'org:test:1.0' { + variant('api') { + attribute('custom', 'c1') + } + variant('runtime') { + attribute('custom', 'c2') + } + } + } + + buildFile << """ + configurations.conf.attributes.attribute(CUSTOM_ATTRIBUTE, '$configurationValue') + + dependencies { + constraints { + conf('org:test') { + attributes { + attribute(CUSTOM_ATTRIBUTE, '$dependencyValue') + } + } + } + conf 'org:test:1.0' + } + """ + + when: + repositoryInteractions { + 'org:test:1.0' { + expectResolve() + } + } + succeeds 'checkDeps' + + then: + resolve.expectGraph { + root(":", ":test:") { + module('org:test:1.0') { + configuration = expectedVariant + variant(expectedVariant, expectedAttributes) + } + constraint('org:test', 'org:test:1.0') { + configuration = expectedVariant + variant(expectedVariant, expectedAttributes) + } + } + } + + where: + configurationValue | dependencyValue | expectedVariant | expectedAttributes + 'c2' | 'c1' | 'api' | ['org.gradle.status': defaultStatus(), 'org.gradle.usage': 'java-api', 'org.gradle.libraryelements': 'jar', 'org.gradle.category': 'library', custom: 'c1'] + 'c1' | 'c2' | 'runtime' | ['org.gradle.status': defaultStatus(), 'org.gradle.usage': 'java-runtime', 'org.gradle.libraryelements': 'jar', 'org.gradle.category': 'library', custom: 'c2'] + } + @RequiredFeature(feature = GradleMetadataResolveRunner.GRADLE_METADATA, value = "true") @ToBeFixedForConfigurationCache def "Fails resolution because consumer configuration attributes and constraint attributes conflict"() { diff --git a/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/ivyresolve/parser/GradleModuleMetadataParser.java b/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/ivyresolve/parser/GradleModuleMetadataParser.java index 030c47aee756..5edc958813da 100644 --- a/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/ivyresolve/parser/GradleModuleMetadataParser.java +++ b/subprojects/dependency-management/src/main/java/org/gradle/api/internal/artifacts/ivyservice/ivyresolve/parser/GradleModuleMetadataParser.java @@ -439,7 +439,7 @@ private List consumeDependencyConstraints(JsonReader String group = null; String module = null; String reason = null; - VersionConstraint version = null; + VersionConstraint version = DefaultImmutableVersionConstraint.of(); ImmutableAttributes attributes = ImmutableAttributes.EMPTY; reader.beginObject(); diff --git a/subprojects/dependency-management/src/test/groovy/org/gradle/api/internal/artifacts/ivyservice/ivyresolve/parser/GradleModuleMetadataParserTest.groovy b/subprojects/dependency-management/src/test/groovy/org/gradle/api/internal/artifacts/ivyservice/ivyresolve/parser/GradleModuleMetadataParserTest.groovy index 88abade1f193..feaa387f61fe 100644 --- a/subprojects/dependency-management/src/test/groovy/org/gradle/api/internal/artifacts/ivyservice/ivyresolve/parser/GradleModuleMetadataParserTest.groovy +++ b/subprojects/dependency-management/src/test/groovy/org/gradle/api/internal/artifacts/ivyservice/ivyresolve/parser/GradleModuleMetadataParserTest.groovy @@ -381,7 +381,8 @@ class GradleModuleMetadataParserTest extends Specification { "group": "g3", "module": "m3", "version": { "requires": "v3" } - } + }, + { "group": "g4", "module": "m4" } ], "attributes": { "usage": "compile" } }, @@ -404,6 +405,7 @@ class GradleModuleMetadataParserTest extends Specification { 1 * variant1.addDependencyConstraint("g1", "m1", requires("v1"), null, ImmutableAttributes.EMPTY) 1 * variant1.addDependencyConstraint("g2", "m2", prefers("v2"), null, ImmutableAttributes.EMPTY) 1 * variant1.addDependencyConstraint("g3", "m3", requires("v3"), null, ImmutableAttributes.EMPTY) + 1 * variant1.addDependencyConstraint("g4", "m4", emptyConstraint(), null, ImmutableAttributes.EMPTY) 1 * variant1.setAvailableExternally(false) 1 * metadata.addVariant("runtime", attributes(usage: "runtime", packaging: "zip")) >> variant2 1 * variant2.addDependencyConstraint("g3", "m3", prefers("v3"), null, ImmutableAttributes.EMPTY)