ProblemsWithCompletePageHandlerAPI

While working on FoswikiCache I came a cross a problem with a relatively new plugin handler, completePageHandler(text, hdr). This is called at the very very end of the render pipeline right before the content is written out to the browser. It allows plugin writers to read and alter the content. It may seem as if you could also alter the http header information but that does not have any effect because at that time the http header has already been responded. Unfortunately this also includes the Content-Length information computed on behalf of the text available so far. This means that changing the text in a completePageHandler() might result in a wrong content length as known so far. At least it looks like.

There are two plugins today that make use of this handler: SafeWikiPlugin and BrowserBoosterPlugin. Both change the text. None change the hdr. SafeWikiPlugin reads the hdr info to find out which content type the text is and only proceeds if this is text/html.

Now lets look a bit closer at the execution order in writeCompletePage() as it is on current SVN/trunk:

sub writeCompletePage {
    my ( $this, $text, $pageType, $contentType ) = @_;
    $contentType ||= 'text/html';

    ...

    $this->generateHTTPHeaders( $pageType, $contentType );
    my $hdr = $this->{response}->printHeaders;

    # Call final handler
    $this->{plugins}->dispatch( 'completePageHandler', $text, $hdr );

    $this->{response}->print($text);
}

As you see writing out the real data is handled by a Foswiki::Response object. Even though as it looks like writeCompletePage() writes out the page, it does not. Foswiki::Response::printHeaders() does not write out any http headers. Foswiki::Response::print() does not write out any text at this moment. Still, http headers have been generated using Foswiki::generateHTTPHeaders() and where committed to the current response object. The response itself is written out much later as part of the Foswiki::Engine::finalize() method. At that time, it probably might contain mismatching content and length information as a consequence of a completePageHandler() altering the text.

My question is: do we really need http header information in writeCompletePage()? It seems as if this is bound to cause trouble of all sorts. The only plugin reading these http response headers is SafeWikiPlugin to determine the content type. This information however is available quite easily as it is an argument to writeCompletePage() already. So I'd propose to alter the writeCompletePage() handler and replace $hdr with $contentType.

-- MichaelDaum - 09 Jun 2009

Discussion

I wasn't aware of the content-length already being responded, when I wrote the BrowserBoosterPlugin. I can live with a leaner completePageHandler.

-- OliverKrueger - 09 Jun 2009

I only wanted the completePageHandler to work on the content, not the headers; but the headers were already there when it went in because of the way TWiki worked. Since Gilmar's refactor, the headers have gone. IMHO no completePageHandler should ever rely on the headers being present. Not having it there is good. I'd be happy to modify SafeWikiPlugin as necessary, if you can point me in the right direction.

-- CrawfordCurrie - 10 Jun 2009

I'd propose to alter the writeCompletePage() handler and replace $hdr with $contentType. SafeWikiPlugin's check for text/html could be rewritten in a trivial way.

-- MichaelDaum - 10 Jun 2009