Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ArrayIndexOutOfBoundsException in SwaggerUiConfigParameters #2509

Closed
GianniGiglio opened this issue Feb 9, 2024 · 13 comments
Closed

ArrayIndexOutOfBoundsException in SwaggerUiConfigParameters #2509

GianniGiglio opened this issue Feb 9, 2024 · 13 comments

Comments

@GianniGiglio
Copy link

Description:

An "ArrayIndexOutOfBoundsException" error has been reported within the SwaggerUiConfigParameters class. This exception occurs during the execution of methods invoked by the AbstractSwaggerIndexTransformer class. The issue was raised by one of our customers, and although we've attempted to reproduce it, we have been unsuccessful thus far. However, monitoring of all customer logs indicates that this issue is not isolated and has affected multiple customers.

Methode:

`protected String addParameters(String html) throws JsonProcessingException {
String layout = swaggerUiConfigParameters.getLayout() != null ? swaggerUiConfigParameters.getLayout() : "StandaloneLayout";
StringBuilder stringBuilder = new StringBuilder("layout: "" + layout + "" ,\n");

	Map<String, Object> parametersObjectMap = swaggerUiConfigParameters.getConfigParameters().entrySet().stream()
			.filter(entry -> !SwaggerUiConfigParameters.OAUTH2_REDIRECT_URL_PROPERTY.equals(entry.getKey()))
			.filter(entry -> !SwaggerUiConfigParameters.URL_PROPERTY.equals(entry.getKey()))
			.filter(entry -> !SwaggerUiConfigParameters.URLS_PROPERTY.equals(entry.getKey()))
			.filter(entry -> SwaggerUiConfigParameters.VALIDATOR_URL_PROPERTY.equals(entry.getKey())
					|| ((entry.getValue() instanceof String) ? StringUtils.isNotEmpty((String) entry.getValue()) : entry.getValue() != null))
			.collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue, (e1, e2) -> e2,
					LinkedHashMap::new));

	if (!CollectionUtils.isEmpty(parametersObjectMap)) {
		String result = objectMapper.writerWithDefaultPrettyPrinter().writeValueAsString(parametersObjectMap);
		result = result.substring(1, result.length() - 1);
		stringBuilder.append(result);
		html = html.replace("layout: \"StandaloneLayout\"", stringBuilder.toString());
	}

	return html;
}`

Stacktrace:

java.lang.ArrayIndexOutOfBoundsException: Index 11 out of bounds for length 11 at java.util.stream.SortedOps$SizedRefSortingSink.accept(Unknown Source) at java.util.HashMap$KeySpliterator.forEachRemaining(Unknown Source) at java.util.stream.AbstractPipeline.copyInto(Unknown Source) at java.util.stream.AbstractPipeline.wrapAndCopyInto(Unknown Source) at java.util.stream.ReduceOps$ReduceOp.evaluateSequential(Unknown Source) at java.util.stream.AbstractPipeline.evaluate(Unknown Source) at java.util.stream.ReferencePipeline.collect(Unknown Source) at org.springdoc.core.properties.SwaggerUiConfigParameters.put(SwaggerUiConfigParameters.java:307) at org.springdoc.core.properties.SwaggerUiConfigParameters.getConfigParameters(SwaggerUiConfigParameters.java:284) at org.springdoc.ui.AbstractSwaggerIndexTransformer.addParameters(AbstractSwaggerIndexTransformer.java:198) at org.springdoc.ui.AbstractSwaggerIndexTransformer.defaultTransformations(AbstractSwaggerIndexTransformer.java:173)
Possible Explanation:
The ArrayIndexOutOfBoundsException typically occurs when attempting to access an element of an array or collection at an index that is beyond its bounds. In this context, the usage of the sorted operation in a stateful stream operation within the SwaggerUiConfigParameters class might lead to unexpected behavior. Stateful stream operations maintain state during the execution of the stream pipeline, which could potentially alter the order or length of elements being processed. Consequently, if the index being accessed within the SwaggerUiConfigParameters class exceeds the updated length of the array or collection due to the stateful operation, it could result in the ArrayIndexOutOfBoundsException being thrown.

Similar non springdoc-issue:
oracle/graal#5902

