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

Calling Swagger UI via different context paths fails #2642

Closed
hwanders opened this issue Jul 9, 2024 · 6 comments
Closed

Calling Swagger UI via different context paths fails #2642

hwanders opened this issue Jul 9, 2024 · 6 comments
Labels
incomplete incomplete description: Make sure you Provide a Minimal, Reproducible Example - with HelloController

Comments

@hwanders
Copy link

hwanders commented Jul 9, 2024

Requesting the Swagger UI from two different public URLs having different context paths causes the second UI initialization to fail. The second UI call will have a wrong path written in swagger-initializer.js: configUrl is based on the path from the first request.

To Reproduce
Setup a reverse proxy (for example at localhost:80) in front of a backend service BS (for example at localhost:8080) which adds a prefix to all the backend service's URLs. So the backend service endpoints are available at http://localhost:80/service/** (reverse proxy with path prefix /service) and http://localhost:8080/** (backend service).
The backend service should be configured to provide a Swagger UI.

The following scenarios, each executed on a freshly restarted backend service, reproduce the problem:

  • First reverse proxy - then backend service
    1. Call the UI via the reverse proxy (works fine)
    2. Call the UI via the service without the reverse proxy (fails)
  • First backend service - then reverse proxy
    1. Call the UI via the service without the reverse proxy (works fine)
    2. Call the UI via the reverse proxy (fails)

I'm using springdoc-openapi-starter-webmvc-ui version 2.6.0.

Reason
SwaggerIndexPageTransformer initializes the singleton SwaggerUiConfigParameters by calling swaggerWelcomeCommon.buildFromCurrentContextPath(request) if the configuration's configUrl is null. The configUrl (and perhaps some other properties) is derived from the current request's context path.

Subsequent requests, even if they have a different context path, will use the initialized configuration without re-evaluating it.
The browser will then try to load the configuration from an URL which is not available.

Expected behavior
The configUrl should be re-evaluated (if necessary) to generate a correct swagger-initializer.js.

Ideas
Perhaps the SwaggerUiConfigParameters shouldn't be modified at all by the transformer.
This is to avoid concurrency problems caused by a hypothetical re-evaluation overwriting the configuration.

I'm not sure whether SwaggerUiConfigParameters.configUrl is a relevant public API for anybody. Changing or removing it is probably a breaking change.

@hwanders hwanders changed the title Calling Swagger UI via gateway and then directly fails Calling Swagger UI via different context paths fails Jul 11, 2024
@ababin
Copy link

ababin commented Sep 9, 2024

I have the same problem. I used 2.1.0 version. But now is 2.6.0. And without any changes. As I understand, the problem is openapi caches swagger-initializer.js. It is easy to reproduce. At first you need to be shure your application.yml has property server.forward-headers-strategy: framework. Then, just need to make request with different values of the header X-Forwarded-Prefix, which is used when preparing configUrl in swagger-initializer.js. After that there is no metter what value will be passed in X-Forwarded-Prefix in the further requests. The result will be the same (configUrl) - as after the FIRST request.

@bnasslahsen
Copy link
Collaborator

@ababin, or @hwanders

This description is quite unclear.

Try adding a failing test similar to what is available here, to showcase the issue you are facing:

/~https://github.com/springdoc/springdoc-openapi/tree/7bb5fc282f856500238bac81a9292a349fa4f873/springdoc-openapi-starter-webmvc-ui/src/test/java/test/org/springdoc/ui/app32

This ticket will be closed, but can be reopened if your provide all the elements.

@bnasslahsen bnasslahsen added the incomplete incomplete description: Make sure you Provide a Minimal, Reproducible Example - with HelloController label Sep 21, 2024
@hwanders
Copy link
Author

hwanders commented Sep 22, 2024

@bnasslahsen Sure, no problem.

