Re: patch: infoemail for err pages

Looking better.

To be picky I would probably collapse some of the memBufPrintf calls,
especially where the data is closely related.

memBufPrintf has the same usage as the standard printf() call except that it
prints into a MemBuf instead of the screen. This means that you can have a
complex argument.. also, I generally find it easier to have constant strings
in the format rather than as a separate string..

    memBufPrintf(mb, "%s", "?subject=CacheErrorInfo%20-%20");
    memBufPrintf(mb, "%s", errorPageName(err->type));

would then become

    memBufPrintf(mb, "?subject=CacheErrorInfo%%20-%%20%s",
errorPageName(err->type));

But on the other hand, I think there is a general data flaw here. You need to
take care that any output is properly URL escaped, not only the constant
strings you are providing, so for each URL argument you should probably print
it into a temporary MemBuf (without URL escapes) and then print the whole
argument URL escaped into the result MemBuf..

I.e. something like the following fragment:

static int
errorDump(ErrorState *err, MemBuf *mb)
{
    MemBuf str = MemBufNULL;

    /* Subject line */
    memBufPrintf(&str, "CacheErrorInfo - %s", errorPageName(err->type));
    memBufPrintf(mb, "?subject=%s", rfc1738_escape(str.buf));

    /* Body */
    memBufReset(&str);
    memBufPrintf(&str, "CacheHost: %s\r\n", getMyHostname());
    memBufPrintf(&str, "ErrPage: %s\r\n", errorPageName(err->type));
    .... [the rest of the code making the body here, printing into &str]
    memBufPrintf(mb, "&body=%s", rfc1738_escape(str.buf));

    memBufClear(&str);
    return 0;
}

Regards
Henrik

PsychoTekk .de wrote:
> hi,
> i hope this time it's perfect :)
> http://www.psychotekk.de/extern/errorpage.diff
>
> the ERR files still need to be modified
> (since search + replace seems easier here
> i did not post a patch)
> "mailto:%w" needs to be extended to
> "mailto:%w%W"
>
> regards,
> Clemens
Received on Fri Sep 06 2002 - 09:51:21 MDT