spring-framework 34685 이슈

이슈 요약

Spring 6.2.3 이후 버전에서 GenericConversionService에서 잘못된 컨버터가 선택되는 문제가 발생했습니다.

 

6.2.3 전후 컨버터 선택 로직의 변화

6.2.3 이전 - 컨버터 선택 방식

@Nullable  
public GenericConverter getConverter(TypeDescriptor sourceType, TypeDescriptor targetType) {  
    for (GenericConverter converter : this.converters) {  
       if (!(converter instanceof ConditionalGenericConverter genericConverter) ||  
             genericConverter.matches(sourceType, targetType)) {  
          return converter;  
       }  
    }  
    return null;  
}

컨버터의 변환 정보가 ConvertersForPair 타입으로 저장되고 getConverter 실행시 컨버터의 matches 메서드를 사용하여 각 컨버터가 자체 로직으로 매칭 여부를 판단했습니다.

 

@Override
public boolean matches(TypeDescriptor sourceType, TypeDescriptor targetType) {
    if (this.typeInfo.getTargetType() != targetType.getObjectType()) {
        return false;
    }
    ResolvableType rt = targetType.getResolvableType();
    if (!(rt.getType() instanceof Class) && !rt.isAssignableFromResolvedPart(this.targetType)) {
        return false;
    }
    return !(this.converter instanceof ConditionalConverter conditionalConverter) ||
            conditionalConverter.matches(sourceType, targetType);
}

// ResolvableType
public boolean isAssignableFromResolvedPart(ResolvableType other) {  
    return isAssignableFrom(other, false, null, true);  
}

private boolean isAssignableFrom(ResolvableType other, boolean strict,  
       @Nullable Map<Type, Type> matchedBefore, boolean upUntilUnresolvable) {
    if (this.type instanceof Class<?> clazz && other.type instanceof Class<?> otherClazz) {
        return (strict ? clazz.isAssignableFrom(otherClazz) : ClassUtils.isAssignable(clazz, otherClazz));
    }
}

GenericConversionService에서는 제너릭 타입에 대해서 isAssignableFromResolvedPart 메서드 내부의 Class.isAssignableFrom 메서드를 활용하여 할당 가능 여부를 판단하고 있습니다.

이는 할당 가능 여부를 판단할 컨버터의 targetType이 List<Map<String, ?>> 타입으로 선언되어 있더라도 List 타입으로 할당 가능 여부를 판단함을 의미합니다.

 

6.2.3 이후 - 변경된 컨버터 선택 방식

34535 이슈에서 코틀린의 List를 사용할 때, 기존 로직이 적절한 컨버터를 선택하지 못하는 문제가 보고되었고 이를 해결하기 위해 matchesFallback 메서드가 추가되었습니다.

public boolean matchesFallback(TypeDescriptor sourceType, TypeDescriptor targetType) {  
    return (this.typeInfo.getTargetType() == targetType.getObjectType() &&  
          this.targetType.hasUnresolvableGenerics() &&  
          (!(this.converter instanceof ConditionalConverter conditionalConverter) ||  
                conditionalConverter.matches(sourceType, targetType)));  
}

 

 

또한 기존의 getConverter 메서드는 다음과 같이 개선되었습니다.

@Nullable  
public GenericConverter getConverter(TypeDescriptor sourceType, TypeDescriptor targetType) {  
    for (GenericConverter converter : this.converters) {  
       if (!(converter instanceof ConditionalGenericConverter genericConverter) ||  
             genericConverter.matches(sourceType, targetType)) {  
          return converter;  
       }  
    }  
    for (GenericConverter converter : this.converters) {  
       if (converter instanceof ConverterAdapter converterAdapter &&  
             converterAdapter.matchesFallback(sourceType, targetType)) {  
          return converter;  
       }  
    }  
    return null;  
}

 

34685 이슈 분석

conversionService.addConverter(new StringToCollectionConverter(conversionService));
conversionService.addConverter(new StringToListOfMapConverter());

List<String> result = (List<String>) conversionService.convert("foo,bar",
        TypeDescriptor.valueOf(String.class),
        TypeDescriptor.collection(List.class, TypeDescriptor.valueOf(String.class))
);

GenericConversionService에 등록된 컨버터들이 위와 같이 설정된 상황에서 잘못된 컨버터가 선택되는 문제가 발생하였다고 합니다.

• 기대: StringToCollectionConverter
• 결과: StringToListOfMapConverter

 

문제 원인 분석

