From 6b644c5a5ba51c97999cda20a305c5a69626da94 Mon Sep 17 00:00:00 2001 From: Joe Hansche Date: Tue, 10 Mar 2026 18:50:38 -0400 Subject: [PATCH] Fix NPE in Options.retainAll when map option entry omits value Map option entries may omit 'value' when the value type's default is desired. This is common in OpenAPI v2 annotations, e.g. a security requirement that needs bearer auth but no scopes: security: { security_requirement: { key: "bearer" } } Wire's canonicalizeValue correctly produces null for the missing value, but gatherFields and retainAll assumed non-null values everywhere, crashing with NPE during schema pruning. Fix by accepting nullable values: make retainAll's parameter Any?, add an o == null -> null branch, and remove unnecessary !! operators on map/list iteration variables. Co-Authored-By: Claude Opus 4.6 --- .../com/squareup/wire/schema/Options.kt | 18 +++-- .../com/squareup/wire/schema/PrunerTest.kt | 81 +++++++++++++++++++ 2 files changed, 91 insertions(+), 8 deletions(-) diff --git a/wire-schema/src/commonMain/kotlin/com/squareup/wire/schema/Options.kt b/wire-schema/src/commonMain/kotlin/com/squareup/wire/schema/Options.kt index 1e4b324e7a..a6bcbc232a 100644 --- a/wire-schema/src/commonMain/kotlin/com/squareup/wire/schema/Options.kt +++ b/wire-schema/src/commonMain/kotlin/com/squareup/wire/schema/Options.kt @@ -333,18 +333,18 @@ class Options( // When the key isn't a `ProtoMember`, this key/value pair is a inlined value of a map // field. We don't need to track the key type in map fields for they are always of // scalar types. We however have to check the value type. - gatherFields(sink, type, value!!, pruningRules) + gatherFields(sink, type, value, pruningRules) continue } } if (pruningRules.prunes(protoMember)) continue sink.getOrPut(type, ::ArrayList).add(protoMember) - gatherFields(sink, protoMember.type, value!!, pruningRules) + gatherFields(sink, protoMember.type, value, pruningRules) } } is List<*> -> { for (e in o) { - gatherFields(sink, type, e!!, pruningRules) + gatherFields(sink, type, e, pruningRules) } } } @@ -373,9 +373,11 @@ class Options( schema: Schema, markSet: MarkSet, type: ProtoType?, - o: Any, + o: Any?, ): Any? { return when { + o == null -> null + o is Map<*, *> -> { val map = mutableMapOf() for ((key, value) in o) { @@ -384,7 +386,7 @@ class Options( else -> { // When the key isn't a `ProtoMember`, this key/value pair is a inlined value of a map // field. - val retainedValue = retainAll(schema, markSet, type, value!!) + val retainedValue = retainAll(schema, markSet, type, value) // if `retainedValue` is a map, its value represents an inline message, and we need to // mark the proto member. if (retainedValue is Map<*, *>) { @@ -403,11 +405,11 @@ class Options( } val field = schema.getField(protoMember)!! - val retainedValue = retainAll(schema, markSet, field.type, value!!) + val retainedValue = retainAll(schema, markSet, field.type, value) if (retainedValue != null) { map[protoMember] = retainedValue // This retained field is non-empty. } else if (isCoreMemberOfGoogleProtobuf) { - map[protoMember] = value + map[protoMember] = value!! } } map.ifEmpty { null } @@ -416,7 +418,7 @@ class Options( o is List<*> -> { val list = mutableListOf() for (value in o) { - val retainedValue = retainAll(schema, markSet, type, value!!) + val retainedValue = retainAll(schema, markSet, type, value) if (retainedValue != null) { list.add(retainedValue) // This retained value is non-empty. } diff --git a/wire-schema/src/jvmTest/kotlin/com/squareup/wire/schema/PrunerTest.kt b/wire-schema/src/jvmTest/kotlin/com/squareup/wire/schema/PrunerTest.kt index 0186bdb63b..549c16a3c2 100644 --- a/wire-schema/src/jvmTest/kotlin/com/squareup/wire/schema/PrunerTest.kt +++ b/wire-schema/src/jvmTest/kotlin/com/squareup/wire/schema/PrunerTest.kt @@ -257,6 +257,87 @@ class PrunerTest { ) } + /** + * Map option entries may omit 'value' when the value type's default is desired. This is common + * in OpenAPI v2 annotations, e.g. a security requirement that needs bearer auth but no scopes: + * + * ``` + * security: { security_requirement: { key: "bearer" } } + * ``` + * + * The missing 'value' means an empty SecurityRequirementValue (no scopes). Wire must handle + * these null map values without crashing during pruning. + */ + @Test + fun rootCanHandleInlinedOptionWithMapFieldsMissingValue() { + val schema = buildSchema { + add( + "openapiv2.proto".toPath(), + """ + |syntax = "proto3"; + | + |import "google/protobuf/descriptor.proto"; + | + |package grpc.gateway.protoc_gen_openapiv2.options; + | + |message SecurityRequirement { + | message SecurityRequirementValue { + | repeated string scope = 1; + | } + | map security_requirement = 1; + |} + | + |message Swagger { + | repeated SecurityRequirement security = 1; + |} + | + |extend google.protobuf.FileOptions { + | Swagger openapiv2_swagger = 80000; + |} + """.trimMargin(), + ) + add( + "test.proto".toPath(), + """ + |syntax = "proto3"; + | + |import "openapiv2.proto"; + | + |package test; + | + |option (grpc.gateway.protoc_gen_openapiv2.options.openapiv2_swagger) = { + | security: { security_requirement: { key: "bearer" } } + |}; + """.trimMargin(), + ) + } + val pruned = schema.prune( + PruningRules.Builder() + .addRoot("grpc.gateway.protoc_gen_openapiv2.options.SecurityRequirement#security_requirement") + .build(), + ) + assertThat(pruned.protoFile("openapiv2.proto")!!.toSchema()) + .isEqualTo( + """ + |// Proto schema formatted by Wire, do not edit. + |// Source: openapiv2.proto + | + |syntax = "proto3"; + | + |package grpc.gateway.protoc_gen_openapiv2.options; + | + |message SecurityRequirement { + | map security_requirement = 1; + | + | message SecurityRequirementValue { + | repeated string scope = 1; + | } + |} + | + """.trimMargin(), + ) + } + @Ignore("Pruning inlined map options is not supported") @Test fun pruneCanHandleInlinedOptionMemberWithMapFields() {