Skip to content

X-Fowarded-Prefix value replacing the context path while generating link using spring HATEOAS #31241

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

Closed
abhijit-vmw opened this issue Sep 15, 2023 · 17 comments
Labels
status: superseded An issue that has been superseded by another

Comments

@abhijit-vmw
Copy link

We are facing this issue while using X-Fowarded-* headers to generate resource links.

When we use X-Forwarded-Prefix header in nginx configuration file, it basically replace this path with context path. Instead it should prepend prefix to context path.

For example, if context path is /myapp and if we set prefix to /api, it should return url like https://example.com/api/myapp/ but instead it returns like https://example.com/api/.

We are using spring boot 2.7.3.. I am referring this doc - https://docs.spring.io/spring-hateoas/docs/1.0.4.RELEASE/reference/html/#server.link-builder.forwarded-headers (section 3.4) and using ForwardedHeaderFilter to enable X forwarded headers.

Is there any way to prepend prefix to context path instead of replace?

Steps to reproduce.

  1. Create sample spring boot app.
  2. Add bean of ForwardedHeaderFilter or use property server.forward-headers-strategy
  3. Set context path for app. Ex. /myapp
  4. Pass X-Forwarded-* headers while running app.

curl -v localhost:8080/employees
-H ‘X-Forwarded-Proto: https’
-H ‘X-Forwarded-Host: 'example.com’
-H ‘X-Forwarded-Prefix: /api’

Expected result - href Link should be: https://example.com/api/myapp/hello
Actual result - href Link: https://example.com/api/hello

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Sep 15, 2023
@rstoyanchev
Copy link
Contributor

There is no way currently to prepend instead of replace. Does /myapp really need to appear in public links?

@dsyer
Copy link
Member

dsyer commented Sep 15, 2023

That seems like a bug to me. In what circumstances would you expect context path not to be included in the request URI? The responsible code is in ForwardedPrefixExtractor

public String getContextPath() {
	return (this.forwardedPrefix != null ? this.forwardedPrefix : this.delegate.get().getContextPath());
}

So it explicitly drops the context path if a X-Forwarded-Prefix is present. I can't see how that would ever be correct, but maybe I'm missing something.

@rstoyanchev
Copy link
Contributor

It's not a bug in the sense that it was a very conscious decision to do this. Original discussion in #18842 and also #18949.

@rstoyanchev
Copy link
Contributor

Another more recent discussion, under #27739 with a concrete scenario. Not sure if we need to make this configurable but it's been working this way for a very long time now. I don't think we can just flip the behavior.

@dsyer
Copy link
Member

dsyer commented Sep 15, 2023

It looks like I agree with past me - the discussion in #18842 didn't make any sense back then and it still doesn't. I gave up back then because no context path is normal for Spring Boot, so I didn't expect to encounter the bug very often. The proxy is not relevant, to the use case in this issue anyway. It is the local app that has a context path (for whatever reason - I agree with you that it doesn't need to have one, but if it does then this bug shows up). IMO it's still a bug, until I hear from someone who actually uses a context path and needs the links to be wrong.

@dsyer
Copy link
Member

dsyer commented Sep 15, 2023

I just chatted with @rwinch and he had a use case where omitting the context path was necessary (but still screwy if you ask me). He wanted the proxy to map requests to https://app.example.com/{path} to http://localhost:8080/app/{path}, where "/app" is now the context path, and therefore for links to come back with the context path stripped the proxy passes "X-Forwarded-Prefix: " (empty).

IMHO this is still crazy because there is nothing special about a context path for the proxy, which can know nothing of the implementation details of the backend. If it wasn't for the servlet spec we wouldn't even have the notion of a context path, and the proxy has no business assuming that the backend is built using servlets or Spring. But I concede that in this case at least passing an empty (not null) prefix is the only way to signal that you want something deleted from the link addresses. The problem is that we don't have a way for a proxy to ask for a path segment to be deleted.

For the sake of clarity, my intuition for this header is that the proxy is telling you about a prefix that it has removed. So a gateway where https://example.com/app/{path} is mapped to http://localhost:8080/{path} sends a header value "/app". This seems far more regular, and is a less crazy setup than assuming the proxy somehow knows about context paths. That's the problem with the current implementation - in my example the proxy has to send "/app/context-path" as the prefix header, even though it only stripped the first segment.

My advice for anyone who encounters this bug (including future me): don't use a context path in your Spring Boot applications. It's much simpler for everyone.

@abhijit-vmw
Copy link
Author

abhijit-vmw commented Sep 15, 2023

@dsyer, How can we force someone not to use context path?

