-
-
Notifications
You must be signed in to change notification settings - Fork 4k
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
X-Accel-Redirect setup doesn't work due to strange behaior of rewrite #5208
Comments
Hmm, good question. I think the rewrite module decides whether it should rewrite the query based on the original string being input, before doing placeholder replacement, by checking if it contains a This is definitely tricky. There are security considerations here, because not all placeholders contain trusted values, and in many configs there are implicit |
Hm, if the rewrite destination has a |
As a workaround, would it be possible for your app to send the path and query as different headers for now? Then it should be easy to merge them together with a rewrite with Caddy. |
Thanks for your answers. @francislavoie Unfortunately, even treating path and query with different header does not solve it, as the query part still gets URL-encoded( I agree automatic encoding makes sense for untrusted variables. Some ideas about how this could be implemented:
Still very much interested about any ideas for workaround if you have any (at this point, the only thing I found is url-decoding the string manually with
after my rewrite block, but that's super dirty and will break as soon as there are new special characters in my query strings. |
l'm trying to do something similar. I have a Python app, which stores files in a S3-like service. I want to have permanent links to those files, but the S3 service uses transient URLs like: https://cloud.canary.kaikosystems.com/media/kaiko-canary/artifacts/2022/09/20/file_au74XBv.jpg?X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=REDACTED%2Ffr-par%2Fs3%2Faws4_request&X-Amz-Date=20230119T143400Z&X-Amz-Expires=3600&X-Amz-SignedHeaders=host&X-Amz-Signature=redacted So I have configured Caddy with
The Python returns a response with both a When I request one of those URLs, I'm getting a 403 with the following headers:
I notice that |
Directives are sorted according to this order: https://caddyserver.com/docs/caddyfile/directives#directive-order. Notice that |
@francislavoie Thanks for your response. Unfortunately, using
has the effect to remove the header entirely from the response. |
@mvaled That:
replaces all instances of the You probably meant:
|
@mholt Thanks! I did like you suggested and now I'm getting the |
I activated the global
Then come two logs from the rewriter, and we can see the problem there. The first rewrite is our
The second rewrite simply removes the '/media' from the URL:
Finally comes the internal redirect:
|
A little trick helps:
|
I did some thinking, and I think the best solution is to provide a new option to
|
I just wanted to add a working Caddyfile snippet I am using with Caddy 2.7.6:
|
The problem with that is it'll append another |
That's not what I see in my testing with Caddy 2.7.6 or master @francislavoie.
|
That might have to do with how rewrite decides whether to rewrite the query or not. If you put a question mark at the end it signals to the rewrite handler to also rewrite the query string. That's my guess :) |
any tips on where I can add a test so we're all on the same page with this rewrite behavior (and so it doesn't get altered without a bit of discussion)? this X-Accel-Redirect with S3 seems like a pretty common use case |
@ottenhoff You mean like a rewrite that is caddy/modules/caddyhttp/rewrite/rewrite_test.go Lines 157 to 161 in 4af38e5
|
@mholt that's not quite the same because |
I'm still trying to figure out why this config works and produces a transparent redirect to the S3 asset with correct GET params (?X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=AKQS...)
and this rewrite results in /path/to/s3/asset with all necessary AWS GET params stripped:
|
@francislavoie Right; that part still needs a patch (I think you've had a PR open on it but I just haven't put the energy into reviewing it yet 😅 ) |
Hey ! I'm trying to configure a pretty simple X-Accel-Redirect kind of setup under Caddy 2.6.2, but think I encountered an issue that prevents me to do so.
My stripped down Caddyfile looks like this:
The
service_a
returns a response with a headerX-Accel-Redirect: /hello?some=param&some_other=param
. This is be picked by Caddy and rewritten so that this query is handled byservice_b
. The rewrite of the path works, but the GET params get swallowed and are not visible toservice_b
where I only see/hello
.It works as expected when I hardcode the same value in the caddy file like this (which IMO should be 100% equivalent).
Just to understand what's happening, I tried with some variants such as
rewrite * /hello?{rp.header.X-Accel-Redirect}
with just the paramters in the header. In this case,service_b
seeshttp://localhost/hello?some%3Dparam%26some_other%3Dparam
, so not good either.Seems some rogue urlencoding happens, but I couldn't find anything in the doc about that.
Any pointer about how to solve this ? I searched through the issues but could not find anything about this exactly.
The text was updated successfully, but these errors were encountered: