Re: [PATCH] Server PAC file from Squid

On Tue, 29 Jun 2010 14:29:43 -0600, Alex Rousskov
<rousskov_at_measurement-factory.com> wrote:
> On 06/28/2010 10:01 PM, Amos Jeffries wrote:
>> One of the old requests is to server the PAC files directly from Squid
>> without needing a local HTTP server just for the one file.
>>
>> This patch makes Squid intercept requests which use it's
visible_hostname
>> (and hostname_aliases) for /wpad.dat and pass out a PAC file specified
in
>> the configuration.
>>
>> This is particularly useful for public hot-spot installs. Allowing
>> tighter
>> security since unauthorized and badly configured clients no longer need
>> access to anywhere but the proxy itself; and less configuration since
an
>> additional web server is no longer required just to serve the PAC file.
>>
>> NP: An ERR_AGENT_PACFILE error template is added to display
configuration
>> details. For now, like the manual configuration template an assumption
>> has
>> to be made about the Squid listening ports since we don't yet have any
to
>> dynamically insert the listening port into error pages.
>
> There is no argument that the feature is useful.
>
> Big-picture comments:
>
> * IMO, the proposed implementation is a missed opportunity to support
> not just one specific internally stored object but any number of any
> admin-configured internally stored objects. Instead of bloating code and
> configuration options with each particular use case, we should add a
> single configuration option to map internal URL paths to locally stored
> [error] responses.

I wasn't wanting to go quite that far yet.

>
> For example, the simplest configuration option could work like this:
>
> serve_internally "/wpad.dat" "errors/templates/ERR_WPAD"
> serve_internally "/logo.png" "icons/logo.png"
> ...
>
> The code to handle the above would be almost as simple as the current
> code, but will open up a lot more possibilities. The above can be
> extended with more flexible ACL-driven options in the future, of course.
>

The PAC file needs to still be a special case, either way due to its
unusual content-type.

>
>
> * Will placing the intercept in forward.cc allow caching of the
> response? If yes, can the admin prevent caching somehow? If caching is
> allowed and not preventable, it may be better to prohibit it, so that
> dynamic substitutions inside the error page template continue to work.

As far as error responses can be cached yes.
The default for errors is non-cacheable, but we can add/update any header
in the forward.cc to the StoreEntry after the body has been altered/added
(the PAC file content-type is currently the only alteration).

>
> Lower-level comment:
>
> * Please s/ERR_BLANK/ERR_INTERNAL_NOT_FOUND/ (or similar) and add a
> comment describing the new ERR_ constant.

Okay. Adding ERR_INTERNAL_NOT_FOUND too.

>
> * Should errors/templates/ERR_BLANK file contain some basic "not found"
> text? Seems wrong to serve an empty error response by default.

 ERR_BLANK is still just as useful on its own for "deny_info ERR_BLANK
foo" to allow admin to send back HTTP status and headers but not error
texts. As an alternative to TCP_RESET when that is a bit too much eliding
for some.

Amos
Received on Tue Jun 29 2010 - 23:04:27 MDT