This is our use case.

  1. We have multimodule spring boot application. Each module has it's context path. In general we use some word as context path, which is kind of module name. So resource URL is like http://example.com/<context-path>/<api-path>
  2. Applicatioin is running behind proxy. Proxy has rule for each module like /api/<context-path-module-1> will redirect to http://example.com/<context-path-module-1>/*, /api/<context-path-module-2> will redirect to http://example.com/<context-path-module-2>/* and so on. It means while calling resource from outside world, we need /api before context path. So any href link shown in response should have /api in it to be accessible from outside world.

Now in this scenario, we need /api as a prefix which should be after hostname and before context path. So if I try set /api using X-Forwarded-Prefix header, it replaces with context path, and href link displayed as http://example.com/api/* instead of http://example.com/api/<context-path-module-1>/*. Can't access href link (http://example.com/api/*) from outside world as context path is missing. Is this not a bug or valid scenario the other way?

From the discussion threads you shared I agree with idea that ForwardedHeaderFilter should have contextPathOverride property. But seems in current implementation it's missing.

@dsyer
Copy link
Member

dsyer commented Sep 16, 2023

Until something changes you have 2 options: write your own filter or remove the context path. The context path really isn't necessary in a Boot application (it's kind of an artifact of the old application server deployment model), so all you need to do is add your prefix to all the @RequestMapping paths (once per @Controller is enough and you can paramaterize it with a configuration property if you need that flexibility).

@rwinch
Copy link
Member

rwinch commented Sep 18, 2023

Prefix as a Noun (Replace) vs Verb (Inserting)

I think the confusion comes from the fact that the word prefix is both a noun (how ForwardedHeaderFilter behaves) and a verb (the requested behavior in this issue). If prefix were acting as a verb (as described in this issue), one would rightfully expect the header value to be placed in front of the existing prefix (context path). However, Spring’s ForwardedHeaderFilter treats prefix as a noun and thus the existing prefix is replaced by the header value.

An Example

The prefix is identified as the portion of the URL prior to the capture group. A concrete example can help to make this more clear. Consider the mapping of proxy server -> downstream server below:

 https://example.com/api/myapp/{path} -> http://localhost:8080/myapp/{path}

The prefix of the proxy is /api/myapp because that is the part before the capture group of {path}, but the downstream server’s prefix is /myapp. These are not the same and so an X-Forwarded-Header: /api/myapp can be used to instruct the downstream server to render links using the same prefix that the proxy server is using.

Proxy and Context Path?

While a context path is a Java specific construct, the idea of a base path is not. For example, Ruby allows specifying a relative_url_root which is similar to a context path.

In the example above, there is nothing Java specific that the proxy needs to understand. The proxy only needs to understand how it is routing the requests (which it must know already). This allows a Java application to adjust it's context root (base path or prefix), a Ruby application to adjust its relative_url_root (base path or prefix), etc so that the URLs that are constructed align with the proxy server's settings.

A Use Case

@dsyer mentioned our discussion, but I think some additional context could be beneficial.

A use case that I see frequently is that an organization pays licenses per production application server. This means that they prefer to deploy multiple applications to each application server to avoid paying the licensing fees.

I also see organizations that are using more resource intensive application servers. This means that they prefer to deploy multiple applications to each application server to avoid consuming additional resources.

In both of these use cases, applications must define a non-empty context root because there is more than one application associated to the same application server.

While their application is deployed with a non-empty context root, they do not want this expressed in the path of their URLs because they use a different subdomain for each application. Using different subdomains for each application provides benefits such as:

  • Added security (e.g. same origin policy)
  • Allows for scaling the applications differently (a different domain can point to different IP addresses)

In this scenario, the organization's wish to have a mapping of proxy server -> downstream server like this:

https://app1.example.com/ -> http://localhost:8080/app1
https://app2.example.com/ -> http://localhost:8080/app2

Benefits to Replacing the Prefix

As Spring’s ForwardedHeaderFilter works now, this is possible since X-Forwarded-Prefix replaces the prefix (rather than modifying the prefix) of the downstream server.

In general, if X-Forwarded-Prefix instructed the downstream server to prepend the value to its existing base URL (rather than replace it), that would mean there is no way to have a prefix that does not include the original prefix.

How to Configure ForwardedHeaderFilter to support <prefix> + <context>

Another benefit to replacing vs inserting is that your use case is also supported if you are able to adjust the value of X-Forwarded-Prefix. If I'm understanding your requirements correctly, you should be able to solve your problem by setting X-Forwarded-Prefix to /api/myapp. Does this work for you?

