On Mon, Feb 13, 2017 at 11:33:53AM +0100, Michael Hamburger wrote:
> > Thanks for the description. However this change causes a big issue which
> > is that a printf-format file is directly exposed outside. The nasty side
> > effect is that someone using 2 "%s" instead of exactly one will cause
> > the process to randomly crash for example, and there's a warning about
> > it just before their declaration, which explains why they were not merged
> > with the other ones.
> >
> Weren't it easier to parse the custom errorfile and when two or more "%s"
> are found terminate HAProxy and give a error message that only one "%s" must
> be used.
That's absolutely ugly and hackish. At least it's not future-proof.
> > Interestingly your proposal raises a new issue which we didn't think
> > about. Recently a few of us wanted to make the error pages more
> > configurable (typically templates in which we could use some sample
> > expressions, a-la log-format). But your request shows one limit to this
> > model that we also have to take into account.
> >
> > This tends to make me think that most of the time users only need to
> > set the response body and will not care about the headers which will
> > automatically be filled by haproxy. I'm thinking that we could imagine
> > a new way to declare error files with more arguments :
> >
> > errorfile 401 type text/html content 401.html
> >
> > That would remove the need for the "%s" in the www-authenticate field
> > and would make it easier to later implement dynamic contents since our
> > headers would have to be automatically computed already.
> I agree and to be honest I was surprised that this isn't the case already.
It's historic, the original errorfiles had to be pushed as-is onto the
socket, and some people use them to perform some redirects for example.
> For my use-case this is totally satisfactory.
Great!
> > Maybe you're interested in trying to implement the stuff above ? If
> > so, just let me know, I can give you a few hints which could possibly
> > help.
> I would like to help but I need to find some free time for that. If you can
> spare some minutes I would love to get a few hints :)
OK so the first thing is to keep the compatiblity with existing errorfiles
because we don't want all users to have to rewrite theirs *as long as they
are valid*.
For this reason what I'm thinking about is that we'll need to keep the
headers and the body separately. And since we want to have the ability
to emit a header for certain responses (401, 407), the proper way to do
it would be to store :
- the header part from the status line to the last header, without the
empty line ;
- the body
The function in charge of sending the response would simply have to send :
- the header block ;
- the empty line ;
- and the body.
The function which needs to emit the 401/407 would additionally emit the
www-authenticate or proxy-authenticate header after the header block.
If we're capable of doing this, as a first step we simply want to parse
the current errorfiles while reading them and to split them between
header and body. This way without even adding any option to the errorfile
directive, it will be possible to pass your own errorfile 401/407 and
have it properly split then reassembled with the header field inserted.
Then as a second step we can improve it to take the content-type and
contents as in the example above, and to build the header on the fly
while parsing the file. As you can see, once the first step is realized
above, it's easy to implement the second one :-)
Cheers,
Willy