@Test
public void shouldReturnCorrectInitializerJSWhenChangingForwardedPrefixHeader() throws Exception {
	// Request swagger-initializer.js with some X-Forwarded-Prefix header.
	mockMvc.perform(get("/swagger-ui/swagger-initializer.js")
					.header("X-Forwarded-Prefix", X_FORWARD_PREFIX));

	// When performing a second request with another X-Forwarded-Prefix header
	// the changed path should be reflected in the configUrl.
	mockMvc.perform(get("/swagger-ui/swagger-initializer.js")
					.header("X-Forwarded-Prefix", "/path/prefix2"))
			.andExpect(status().isOk())
			.andExpect(content().string(
					containsString("\"configUrl\" : \"/path/prefix2/v3/api-docs/swagger-config\",")
			));
}

When adding this to test.org.springdoc.ui.app32.SpringDocBehindProxyTest, we have a failing test.

To sum up: A request to swagger-initializer.js with an X-Forwarded-Prefix header value different from the first one received (in the past) causes a wrong response to be generated (still has the old configUrl).

@bnasslahsen
Copy link
Collaborator

bnasslahsen commented Sep 22, 2024

@hwanders,

I have added a fix for this request.
You can validate it already based on the Latest SNAPSHOT

@hwanders
Copy link
Author

Thank you @bnasslahsen . This fixes the described problem for sequential requests, which is a good progress.

Unfortunately there is still a problem when running requests with different context paths concurrently.

I could indeed write a test which sometimes fails due to concurrency issues:

@Test
public void shouldReturnCorrectInitializerJSWhenChangingForwardedPrefixHeader() throws Exception {
	var tasks = IntStream.range(0, 100).mapToObj(i -> CompletableFuture.runAsync(() -> {
		try {
			mockMvc.perform(get("/swagger-ui/swagger-initializer.js")
							.header("X-Forwarded-Prefix", "/path/prefix" + i))
					.andExpect(status().isOk())
					.andExpect(content().string(
							containsString("\"configUrl\" : \"/path/prefix" + i + "/v3/api-docs/swagger-config\",")
					));
		} catch (Exception e) {
			throw new RuntimeException(e);
		}
	})).toArray(CompletableFuture<?>[]::new);

	CompletableFuture.allOf(tasks).join();
}

Test error:

Expected: a string containing "\"configUrl\" : \"/path/prefix0/v3/api-docs/swagger-config\","
     but: was "window.onload = function() {
[skipped for brevity]
  "configUrl" : "/path/prefix6/v3/api-docs/swagger-config",
[...]
};
"

I guess this is due to SwaggerIndexPageTransformer.swaggerWelcomeCommon being reused across requests and updated during each request.

The swagger-config endpoint has the same problem (probably due to SwaggerUiConfigParameters being reused and modified):

@Test
public void shouldCalculateUrlsBehindProxyWhenChangingForwardedPrefixHeader() {
	var tasks = IntStream.range(0, 100).mapToObj(i -> CompletableFuture.runAsync(() -> {
		try {
			mockMvc.perform(get("/v3/api-docs/swagger-config")
							.header("X-Forwarded-Prefix", X_FORWARD_PREFIX + i))
					.andExpect(status().isOk())
					.andExpect(jsonPath("url",
							equalTo("/path/prefix" + i + "/v3/api-docs")
					))
					.andExpect(jsonPath("configUrl",
							equalTo("/path/prefix" + i + "/v3/api-docs/swagger-config")
					));
		} catch (Exception e) {
			throw new RuntimeException(e);
		}
	})).toArray(CompletableFuture<?>[]::new);

	CompletableFuture.allOf(tasks).join();
}

Test error:

java.util.concurrent.CompletionException: java.lang.AssertionError: JSON path "url"
Expected: "/path/prefix0/v3/api-docs"
     but: was "/path/prefix9/v3/api-docs"

@bnasslahsen
Copy link
Collaborator

@hwanders,

This one forces more changes :)
Can you test it based on the Latest SNAPSHOT ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
incomplete incomplete description: Make sure you Provide a Minimal, Reproducible Example - with HelloController
Projects
None yet
Development

No branches or pull requests

3 participants