curl -v localhost:8080/employees \
-H 'X-Forwarded-Proto: https' \
-H 'X-Forwarded-Host: example.com' \
-H 'X-Forwarded-Prefix: /api/myapp'

Alternative Solution

NOTE: I recommend the solution in How to Configure ForwardedHeaderFilter to support <prefix> + <context> and not the alternative below.

If you insist on the relationship of X-Forwarded-Prefix inserting the value rather than overriding it, you can add a Filter before ForwardedHeaderFilter that uses HttpSerlvetRequestWrapper to modify the X-Forwarded-Prefix header to be <existing-value> + request.getContextPath().

Why X-Forwarded-Prefix Name?

Like all headers that start with X- the X-Forwarded-Prefix header is not "a standard", but it is not something that the Spring team came up with. While, the X-Forwarded-Prefix header is not as prominent as something like X-Forwarded-Proto, it is used by larger products/vendors. A concrete example can be found in Microsoft's YARP documentation which states:

X-Forwarded-Prefix - Sets the request's original PathBase, if any, to the X-Forwarded-Prefix header.

The description states the X-Forwarded-Prefix value is used to set the PathBase (as ForwardedHeaderFilter behaves). It does not insert the X-Forwarded-Prefix.

While the name X-Forwarded-Prefix is not as clear as it could be and it is non-standard (like all X-* headers), I think there is value in using the name X-Forwarded-Prefix because it is used in other contexts.

Adding a Flag for Insert?

Unless, the proposed solution of setting the X-Forwarded-Prefix to /api/myapp does not work, I do not see why we would add a flag. The current behavior can already provide the functionality that is being requested, so I do not see a good reason to add a flag.

What Should Be Done?

I think that we could and should improve the documentation around ForwardedHeaderFilter in both the reference and its Javadoc. This will help users to understand how to use it. I went ahead and created #31273 to address this.

@abhijit-vmw
Copy link
Author

Thanks for detailed explanation Rob. I will try to use your suggestions to fix my issue.

One suggestion is that, the word prefix creates lot of confusion, if possible we should consider renaming this word that better reflects the actual implementation (Replace). And doesn't creates noun vs verb mess. Something like X-Forwarded-Root-Path. Just a suggestion.

We can close this ticket.

@dsyer
Copy link
Member

dsyer commented Sep 19, 2023

I would actually rather not close it again without making a change to the filter. IMO the default behaviour is wrong, but at least we could provide a flag for users to switch to include the context path.

@abhijit-vmw
Copy link
Author

Agree with you Dave. If we can do this, it will be great. For me having 'contextPathOverride' property in filter makes sense.

@rwinch
Copy link
Member

rwinch commented Sep 19, 2023

I've added a couple more sections to my original post (so all of the information can be easily found) to address the comments since my post. The sections I've added are (I cannot seem to link directly to them):

  • Why X-Forwarded-Prefix Name?
  • Adding a Flag for Insert?
  • What Should Be Done?

@rstoyanchev
Copy link
Contributor

rstoyanchev commented Sep 19, 2023

We have multimodule spring boot application. Each module has it's context path. In general we use some word as context path, which is kind of module name.

@abhijit-vmw it's worth mentioning that you can route to each module without a contextPath by assigning a pathPrefix from the WebMvc config to a group of controllers identified with by package or by anything else. It might give you another option to consider.

@bclozel bclozel changed the title X-Fowarded-Prefix value replacing the context path while generating link using spring HETEOAS X-Fowarded-Prefix value replacing the context path while generating link using spring HATEOAS Sep 19, 2023
@rstoyanchev
Copy link
Contributor

As @rwinch stated in detail there are a number of options to make this work, and it's more a matter of setting expectations than a limitation. This is why we know have #31273 to provide more guidance.

@rstoyanchev rstoyanchev closed this as not planned Won't fix, can't repro, duplicate, stale Sep 19, 2023
@rstoyanchev rstoyanchev added status: superseded An issue that has been superseded by another and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Sep 19, 2023
@dsyer
Copy link
Member

dsyer commented Sep 21, 2023

I guess it's a strong disagree from me again (still), just so everyone knows. None of the options to make this work are convenient or "natural" for the use case described. Ask someone from the Spring Cloud Gateway team what they expect to happen if you don't believe me. A flag in the filter would be perfect IMO (and it should change the default behaviour also).

@t-beckmann
Copy link

I‘m with @dsyer interpreting prefix as a prefix, although @rwinch made me think… What is multi-value header #25254 adding to this discussion?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: superseded An issue that has been superseded by another
Projects
None yet
Development

No branches or pull requests

6 participants