To Reproduce
Unable to reproduce locally

  • What version of spring-boot you are using?
    org.springdoc:springdoc-openapi-starter-webmvc-ui:2.2.0

  • What modules and versions of springdoc-openapi are you using?
    org.springdoc:springdoc-openapi-starter-webmvc-ui:2.2.0

  • What is the actual and the expected result using OpenAPI Description (yml or json)?
    Index page doesn't load

  • Provide with a sample code (HelloController) or Test that reproduces the problem
    Unable to reproduce
    Expected behavior

  • A clear and concise description of what you expected to happen.

  • What is the expected result using OpenAPI Description (yml or json)?

Screenshots
If applicable, add screenshots to help explain your problem.

Additional context
Add any other context about the problem here.

@kzander91
Copy link

kzander91 commented Mar 2, 2024

We have encountered this as well, we're using WebFlux and GroupedOpenApi.
For us this can be reproduced pretty reliably by requesting the OpenAPI specs for multiple API groups concurrently. Here's a simple reproducer:
demo1.zip

To reproduce this, I open the SwaggerUI page for each API group in multiple tabs, restart the server and then refresh all tabs at once (in Firefox, you can select multiple tabs and hit Ctrl+R to refresh them all at once). It may take a few attempts, but with this methodology I'd say I can reproduce this issue every 3rd time.

Here's a screen recording of me doing that:

springdoc_aiobe_c.mp4

Once the API spec has been loaded once, concurrent requests seem to always work, therefore I think this only happens with concurrent requests to an uninitialized server.

@bnasslahsen
Copy link
Contributor

bnasslahsen commented Mar 2, 2024

@GianniGiglio and @kzander91,

it's just not reproducible with the provided code.
I believe, you have all the elements to reproduce it in local.

Would you be able to propose PR with a fix for it ?

@kzander91
Copy link

kzander91 commented Mar 3, 2024

@bnasslahsen

I think I found the bug in these two lines:

this.oauthPrefix = uriComponentsBuilder.path(contextPath).path(swaggerUiConfigParameters.getUiRootPath()).path(webJarsPrefixUrl);
swaggerUiConfigParameters.setOauth2RedirectUrl(this.oauthPrefix.path(getOauth2RedirectUrl()).build().toString());

SwaggerWelcomeFlux is a singleton and concurrent requests will modify and then read the same oauthPrefix field, which is a UriComponentsBuilder which is not thread-safe.

Here's an example stack trace when two threads concurrently call oauthPrefix#path():

2024-03-03T12:49:33.758+01:00 ERROR 8968 --- [ctor-http-nio-5] a.w.r.e.AbstractErrorWebExceptionHandler : [f8d52c63-666]  500 Server Error for HTTP GET "/v3/api-docs/swagger-config"

java.lang.ArrayIndexOutOfBoundsException: arraycopy: last destination index 72 out of bounds for byte[40]
	at java.base/java.lang.System.arraycopy(Native Method) ~[na:na]
	Suppressed: reactor.core.publisher.FluxOnAssembly$OnAssemblyException: 
Error has been observed at the following site(s):
	*__checkpoint ⇢ HTTP GET "/v3/api-docs/swagger-config" [ExceptionHandlingWebHandler]
