Re: [PATCH 1/1] snapshot::web Fix internal redirects to farm
On 11/17/24 2:52 PM, MOESSBAUER, Felix wrote:
> On Sat, 2024-11-16 at 23:48 +0100, Philipp Kern wrote:
>> Hi,
>>
>> On 11/15/24 2:27 PM, Felix Moessbauer wrote:
>>> We already have a rule to let Varnish follow the 302 redirects from
>>> the
>>> /archive endpoint to the farm (/file) endpoint. This rule was not
>>> effective, though, as it assumed that the 302 redirects are
>>> absolute
>>> URIs (including the protocol and host part). We now change this to
>>> follow the realtive (internal) redirects, restoring this behavior.
>>>
>>> A nice side-effect is that the artifacts are now delivered with the
>>> correct file-name (instead of the farm hash). Further it
>>> effectively
>>> cuts the number of http requests to s.d.o in half.
>> I tried, but this caused Bad Requests to be emitted by Apache.
>
> Did you see any further indication what Apache did not like about the
> request? Could you try commenting out the "set bereq.http.host =
> beresp.http.host;" line? Maybe the beresp does not contain this header
> (as it is technically a request header), hence overwriting the correct
> value in the request.
>
> I tested the patch locally with Varnish in front of the flask app
> (without Apache). Maybe flask simply does not check this.
>
> Nonetheless, the current (unpatched) code is not effective, as the 302
> redirect is forwarded to the client (can easily be seen in curl).
I tested the change on lw07, and the location header we see from the
webapp is fully qualified, so the new proposed patch does not match either:
> - RespHeader Location: http://snapshot-lw07.debian.org/file/4c086f4fd3df97030a4e300a43ab3f897d5b15f8
On lw07 Apache did not fail to process the request. At this point I am
also confused if the machines are set up identically - AIUI at least
sallinen is configured differently/has different data.
Kind regards
Philipp Kern
Reply to: