fix: refactor HomeFilter to build redirect location correctly
The issue is that your filter hardcodes the redirect to /ide/, which assumes the app is deployed at the root (/). If your app is deployed under a context path (e.g. /deephaven), you need to prepend that dynamically to send to /deephaven/ide
All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.
No docs changes detected for b7653b16a14c16afb5b5be29f618890d776394bf
I have read the CLA Document and I hereby sign the CLA
Related to: #3786
This may be a separate issue? I'm not sure if we need to support proxied context roots?
Agreed this is a separate issue. I am in an environment that must be deployed as a container and all apps are proxied through NGINX and must be at something like /apps/deephaven/
The current code breaks in this scenario.
For example:
https://melloware.com/apps/deephaven when it is proxied deephaven returns Location: /ide so the resulting URL is https://melloware.com/ide which throws a 404 instead of https://melloware.com/apps/deephaven/ide
@niloc132 yeah so we are trying to deploy DeepHaven as a Domino Data Labs application https://docs.dominodatalab.com/en/latest/user_guide/71635d/publish-apps/
this proxies the container so the URL is /apps/deephaven and I am exposing deephaven on 0.0.0.0 on port 8888 which is their requirement for exposing an app.
# Start Deephaven server
echo "Starting Deephaven server on port 8888..."
START_OPTS="-Dhttp.port=8888 -Dhttp.host=0.0.0.0 -DAuthHandlers=io.deephaven.auth.AnonymousAuthenticationHandler" \
./server-jetty-${DH_VERSION}/bin/start
the issue arises as soon as I hit their proxied URL which hits DeepHaven at 0.0.0.0:8888 through their proxy its turning...
/apps/deephaven/
into
/ide
which gives a 404 error because that is not the correct URL through their proxy.
I think we'll accept the PR, but just use caution around expecting this to work long term. We have a refactor planned in our grpc calls from the JS API that may or may not break this use case, and there is already explicit support when creating a JS dh.CoreClient instance with headers to have the call routed in a proxy to the correct server.
Also, use caution with proxy servers that do not fully support h2 end-to-end, as this breaks grpc. As long as you are deploying on localhost without TLS, you can use websocket transports to get your data back and forth, but for production use cases on servers, h2 is assumed by gRPC (and by Deephaven). Quickly browsing some DominoDataLabs documentation I see notes about Apache Arrow Flight (which is what our transport layer is based on), which will use gRPC, so I have to assume some support exists there - but gRPC-web is required by browsers, and some proxies special-case gRPC and break for gRPC-web.
understood! I didn't realize how much of the proxying may or may not work. However I am glad you agree this change at least won't break current behavior!
Would you replicate this change to the jetty11 package that has the same class as well? Just to cover all bases with ensuring consistent code.
Done updated jetty11 as well.