Original Stack Trace:
		at java.base/java.lang.System.arraycopy(Native Method) ~[na:na]
		at java.base/java.lang.String.getBytes(String.java:4726) ~[na:na]
		at java.base/java.lang.AbstractStringBuilder.putStringAt(AbstractStringBuilder.java:1751) ~[na:na]
		at java.base/java.lang.AbstractStringBuilder.append(AbstractStringBuilder.java:588) ~[na:na]
		at java.base/java.lang.StringBuilder.append(StringBuilder.java:179) ~[na:na]
		at org.springframework.web.util.UriComponentsBuilder$FullPathComponentBuilder.append(UriComponentsBuilder.java:919) ~[spring-web-6.1.4.jar:6.1.4]
		at org.springframework.web.util.UriComponentsBuilder$CompositePathComponentBuilder.addPath(UriComponentsBuilder.java:868) ~[spring-web-6.1.4.jar:6.1.4]
		at org.springframework.web.util.UriComponentsBuilder.path(UriComponentsBuilder.java:622) ~[spring-web-6.1.4.jar:6.1.4]
		at org.springdoc.webflux.ui.SwaggerWelcomeWebFlux.calculateOauth2RedirectUrl(SwaggerWelcomeWebFlux.java:112) ~[springdoc-openapi-starter-webflux-ui-2.3.0.jar:2.3.0]
		at org.springdoc.ui.AbstractSwaggerWelcome.buildConfigUrl(AbstractSwaggerWelcome.java:145) ~[springdoc-openapi-starter-common-2.3.0.jar:2.3.0]
		at org.springdoc.webflux.ui.SwaggerWelcomeCommon.buildFromCurrentContextPath(SwaggerWelcomeCommon.java:108) ~[springdoc-openapi-starter-webflux-ui-2.3.0.jar:2.3.0]
		at org.springdoc.webflux.ui.SwaggerWelcomeCommon.getSwaggerUiConfig(SwaggerWelcomeCommon.java:92) ~[springdoc-openapi-starter-webflux-ui-2.3.0.jar:2.3.0]
		at org.springdoc.webflux.ui.SwaggerConfigResource.getSwaggerUiConfig(SwaggerConfigResource.java:67) ~[springdoc-openapi-starter-webflux-ui-2.3.0.jar:2.3.0]
		at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103) ~[na:na]
		at java.base/java.lang.reflect.Method.invoke(Method.java:580) ~[na:na]
		at org.springframework.web.reactive.result.method.InvocableHandlerMethod.lambda$invoke$1(InvocableHandlerMethod.java:185) ~[spring-webflux-6.1.4.jar:6.1.4]
		at reactor.core.publisher.MonoFlatMap$FlatMapMain.onNext(MonoFlatMap.java:132) ~[reactor-core-3.6.3.jar:3.6.3]
		at reactor.core.publisher.MonoZip$ZipCoordinator.signal(MonoZip.java:297) ~[reactor-core-3.6.3.jar:3.6.3]
		at reactor.core.publisher.MonoZip$ZipInner.onNext(MonoZip.java:478) ~[reactor-core-3.6.3.jar:3.6.3]
		at reactor.core.publisher.MonoPeekTerminal$MonoTerminalPeekSubscriber.onNext(MonoPeekTerminal.java:180) ~[reactor-core-3.6.3.jar:3.6.3]
		at reactor.core.publisher.Operators$ScalarSubscription.request(Operators.java:2571) ~[reactor-core-3.6.3.jar:3.6.3]
		at reactor.core.publisher.MonoPeekTerminal$MonoTerminalPeekSubscriber.request(MonoPeekTerminal.java:139) ~[reactor-core-3.6.3.jar:3.6.3]
		at reactor.core.publisher.MonoZip$ZipInner.onSubscribe(MonoZip.java:470) ~[reactor-core-3.6.3.jar:3.6.3]
		at reactor.core.publisher.MonoPeekTerminal$MonoTerminalPeekSubscriber.onSubscribe(MonoPeekTerminal.java:152) ~[reactor-core-3.6.3.jar:3.6.3]
		at reactor.core.publisher.MonoJust.subscribe(MonoJust.java:55) ~[reactor-core-3.6.3.jar:3.6.3]
		at reactor.core.publisher.InternalMonoOperator.subscribe(InternalMonoOperator.java:76) ~[reactor-core-3.6.3.jar:3.6.3]
		at reactor.core.publisher.MonoZip$ZipCoordinator.request(MonoZip.java:220) ~[reactor-core-3.6.3.jar:3.6.3]
		at reactor.core.publisher.MonoFlatMap$FlatMapMain.request(MonoFlatMap.java:194) ~[reactor-core-3.6.3.jar:3.6.3]
		at reactor.core.publisher.MonoIgnoreThen$ThenIgnoreMain.onSubscribe(MonoIgnoreThen.java:135) ~[reactor-core-3.6.3.jar:3.6.3]
		at reactor.core.publisher.MonoFlatMap$FlatMapMain.onSubscribe(MonoFlatMap.java:117) ~[reactor-core-3.6.3.jar:3.6.3]
		at reactor.core.publisher.MonoZip.subscribe(MonoZip.java:129) ~[reactor-core-3.6.3.jar:3.6.3]
		at reactor.core.publisher.InternalMonoOperator.subscribe(InternalMonoOperator.java:76) ~[reactor-core-3.6.3.jar:3.6.3]
		at reactor.core.publisher.MonoDefer.subscribe(MonoDefer.java:53) ~[reactor-core-3.6.3.jar:3.6.3]
		at reactor.core.publisher.MonoIgnoreThen$ThenIgnoreMain.subscribeNext(MonoIgnoreThen.java:241) ~[reactor-core-3.6.3.jar:3.6.3]
		at reactor.core.publisher.MonoIgnoreThen$ThenIgnoreMain.onComplete(MonoIgnoreThen.java:204) ~[reactor-core-3.6.3.jar:3.6.3]
		at reactor.core.publisher.MonoFlatMap$FlatMapMain.onComplete(MonoFlatMap.java:189) ~[reactor-core-3.6.3.jar:3.6.3]
		at reactor.core.publisher.Operators.complete(Operators.java:137) ~[reactor-core-3.6.3.jar:3.6.3]
		at reactor.core.publisher.MonoZip.subscribe(MonoZip.java:121) ~[reactor-core-3.6.3.jar:3.6.3]
		at reactor.core.publisher.Mono.subscribe(Mono.java:4563) ~[reactor-core-3.6.3.jar:3.6.3]
		at reactor.core.publisher.MonoIgnoreThen$ThenIgnoreMain.subscribeNext(MonoIgnoreThen.java:265) ~[reactor-core-3.6.3.jar:3.6.3]
		at reactor.core.publisher.MonoIgnoreThen.subscribe(MonoIgnoreThen.java:51) ~[reactor-core-3.6.3.jar:3.6.3]
		at reactor.core.publisher.InternalMonoOperator.subscribe(InternalMonoOperator.java:76) ~[reactor-core-3.6.3.jar:3.6.3]
		at reactor.core.publisher.MonoFlatMap$FlatMapMain.onNext(MonoFlatMap.java:165) ~[reactor-core-3.6.3.jar:3.6.3]
		at reactor.core.publisher.FluxOnErrorResume$ResumeSubscriber.onNext(FluxOnErrorResume.java:79) ~[reactor-core-3.6.3.jar:3.6.3]
		at reactor.core.publisher.FluxSwitchIfEmpty$SwitchIfEmptySubscriber.onNext(FluxSwitchIfEmpty.java:74) ~[reactor-core-3.6.3.jar:3.6.3]
		at reactor.core.publisher.MonoNext$NextSubscriber.onNext(MonoNext.java:82) ~[reactor-core-3.6.3.jar:3.6.3]
		at reactor.core.publisher.FluxConcatMapNoPrefetch$FluxConcatMapNoPrefetchSubscriber.innerNext(FluxConcatMapNoPrefetch.java:259) ~[reactor-core-3.6.3.jar:3.6.3]
		at reactor.core.publisher.FluxConcatMap$ConcatMapInner.onNext(FluxConcatMap.java:865) ~[reactor-core-3.6.3.jar:3.6.3]
		at reactor.core.publisher.FluxMapFuseable$MapFuseableSubscriber.onNext(FluxMapFuseable.java:129) ~[reactor-core-3.6.3.jar:3.6.3]
		at reactor.core.publisher.MonoPeekTerminal$MonoTerminalPeekSubscriber.onNext(MonoPeekTerminal.java:180) ~[reactor-core-3.6.3.jar:3.6.3]
		at reactor.core.publisher.Operators$ScalarSubscription.request(Operators.java:2571) ~[reactor-core-3.6.3.jar:3.6.3]
		at reactor.core.publisher.MonoPeekTerminal$MonoTerminalPeekSubscriber.request(MonoPeekTerminal.java:139) ~[reactor-core-3.6.3.jar:3.6.3]
		at reactor.core.publisher.FluxMapFuseable$MapFuseableSubscriber.request(FluxMapFuseable.java:171) ~[reactor-core-3.6.3.jar:3.6.3]
		at reactor.core.publisher.Operators$MultiSubscriptionSubscriber.request(Operators.java:2331) ~[reactor-core-3.6.3.jar:3.6.3]
		at reactor.core.publisher.FluxConcatMapNoPrefetch$FluxConcatMapNoPrefetchSubscriber.request(FluxConcatMapNoPrefetch.java:339) ~[reactor-core-3.6.3.jar:3.6.3]
		at reactor.core.publisher.MonoNext$NextSubscriber.request(MonoNext.java:108) ~[reactor-core-3.6.3.jar:3.6.3]
		at reactor.core.publisher.Operators$MultiSubscriptionSubscriber.set(Operators.java:2367) ~[reactor-core-3.6.3.jar:3.6.3]
		at reactor.core.publisher.Operators$MultiSubscriptionSubscriber.onSubscribe(Operators.java:2241) ~[reactor-core-3.6.3.jar:3.6.3]
		at reactor.core.publisher.MonoNext$NextSubscriber.onSubscribe(MonoNext.java:70) ~[reactor-core-3.6.3.jar:3.6.3]
		at reactor.core.publisher.FluxConcatMapNoPrefetch$FluxConcatMapNoPrefetchSubscriber.onSubscribe(FluxConcatMapNoPrefetch.java:164) ~[reactor-core-3.6.3.jar:3.6.3]
		at reactor.core.publisher.FluxIterable.subscribe(FluxIterable.java:201) ~[reactor-core-3.6.3.jar:3.6.3]
		at reactor.core.publisher.FluxIterable.subscribe(FluxIterable.java:83) ~[reactor-core-3.6.3.jar:3.6.3]
		at reactor.core.publisher.InternalMonoOperator.subscribe(InternalMonoOperator.java:76) ~[reactor-core-3.6.3.jar:3.6.3]
		at reactor.core.publisher.MonoDefer.subscribe(MonoDefer.java:53) ~[reactor-core-3.6.3.jar:3.6.3]
		at reactor.core.publisher.Mono.subscribe(Mono.java:4563) ~[reactor-core-3.6.3.jar:3.6.3]
		at reactor.core.publisher.MonoIgnoreThen$ThenIgnoreMain.subscribeNext(MonoIgnoreThen.java:265) ~[reactor-core-3.6.3.jar:3.6.3]
		at reactor.core.publisher.MonoIgnoreThen.subscribe(MonoIgnoreThen.java:51) ~[reactor-core-3.6.3.jar:3.6.3]
		at reactor.core.publisher.InternalMonoOperator.subscribe(InternalMonoOperator.java:76) ~[reactor-core-3.6.3.jar:3.6.3]
		at reactor.core.publisher.MonoDeferContextual.subscribe(MonoDeferContextual.java:55) ~[reactor-core-3.6.3.jar:3.6.3]
		at reactor.netty.http.server.HttpServer$HttpServerHandle.onStateChange(HttpServer.java:1169) ~[reactor-netty-http-1.1.16.jar:1.1.16]
		at reactor.netty.ReactorNetty$CompositeConnectionObserver.onStateChange(ReactorNetty.java:710) ~[reactor-netty-core-1.1.16.jar:1.1.16]
		at reactor.netty.transport.ServerTransport$ChildObserver.onStateChange(ServerTransport.java:481) ~[reactor-netty-core-1.1.16.jar:1.1.16]
		at reactor.netty.http.server.HttpServerOperations.onInboundNext(HttpServerOperations.java:652) ~[reactor-netty-http-1.1.16.jar:1.1.16]
		at reactor.netty.channel.ChannelOperationsHandler.channelRead(ChannelOperationsHandler.java:114) ~[reactor-netty-core-1.1.16.jar:1.1.16]
		at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:444) ~[netty-transport-4.1.107.Final.jar:4.1.107.Final]
		at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420) ~[netty-transport-4.1.107.Final.jar:4.1.107.Final]
		at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412) ~[netty-transport-4.1.107.Final.jar:4.1.107.Final]
		at reactor.netty.http.server.HttpTrafficHandler.channelRead(HttpTrafficHandler.java:238) ~[reactor-netty-http-1.1.16.jar:1.1.16]
		at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:442) ~[netty-transport-4.1.107.Final.jar:4.1.107.Final]
		at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420) ~[netty-transport-4.1.107.Final.jar:4.1.107.Final]
		at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412) ~[netty-transport-4.1.107.Final.jar:4.1.107.Final]
		at io.netty.channel.CombinedChannelDuplexHandler$DelegatingChannelHandlerContext.fireChannelRead(CombinedChannelDuplexHandler.java:436) ~[netty-transport-4.1.107.Final.jar:4.1.107.Final]
		at io.netty.handler.codec.ByteToMessageDecoder.fireChannelRead(ByteToMessageDecoder.java:346) ~[netty-codec-4.1.107.Final.jar:4.1.107.Final]
		at io.netty.handler.codec.ByteToMessageDecoder.channelRead(ByteToMessageDecoder.java:318) ~[netty-codec-4.1.107.Final.jar:4.1.107.Final]
		at io.netty.channel.CombinedChannelDuplexHandler.channelRead(CombinedChannelDuplexHandler.java:251) ~[netty-transport-4.1.107.Final.jar:4.1.107.Final]
		at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:442) ~[netty-transport-4.1.107.Final.jar:4.1.107.Final]
		at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420) ~[netty-transport-4.1.107.Final.jar:4.1.107.Final]
		at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412) ~[netty-transport-4.1.107.Final.jar:4.1.107.Final]
		at io.netty.channel.DefaultChannelPipeline$HeadContext.channelRead(DefaultChannelPipeline.java:1410) ~[netty-transport-4.1.107.Final.jar:4.1.107.Final]
		at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:440) ~[netty-transport-4.1.107.Final.jar:4.1.107.Final]
		at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420) ~[netty-transport-4.1.107.Final.jar:4.1.107.Final]
		at io.netty.channel.DefaultChannelPipeline.fireChannelRead(DefaultChannelPipeline.java:919) ~[netty-transport-4.1.107.Final.jar:4.1.107.Final]
		at io.netty.channel.nio.AbstractNioByteChannel$NioByteUnsafe.read(AbstractNioByteChannel.java:166) ~[netty-transport-4.1.107.Final.jar:4.1.107.Final]
		at io.netty.channel.nio.NioEventLoop.processSelectedKey(NioEventLoop.java:788) ~[netty-transport-4.1.107.Final.jar:4.1.107.Final]
		at io.netty.channel.nio.NioEventLoop.processSelectedKeysOptimized(NioEventLoop.java:724) ~[netty-transport-4.1.107.Final.jar:4.1.107.Final]
		at io.netty.channel.nio.NioEventLoop.processSelectedKeys(NioEventLoop.java:650) ~[netty-transport-4.1.107.Final.jar:4.1.107.Final]
		at io.netty.channel.nio.NioEventLoop.run(NioEventLoop.java:562) ~[netty-transport-4.1.107.Final.jar:4.1.107.Final]
		at io.netty.util.concurrent.SingleThreadEventExecutor$4.run(SingleThreadEventExecutor.java:997) ~[netty-common-4.1.107.Final.jar:4.1.107.Final]
		at io.netty.util.internal.ThreadExecutorMap$2.run(ThreadExecutorMap.java:74) ~[netty-common-4.1.107.Final.jar:4.1.107.Final]
		at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30) ~[netty-common-4.1.107.Final.jar:4.1.107.Final]
		at java.base/java.lang.Thread.run(Thread.java:1583) ~[na:na]

