search

springdoc / springdoc-openapi

Library for OpenAPI 3 with spring-boot
https://springdoc.org
Apache License 2.0
3.25k stars 489 forks source link

ArrayIndexOutOfBoundsException in SwaggerUiConfigParameters #2509

Closed GianniGiglio closed 5 months ago

GianniGiglio commented 7 months ago

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: https://github.com/oracle/graal/issues/5902

To Reproduce Unable to reproduce locally

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

Additional context Add any other context about the problem here.

kzander91 commented 6 months ago

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:

https://github.com/springdoc/springdoc-openapi/assets/61500114/c2f4c3a8-7b32-41e9-a0f7-0e78eabf164f

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 commented 6 months ago

@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 commented 6 months ago

@bnasslahsen

I think I found the bug in these two lines: https://github.com/springdoc/springdoc-openapi/blob/492d542a8a74541a79348c7e1e1e788f41411b34/springdoc-openapi-starter-webflux-ui/src/main/java/org/springdoc/webflux/ui/SwaggerWelcomeWebFlux.java#L111-L112 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 commented 6 months ago

@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 commented 6 months ago

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

NielsDoucet commented 6 months ago

@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 commented 6 months ago

Good catch, we iterate over the set here https://github.com/springdoc/springdoc-openapi/blob/492d542a8a74541a79348c7e1e1e788f41411b34/springdoc-openapi-starter-common/src/main/java/org/springdoc/core/properties/SwaggerUiConfigParameters.java#L307 and here: https://github.com/springdoc/springdoc-openapi/blob/492d542a8a74541a79348c7e1e1e788f41411b34/springdoc-openapi-starter-common/src/main/java/org/springdoc/core/properties/SwaggerUiConfigParameters.java#L214-L221

And can concurrently modify it here: https://github.com/springdoc/springdoc-openapi/blob/492d542a8a74541a79348c7e1e1e788f41411b34/springdoc-openapi-starter-common/src/main/java/org/springdoc/core/properties/SwaggerUiConfigParameters.java#L193-L206

@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 commented 6 months ago

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 commented 6 months ago

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

bnasslahsen commented 6 months ago

@NielsDoucet or @kzander91,

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

kzander91 commented 6 months ago

@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 commented 5 months ago

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 commented 5 months ago

@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?

EugeneMsv commented 2 months ago

The issue is still present in springdoc-openapi-starter-common-2.4.0

bnasslahsen commented 2 months ago

@EugeneMsv,

Your comment could be useful if you provided a Minimal, Reproducible Example - with HelloController that reproduces the problem.

dgswan commented 1 month ago

@bnasslahsen could you please check here the minimal, reproducible example? I assume the issue is caused by the changes introduced in #2213. I have prepared the PR to fix the issue and will be looking forward to your feedback. Thank you!