Skip to content

Commit d7a5229

Browse files
committed
fix: don't apply Bean factories for proto enums
Using the full original factory to construct a mutator for protobuf enum types would trigger a deeply nested mutator construction attempt on the EnumValueDescriptor class using e.g. the `CachedConstructorMutatorFactory`. While this still ended up at the dedicated enum mutator for most cases, for complex protobuf parameters it could lead to the construction of invalid mutators. At the point where an enum type is reached we now use a simplified factory instead that only deals with repeated or nullable enums.
1 parent 6fe8978 commit d7a5229

File tree

7 files changed

+61
-27
lines changed

7 files changed

+61
-27
lines changed

src/main/java/com/code_intelligence/jazzer/mutation/mutator/collection/BUILD.bazel

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ java_library(
22
name = "collection",
33
srcs = glob(["*.java"]),
44
visibility = [
5-
"//src/main/java/com/code_intelligence/jazzer/mutation/mutator:__pkg__",
5+
"//src/main/java/com/code_intelligence/jazzer/mutation/mutator:__subpackages__",
66
"//src/test/java/com/code_intelligence/jazzer/mutation/mutator:__subpackages__",
77
],
88
deps = [

src/main/java/com/code_intelligence/jazzer/mutation/mutator/lang/BUILD.bazel

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ java_library(
22
name = "lang",
33
srcs = glob(["*.java"]),
44
visibility = [
5-
"//src/main/java/com/code_intelligence/jazzer/mutation/mutator:__pkg__",
5+
"//src/main/java/com/code_intelligence/jazzer/mutation/mutator:__subpackages__",
66
"//src/test/java/com/code_intelligence/jazzer/mutation/mutator:__subpackages__",
77
],
88
deps = [

src/main/java/com/code_intelligence/jazzer/mutation/mutator/proto/BUILD.bazel

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ java_library(
1313
"//src/main/java/com/code_intelligence/jazzer/mutation/api",
1414
"//src/main/java/com/code_intelligence/jazzer/mutation/combinator",
1515
"//src/main/java/com/code_intelligence/jazzer/mutation/engine",
16+
"//src/main/java/com/code_intelligence/jazzer/mutation/mutator/collection",
17+
"//src/main/java/com/code_intelligence/jazzer/mutation/mutator/lang",
1618
"//src/main/java/com/code_intelligence/jazzer/mutation/support",
1719
"//src/main/java/com/code_intelligence/jazzer/mutation/utils",
1820
],

src/main/java/com/code_intelligence/jazzer/mutation/mutator/proto/BuilderMutatorFactory.java

Lines changed: 30 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,8 @@
5353
import com.code_intelligence.jazzer.mutation.api.SerializingInPlaceMutator;
5454
import com.code_intelligence.jazzer.mutation.api.SerializingMutator;
5555
import com.code_intelligence.jazzer.mutation.engine.ChainedMutatorFactory;
56+
import com.code_intelligence.jazzer.mutation.mutator.collection.CollectionMutators;
57+
import com.code_intelligence.jazzer.mutation.mutator.lang.LangMutators;
5658
import com.code_intelligence.jazzer.mutation.support.Preconditions;
5759
import com.google.protobuf.Any;
5860
import com.google.protobuf.Descriptors.Descriptor;
@@ -140,31 +142,35 @@ private ExtendedMutatorFactory withDescriptorDependentMutatorFactoryIfNeeded(
140142
if (field.getJavaType() == JavaType.ENUM) {
141143
// Proto enum fields are special as their type (EnumValueDescriptor) does not encode their
142144
// domain - we need the actual EnumDescriptor instance.
145+
MutatorFactory enumFactory =
146+
(type, factory) ->
147+
asSubclassOrEmpty(type, EnumValueDescriptor.class)
148+
.map(
149+
unused -> {
150+
EnumDescriptor enumType = field.getEnumType();
151+
List<EnumValueDescriptor> values = enumType.getValues();
152+
String name = enumType.getName();
153+
if (values.size() == 1) {
154+
// While we generally prefer to error out instead of creating a
155+
// mutator that can't actually mutate its domain, we can't do that
156+
// for proto enum fields as the user creating the fuzz test may not be
157+
// in a position to modify the existing proto definition.
158+
return fixedValue(values.get(0));
159+
} else {
160+
return mutateThenMapToImmutable(
161+
mutateIndices(values.size()),
162+
values::get,
163+
EnumValueDescriptor::getIndex,
164+
unused2 -> "Enum<" + name + ">");
165+
}
166+
});
167+
// Apart from the specific enum factory we need handling for `null` and other collection
168+
// types. Note, that we don't include the full original factory here because we don't want
169+
// to follow constructors or builders of the EnumValueDescriptor class.
143170
return ChainedMutatorFactory.of(
144-
originalFactory.getCache(),
145-
Stream.of(
146-
originalFactory,
147-
(type, factory) ->
148-
asSubclassOrEmpty(type, EnumValueDescriptor.class)
149-
.map(
150-
unused -> {
151-
EnumDescriptor enumType = field.getEnumType();
152-
List<EnumValueDescriptor> values = enumType.getValues();
153-
String name = enumType.getName();
154-
if (values.size() == 1) {
155-
// While we generally prefer to error out instead of creating a
156-
// mutator that can't actually mutate its domain, we can't do that for
157-
// proto enum fields as the user creating the fuzz test may not be in
158-
// a position to modify the existing proto definition.
159-
return fixedValue(values.get(0));
160-
} else {
161-
return mutateThenMapToImmutable(
162-
mutateIndices(values.size()),
163-
values::get,
164-
EnumValueDescriptor::getIndex,
165-
unused2 -> "Enum<" + name + ">");
166-
}
167-
})));
171+
Stream.concat(
172+
Stream.concat(LangMutators.newFactories(), CollectionMutators.newFactories()),
173+
Stream.of(enumFactory)));
168174
} else if (field.getJavaType() == JavaType.MESSAGE) {
169175
Descriptor messageDescriptor;
170176
if (field.isMapField()) {

src/test/java/com/code_intelligence/jazzer/mutation/ArgumentsMutatorTest.java

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@
2323
import com.code_intelligence.jazzer.mutation.annotation.NotNull;
2424
import com.code_intelligence.jazzer.mutation.mutator.Mutators;
2525
import com.code_intelligence.jazzer.mutation.support.TestSupport.MockPseudoRandom;
26+
import com.code_intelligence.jazzer.protobuf.Proto2.TestProtobuf;
27+
import com.google.protobuf.DescriptorProtos;
2628
import java.io.ByteArrayInputStream;
2729
import java.io.ByteArrayOutputStream;
2830
import java.lang.reflect.Method;
@@ -347,4 +349,26 @@ void testReadEmptyBeanWithRuntimeError() throws NoSuchMethodException {
347349
// expected
348350
}
349351
}
352+
353+
// Regression: ensure we can build an ArgumentsMutator for a fuzz target that
354+
// takes a protobuf message and a protobuf descriptor as parameters.
355+
@SuppressWarnings("unused")
356+
public static void complexProtoFuzzTest(
357+
@NotNull TestProtobuf proto, @NotNull DescriptorProtos.DescriptorProto descriptor) {}
358+
359+
@Test
360+
void testProtoAndDescriptorParameters() throws NoSuchMethodException {
361+
Method method =
362+
ArgumentsMutatorTest.class.getMethod(
363+
"complexProtoFuzzTest", TestProtobuf.class, DescriptorProtos.DescriptorProto.class);
364+
Optional<ArgumentsMutator> maybeMutator =
365+
ArgumentsMutator.forMethod(Mutators.newFactory(), method);
366+
assertThat(maybeMutator).isPresent();
367+
// Ensure that the mutator debug string can be generated. This would fail with the error:
368+
// Caused by: java.lang.NullPointerException: Cannot invoke
369+
//
370+
// "com.code_intelligence.jazzer.mutation.api.SerializingMutator.toDebugString(java.util.function.Predicate)"
371+
// because "productMutator" is null
372+
assertThat(maybeMutator.toString()).contains("-> Message");
373+
}
350374
}

src/test/java/com/code_intelligence/jazzer/mutation/BUILD.bazel

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ java_test_suite(
1010
"//src/main/java/com/code_intelligence/jazzer/mutation/annotation",
1111
"//src/main/java/com/code_intelligence/jazzer/mutation/api",
1212
"//src/main/java/com/code_intelligence/jazzer/mutation/mutator",
13+
"//src/test/java/com/code_intelligence/jazzer/mutation/mutator/proto:proto2_java_proto",
1314
"//src/test/java/com/code_intelligence/jazzer/mutation/support:test_support",
15+
"@protobuf//java/core",
1416
],
1517
)

src/test/java/com/code_intelligence/jazzer/mutation/mutator/proto/BUILD.bazel

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ java_proto_library(
2828
testonly = True,
2929
visibility = [
3030
"//selffuzz:__subpackages__",
31-
"//src/test/java/com/code_intelligence/jazzer/mutation/mutator:__pkg__",
31+
"//src/test/java/com/code_intelligence/jazzer/mutation:__subpackages__",
3232
"//tests:__pkg__",
3333
],
3434
deps = [":proto2_proto"],

0 commit comments

Comments
 (0)