The exact error message my vary a bit. I could reproduce it with calls to /webjars/swagger-ui/index.html and /v3/api-docs/swagger-config.
I have updated my sample with a test that does that: repro-with-test.zip

This is the test:

@Slf4j
@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT)
class ReproTest {

    @LocalServerPort
    private int port;

    @Autowired
    private WebClient client;

    @ParameterizedTest
    @ValueSource(strings = {
            "/webjars/swagger-ui/index.html",
            "/v3/api-docs/swagger-config",
    })
    void repro(String path) {
        int nConcurrent = 16;
        Flux<Void> requests = Flux.defer(() -> request(path).repeat());
        List<Flux<Void>> publishers = new ArrayList<>(nConcurrent);
        for (int i = 0; i < nConcurrent; i++) {
            publishers.add(requests);
        }
        Flux.merge(publishers).take(Duration.ofSeconds(30L)).blockLast();
    }

    private Mono<Void> request(String path) {
        return client.get()
                .uri("http://localhost:{port}" + path, port)
                .retrieve()
                .toBodilessEntity()
                .doOnError(ex -> log.error("Call failed, URL: {}", path))
                .then();
    }

    @TestConfiguration
    static class WebClientConfig {

        @Bean
        WebClient webClient(WebClient.Builder builder) {
            return builder.build();
        }

    }

}

