Allow configuring the Squid response chars limit on purges
Closed, ResolvedPublic

Description

Author: avinoamr

Description:
new $wgSquidResponseLimit configuration directive, with a default value.

SquidUpdate uses a hardcoded configuration for the character limit of the Squid server response (SquidUpdate.php, line 143). This makes it hard to integrate MW into custom compilations of Squid, and/or different reverse proxy applications (like Varnish) that might return a different length of result.

In this patch, I've created a new configuration directive $wgSquidResponseLimit, and set it to 250 by default (same as the hard-coded value).


Version: unspecified
Severity: enhancement

Attached: wgSquidResponseLimit.patch

bzimport added a subscriber: wikibugs-l.
bzimport set Reference to bz21551.
bzimport created this task.Via LegacyNov 17 2009, 2:54 PM
Catrope added a comment.Via ConduitNov 17 2009, 7:05 PM

Patch committed in r59178.

tstarling added a comment.Via ConduitJan 12 2010, 2:14 AM

Reopening since I'm recommending reverting that revision. No compliant HTTP server is going to treat a PURGE like GET, as the original code seems to be implying, so I think that case can just be removed. If you want to detect an error you should look at the response code.

bzimport added a comment.Via ConduitJan 12 2010, 11:23 AM

avinoamr wrote:

Example of a Varnish HTTP response

Attached: varnish_purge_response.txt

bzimport added a comment.Via ConduitJan 12 2010, 11:23 AM

avinoamr wrote:

I disagree.

Varnish allows you to configure whichever output you wish to receive for a PURGE command, and it also adds information about the purge (HTTP response text attached). I realize this might be non-standard, but I believe that keeping MW's code portable (or easy to integrate with different solutions) is a much more meaningful advantage than enforcing standard usage of other products in the market.

Another approach would be to design and implement a similiar VarnishUpdate class, but this might be an overkill.

What do you think?

tstarling added a comment.Via ConduitJan 12 2010, 2:52 PM

I think you're missing the point. The response size limit is a useless paranoid hack for Squid, and is harmful for Varnish, and so should be removed for both. Adding a configuration variable is pointless if the whole code block can be deleted.

bzimport added a comment.Via ConduitJan 13 2010, 8:30 AM

avinoamr wrote:

Oh, I misunderstood. Great solution.

tstarling added a comment.Via ConduitFeb 5 2010, 3:38 AM

Done in r62005.

Add Comment

Column Prototype
This is a very early prototype of a persistent column. It is not expected to work yet, and leaving it open will activate other new features which will break things. Press "\" (backslash) on your keyboard to close it now.