Last modified: 2010-02-05 03:38:06 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T23551, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 21551 - Allow configuring the Squid response chars limit on purges
Allow configuring the Squid response chars limit on purges
Status: RESOLVED FIXED
Product: MediaWiki
Classification: Unclassified
General/Unknown (Other open bugs)
unspecified
All All
: Normal enhancement (vote)
: ---
Assigned To: Nobody - You can work on this!
: patch, patch-need-review
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2009-11-17 14:54 UTC by Roi Avinoam
Modified: 2010-02-05 03:38 UTC (History)
3 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments
new $wgSquidResponseLimit configuration directive, with a default value. (1.27 KB, patch)
2009-11-17 14:54 UTC, Roi Avinoam
Details
Example of a Varnish HTTP response (796 bytes, text/plain)
2010-01-12 11:23 UTC, Roi Avinoam
Details

Description Roi Avinoam 2009-11-17 14:54:12 UTC
Created attachment 6796 [details]
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).
Comment 1 Roan Kattouw 2009-11-17 19:05:40 UTC
Patch committed in r59178.
Comment 2 Tim Starling 2010-01-12 02:14:06 UTC
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.
Comment 3 Roi Avinoam 2010-01-12 11:23:02 UTC
Created attachment 6949 [details]
Example of a Varnish HTTP response
Comment 4 Roi Avinoam 2010-01-12 11:23:30 UTC
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?
Comment 5 Tim Starling 2010-01-12 14:52:33 UTC
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.
Comment 6 Roi Avinoam 2010-01-13 08:30:43 UTC
Oh, I misunderstood. Great solution.
Comment 7 Tim Starling 2010-02-05 03:38:06 UTC
Done in r62005.

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links