It repeatedly performs 16 concurrent requests to these paths for 30 seconds. On my machine, this almost always reproduces the issue.
Give it a few attempts and/or maybe tweak the timeout or nConcurrent flags if it doesn't fail right away.

@bnasslahsen
Copy link
Contributor

@kzander91,

Thank you for your analysis which is very helpful.
I have added a fix for this tricky issue. Could you test baed on the Latest SNAPSHOT to confirm the fix is working for you ?

@kzander91
Copy link

@bnasslahsen
With the latest snapshots, I can no longer reproduce this issue, thank you!

@NielsDoucet
Copy link

NielsDoucet commented Mar 7, 2024

@bnasslahsen The originally reported issue is still present in the SwaggerUiConfigParameters class.
The issue is with the fact that you allow concurrent modification of the urls field of that class, which is not thread-safe: https://github.com/springdoc/springdoc-openapi/blob/main/springdoc-openapi-starter-common/src/main/java/org/springdoc/core/properties/SwaggerUiConfigParameters.java#L288

I think a safe fix would be to proactively clone this field's value before streaming over it.

put(URLS_PROPERTY, Set.copyOf(urls), params);

@kzander91
Copy link

kzander91 commented Mar 7, 2024

Good catch, we iterate over the set here

swaggerUrls = swaggerUrls.stream().sorted(swaggerUrlComparator).filter(elt -> StringUtils.isNotEmpty(elt.getUrl())).collect(Collectors.toCollection(LinkedHashSet::new));

