Hi,

Some more potential NPEs, maybe you can take a look. They are a little complicated here I will describe the chain of calls.

  1. In MetadataGenerationEnvironment.java (spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/main/java/org/springframework/boot/configurationprocessor/MetadataGenerationEnvironment.java), function getAnnotation return null at line 198.

  2. In ConfigurationMetadataAnnotationProcessor (spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/main/java/org/springframework/boot/configurationprocessor/ConfigurationMetadataAnnotationProcessor.java), function processEndpoint, annotation on line 243 will be assigned null.

  3. Same file as step 2 line 245 processEndpoint will be called with the first argument annotation being null.

  4. Still same file line 253 processEndpoint function the first line Map<String, Object> elementValues = this.metadataEnv.getAnnotationElementValues(annotation); is called with annotation being null.

  5. In MetadataGenerationEnvironment.java (spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/main/java/org/springframework/boot/configurationprocessor/MetadataGenerationEnvironment.java), function getAnnotationElementValues at line 237. The expression annotation.getElementValues() wil throw a NPE at line 239.

We found some similar issues, but you can confirm this one before we report the others. Again, thanks so much for providing the feedback!!!

Comment From: philwebb

@HermioneSW Have you seen this NPE in the wild? Or did you find the potential problem using some kind of static analysis?

Comment From: HermioneSW

@philwebb we found it using our static analysis tool.

Comment From: HermioneSW

@philwebb could you please confirm this is indeed a bug? Thanks so much!

Comment From: philwebb

@HermioneSW I categorized it as a task rather than a bug since we've not actually seen it being triggered in the wild. It's targeted to 2.2.x and we'll change the code when we can.

Comment From: aivinog1

@philwebb Hello. Do you mind if I take this? I think I can fix these problems.

Comment From: wilkinsona

Looking at the code, I don't think this NPE can happen.

processEndpoint(Element element, List<Element>) is only called from process(Set<? extends TypeElement>, RoundEnvironment). It's called using forEach passing in each entry in the Map<Element, List<Element>> returned from getElementsAnnotatedOrMetaAnnotatedWith(RoundEnvironment, TypeElement). For the NPE to occur, getElementsAnnotatedOrMetaAnnotatedWith would have to contain a bug that resulted in it adding an item to the list of annotations that didn't actually exist. Only at that point could step 2 above result in an assignment to null.

I don't think we should check that annotation isn't null as this would mask an earlier bug. There's also little benefit to checking for null and throwing an IllegalStateException as processEndpoint will already catch any NPE and turn it into an IllegalStateException.