@Override  
public @Nullable Object convert(@Nullable Object source, @Nullable TypeDescriptor sourceType, TypeDescriptor targetType) {  
    // ...
    GenericConverter converter = getConverter(sourceType, targetType);  
    if (converter != null) {  
       Object result = ConversionUtils.invokeConverter(converter, source, sourceType, targetType);  
       return handleResult(sourceType, targetType, result);  
    }  
    return handleConverterNotFound(source, sourceType, targetType);  
}

protected @Nullable GenericConverter getConverter(TypeDescriptor sourceType, TypeDescriptor targetType) {  
    // ...
    converter = this.converters.find(sourceType, targetType);  
}

public @Nullable GenericConverter find(TypeDescriptor sourceType, TypeDescriptor targetType) {
    List<Class<?>> sourceCandidates = getClassHierarchy(sourceType.getType());
    List<Class<?>> targetCandidates = getClassHierarchy(targetType.getType());

    for (Class<?> sourceCandidate : sourceCandidates) {
        for (Class<?> targetCandidate : targetCandidates) {
            ConvertiblePair convertiblePair = new ConvertiblePair(sourceCandidate, targetCandidate);
            GenericConverter converter = getRegisteredConverter(sourceType, targetType, convertiblePair);
            if (converter != null) {
                return converter;
            }
        }
    }
    return null;
}

등록된 컨버터 중 적합한 컨버터를 찾기 위한 find 메서드에서는 sourceType과 targetType의 클래스 계층을 조회해서 적합한 컨버터를 조회합니다.

 

private @Nullable GenericConverter getRegisteredConverter(TypeDescriptor sourceType,  
       TypeDescriptor targetType, ConvertiblePair convertiblePair) {  
    // ...
    ConvertersForPair convertersForPair = this.converters.get(convertiblePair);  
    if (convertersForPair != null) {  
    GenericConverter converter = convertersForPair.getConverter(sourceType, targetType);  
    if (converter != null) {  
       return converter;  
    }  
  }
}

public @Nullable GenericConverter getConverter(TypeDescriptor sourceType, TypeDescriptor targetType) {
    for (GenericConverter converter : this.converters) {
        if (!(converter instanceof ConditionalGenericConverter genericConverter) ||
              genericConverter.matches(sourceType, targetType)) {
            return converter;
        }
    }
    for (GenericConverter converter : this.converters) {
        if (converter instanceof ConverterAdapter converterAdapter &&
              converterAdapter.matchesFallback(sourceType, targetType)) {
            return converter;
        }
    }
    return null;
}

조금 더 구체적으로 이슈를 기준으로 살펴본다면 targetType인 List<String>의 getClassHierarchy 메서드의 실행 결과는 List, Collection 순서의 리스트가 됩니다.
그리고 matchesFallback 메서드에서 로우 타입 비교를 통해 적합 여부를 판단하고 있어 List<Map<String, ?>>도 List<String>의 후보로 선택되는 문제가 발생하고 있었습니다. 즉 제너릭을 제대로 판단하고 있지 않았습니다.

 

개선 제안

이슈의 문제가 matchesFallback의 제너릭 판단 기준이 약하여 발생하는 문제라 생각하였고, 아래와 같이 재귀적으로 타입을 비교하는 로직을 제안하였습니다.

public boolean matchesFallback(TypeDescriptor sourceType, TypeDescriptor targetType) {
    if (this.typeInfo.getTargetType() != targetType.getObjectType()) {
       return false;
    }

    if (!this.targetType.hasUnresolvableGenerics()) {
       return false;
    }

    boolean assignable = isAssignableTo(targetType.getResolvableType());
    if (!assignable) {
       return false;
    }

    return !(this.converter instanceof ConditionalConverter conditionalConverter) ||
          conditionalConverter.matches(sourceType, targetType);
}

private boolean isAssignableTo(ResolvableType expected) {
    return isAssignable(this.targetType, expected);
}

private boolean isAssignable(ResolvableType actual, ResolvableType expected) {
    Class<?> actualResolved = actual.resolve(Object.class);
    Class<?> expectedResolved = expected.resolve(Object.class);
    if (!expectedResolved.isAssignableFrom(actualResolved)) {
       return false;
    }

    ResolvableType[] actualGenerics = actual.getGenerics();
    ResolvableType[] expectedGenerics = expected.getGenerics();

    if (actualGenerics.length != expectedGenerics.length) {
       return false;
    }

    for (int i = 0; i < actualGenerics.length; i++) {
       if (!isAssignable(actualGenerics[i], expectedGenerics[i])) {
          return false;
       }
    }

    return true;
}

 

• PR: Fix GenericConversionService selects incorrect converter