and here:
public void addUrl(String url) {
this.urls.forEach(elt ->
{
if (!isSwaggerUrlDefined(elt.getName()))
elt.setUrl(url + DEFAULT_PATH_SEPARATOR + elt.getName());
}
);
}

And can concurrently modify it here:

public void addGroup(String group, String displayName) {
SwaggerUrl swaggerUrl = new SwaggerUrl(group, null, displayName);
urls.add(swaggerUrl);
}
/**
* Add group.
*
* @param group the group
*/
public void addGroup(String group) {
SwaggerUrl swaggerUrl = new SwaggerUrl(group, null, group);
urls.add(swaggerUrl);
}

@NielsDoucet

I think a safe fix would be to proactively clone this field's value before streaming over it.

put(URLS_PROPERTY, Set.copyOf(urls), params);

Set.copyOf(urls) iterates over it as well (to create the copy), so this wouldn't fix the problem.

@NielsDoucet
Copy link

Valid point.

Perhaps this will require the urls field to be a CopyOnWriteArraySet then. Or you'll have to do some other means of reading from this field in a thread-safe manner.

@kzander91
Copy link

@bnasslahsen I think this issue should be reopened, as the original bug has not been fixed, as identified by @NielsDoucet.

@bnasslahsen bnasslahsen reopened this Mar 11, 2024
@bnasslahsen
Copy link
Contributor

bnasslahsen commented Mar 11, 2024

@NielsDoucet or @kzander91,

Can you propose a PR for it or provide a reproducer project ?

@kzander91
Copy link

@bnasslahsen I'm not familiar enough with the code base to attempt to properly fix this issue. Maybe simply replacing the HashSet with a thread-safe implementation is enough, but I believe the problem lies deeper: Why are singleton beans being modified by web requests in the first place?

Looking at the code suggests that URLs have to be reconstructed based on the incoming request, I suppose that's because during startup, the app can't know the external URL it has been deployed behind. If that's the case and we therefore have to init with the first request, I would suggest to at least guard those critical sections using synchronized (or a virtual thread-friendly ReentrantLock).

@bnasslahsen
Copy link
Contributor

This ticket will be closed as not reproduced anymore,
If someone, can reproduce the issue feel free to provide a Provide a Minimal, Reproducible Example - with HelloController that reproduces the problem or better propose a PR.

@NielsDoucet
Copy link

@bnasslahsen While I understand your reaction to this, I believe @kzander91's response summarizes why neither him nor I feel comfortable suggesting a fix for this concurrency issue. Is there nothing you can do about this, given all the provided context?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants