811 Commits

Author SHA1 Message Date
Willy Tarreau
507385d0e1 [MEDIUM] centralize buffer timeout checks at the top of process_session
it's more efficient and easier to check all the timeouts at once and
always rely on the buffer flags than to check them everywhere.
2008-08-17 13:04:25 +02:00
Willy Tarreau
26ed74dadc [MEDIUM] use buffer->wex instead of buffer->cex for connect timeout
It's a shame not to use buffer->wex for connection timeouts since by
definition it cannot be used till the connection is not established.
Using it instead of ->cex also makes the buffer processing more
symmetric.
2008-08-17 12:11:14 +02:00
Willy Tarreau
dafde43410 [MAJOR] process_session: rely only on buffer flags
Instead of calling all functions in a loop, process_session now
calls them according to buffer flags changes. This ensures that
we almost never call functions for nothing. The flags settings
are still quite coarse, but the number of average functions
calls per session has dropped from 31 to 18 (the calls to
process_srv dropped from 13 to 7 and the calls to process_cli
dropped from 13 to 8).

This could still be improved by memorizing which flags each
function uses, but that would add a level of complexity which
is not desirable and maybe even not worth the small gain.
2008-08-17 01:15:41 +02:00
Willy Tarreau
e393fe224b [MEDIUM] buffers: add BF_EMPTY and BF_FULL to remove dependency on req/rep->l
It is not always convenient to run checks on req->l in functions to
check if a buffer is empty or full. Now the stream_sock functions
set flags BF_EMPTY and BF_FULL according to the buffer contents. Of
course, functions which touch the buffer contents adjust the flags
too.
2008-08-16 22:18:07 +02:00
Willy Tarreau
ba392cecf9 [CLEANUP] get rid of BF_SHUT*_PENDING
BF_SHUTR_PENDING and BF_SHUTW_PENDING were poor ideas because
BF_SHUTR is the pending of BF_SHUTW_DONE and BF_SHUTW is the
pending of BF_SHUTR_DONE. Remove those two useless and confusing
"pending" versions and rename buffer_shut{r,w}_* functions.
2008-08-16 21:13:23 +02:00
Willy Tarreau
d5382b4aaa [BUG] maintain_proxies must not disable backends
maintain_proxies could disable backends (p->maxconn == 0) which is
wrong (but apparently harmless). Add a check for p->maxconn == 0.
2008-08-16 18:41:13 +02:00
Willy Tarreau
a7c52761b4 [BUG] process_response: do not touch srv_state
process_response is not allowed to touch srv_state (this is an
incident which has survived the code migration). This bug was
causing connection exhaustion on frontend due to some closed
sockets marked SV_STDATA again.
2008-08-16 18:40:18 +02:00
Willy Tarreau
d9f483646d [BUG] buffers: remove BF_MAY_CONNECT and fix forwarding issue
It wasn't really wise to separate BF_MAY_CONNECT and BF_MAY_FORWARD,
as it caused trouble in TCP mode because the connection was allowed
but not the forwarding. Remove BF_MAY_CONNECT.
2008-08-16 16:39:26 +02:00
Willy Tarreau
9a8c5de375 [BUG] process_response must not enable the read FD
Since the separation of TCP and HTTP state machines, the HTTP
code must not play anymore with the file descriptor status
without checking if they are closed. Remains of such practice
have caused busy loops under some circumstances (mainly when
client closed during headers response).
2008-08-16 16:11:07 +02:00
Willy Tarreau
7a52a5c468 [BUG] ev_sepoll: closed file descriptors could persist in the spec list
If __fd_clo() was called on a file descriptor which was previously
disabled, it was not removed from the spec list. This apparently
could not happen on previous code because the TCP states prevented
this, but now it happens regularly. The effects are spec entries
stuck populated, leading to busy loops.
2008-08-16 16:06:02 +02:00
Willy Tarreau
f853320b44 [MINOR] term_trace: add better instrumentations to trace the code
A new member has been added to the struct session. It keeps a trace
of what block of code performs a close or a shutdown on a socket, and
in what sequence. This is extremely convenient for post-mortem analysis
where flag combinations and states seem impossible. A new ABORT_NOW()
macro has also been added to make the code immediately segfault where
called.
2008-08-16 14:55:08 +02:00
Willy Tarreau
1ae3a057df [MEDIUM] remove unused references to {CL|SV}_STSHUT*
All references to CL_STSHUT* and SV_STSHUT* were removed where
possible. Some of them could not be removed because they are
still in use by the unix sockets.

A bug remains at this stage. Injecting with a very short timeout
sometimes leads to a client in close state and a server in data
state with all buffer flags indicating a shutdown but the server
fd still enable, thus causing a busy loop.
2008-08-16 10:56:30 +02:00
Willy Tarreau
461f662846 [MAJOR] clearly separate HTTP response processing from TCP server state
The HTTP response is now processed in its own function, regardless of
the TCP state. All FSMs have become fairly simpler and must still be
improved by removing useless CL_STSHUT* and SV_STSHUT* (still used by
proto_uxst). The number of calls to process_* is still huge though.

Next steps consist in :
  - removing useless assignments of CL_STSHUT* and SV_STSHUT*
  - add a BF_EMPTY flag to buffers to indicate an empty buffer
  - returning smarter values in process_* so that each callee
    may explicitly indicate whom needs to be called after it.
  - unify read and write timeouts for a same side. The way it
    is now is too complicated and error-prone
  - auditing code for regression testing

We're close to getting something which works fairly better now.
2008-08-15 23:43:19 +02:00
Willy Tarreau
cebf57e0bf [MAJOR] better separation of response processing and server state
TCP timeouts are not managed anymore by the response FSM. Warning,
the FORCE_CLOSE state does not work anymore for now. All remaining
bugs causing stale connections have been swept.
2008-08-15 18:16:37 +02:00
Willy Tarreau
f5483bf639 [MAJOR] get rid of the SV_STHEADERS state
The HTTP response code has been moved to a specific function
called "process_response" and the SV_STHEADERS state has been
removed and replaced with the flag AN_RTR_HTTP_HDR.
2008-08-14 18:35:40 +02:00
Willy Tarreau
e46ab5524f [BUG] fix recently introduced loop when client closes early
Due to a recent change in the FSMs, if the client closes with buffer
full, then the server loops waiting for headers. We can safely ignore
this case since the server FSM will have to be reworked too. Let's
fix the root cause for now.
2008-08-14 00:18:39 +02:00
Willy Tarreau
c65a3ba3d4 [MAJOR] completely separate HTTP and TCP states on the request path
For the first time, HTTP and TCP are not merged anymore. All request
processing has moved to process_request while the TCP processing of
the frontend remains in process_cli. The code is a lot cleaner,
simpler, smaller (1%) and slightly faster (1% too).

Right now, the HTTP state machine cannot easily command the TCP
state machine, but it does not cause that many difficulties.

The response processing has not yet been extracted, and the unix-stream
state machines have to be broken down that way too.

The CL_STDATA, CL_STSHUTR and CL_STSHUTW states still exist and are
exactly the sames. They will have to be all merged into CL_STDATA
once the work has stabilized. It is also possible that this single
state will disappear in favor of just buffer flags.
2008-08-14 00:18:39 +02:00
Willy Tarreau
7f875f6c8f [MEDIUM] simplify and centralize request timeout cancellation and request forwarding
Instead of playing with req->flags and request timeout everywhere,
tweak them only at precise locations.
2008-08-14 00:18:38 +02:00
Willy Tarreau
adfb8569f7 [MAJOR] get rid of SV_STANALYZE (step 2)
The SV_STANALYZE state was installed on the server side but was really
meant to be processed with the rest of the request on the client side.
It suffered from several issues, mostly related to the way timeouts were
handled while waiting for data.

All known issues related to timeouts during a request - and specifically
a request involving body processing - have been raised and fixed. At this
point, the code is a bit dirty but works fine, so next steps might be
cleanups with an ability to come back to the current state in case of
trouble.
2008-08-14 00:18:38 +02:00
Willy Tarreau
67f0eead22 [MAJOR] kill CL_STINSPECT and CL_STHEADERS (step 1)
This is a first attempt at separating data processing from the
TCP state machine. Those two states have been replaced with flags
in the session indicating what needs to be analyzed. The corresponding
code is still called before and in lieu of TCP states.

Next change should get rid of the specific SV_STANALYZE which is in
fact a client state.

Then next change should consist in making it possible to analyze
TCP contents while being in CL_STDATA (or CL_STSHUT*).
2008-08-14 00:18:38 +02:00
Aleksandar Lazic
697bbb0106 [PATCH] appsessions: cleanup DEBUG_HASH and initialize request_counter
This patch cleanup the -DDEBUG=DEBUG_HASH output setting and initialize
the request_counter for the appsessions.
2008-08-13 23:43:26 +02:00
Willy Tarreau
9f1f24bb7f [BUG] client timeout incorrectly rearmed while waiting for server
Client timeout could be refreshed in stream_sock_*, but this is
undesired when the timeout is already set to eternity. The effect
is that a session could still be aborted if client timeout was
smaller than server timeout. A second effect is that sessions
expired on the server side would expire with "cD" flags.

The fix consists in not updating it if it was not previously set.
A cleaner method might consist in updating the buffer timeout. This
is probably what will be done later when the state machines only
deal with the buffers.
2008-08-11 11:34:18 +02:00
Willy Tarreau
ce09c52187 [BUG] server timeout was not considered in some circumstances
Due to a copy-paste typo, the client timeout was refreshed instead
of the server's when waiting for server response. This means that
the server's timeout remained eternity.
2008-08-11 11:34:16 +02:00
Willy Tarreau
fb0528bd56 [BUG] fix segfault with url_param + check_post
If an HTTP/0.9-like POST request is sent to haproxy while
configured with url_param + check_post, it will crash. The
reason is that the total buffer length was computed based
on req->total (which equals the number of bytes read) and
not req->l (number of bytes in the buffer), thus leading
to wrong size calculations when calling memchr().

The affected code does not look like it could have been
exploited to run arbitrary code, only reads were performed
at wrong locations.
2008-08-11 11:34:01 +02:00
Willy Tarreau
718f0ef129 [MEDIUM] process_cli: don't rely at all on server state
A new buffer flag BF_MAY_FORWARD has been added so that the client
FSM can check whether it is allowed to forward the response to the
client. The client FSM does not have to monitor the server state
anymore.
2008-08-10 16:21:32 +02:00
Willy Tarreau
dc0a6a0dea [MEDIUM] process_srv: don't rely at all on client state
A new buffer flag BF_MAY_CONNECT has been added so that the server
FSM can check whether it is allowed to establish a connection or
not. That way, the client FSM only has to move this flag and the
server side does not need to monitor client state anymore.
2008-08-03 22:47:10 +02:00
Willy Tarreau
6468d924ea [MEDIUM] process_srv: rely on buffer flags for client shutdown
The open/close nature of each half of the client side is known
to the buffer, so let the server state machine rely on this
instead of checking the client state for CL_STSHUT* or
CL_STCLOSE.
2008-08-03 20:48:51 +02:00
Willy Tarreau
89edf5e629 [MEDIUM] buffers: ensure buffer_shut* are properly called upon shutdowns
It is important that buffer states reflect the state of both sides so
that we can remove client and server state inter-dependencies.
2008-08-03 20:48:50 +02:00
Willy Tarreau
48d63db7a8 [MEDIUM] memory: update pool_free2() to support NULL pointers
In order to make pool usage more convenient, let pool_free2()
support NULL pointers by doing nothing, just like the standard
free(3) call does.

The various call places have been updated to remove the now
useless checks.
2008-08-03 20:48:50 +02:00
Willy Tarreau
a534fea478 [CLEANUP] remove 65 useless NULL checks before free
C specification clearly states that free(NULL) is a no-op.
So remove useless checks before calling free.
2008-08-03 20:48:50 +02:00
Ross West
af72a1d8ec [MINOR] permit renaming of x-forwarded-for header
Because I needed it in my situation - here's a quick patch to
allow changing of the "x-forwarded-for" header by using a suboption to
"option forwardfor".

Suboption "header XYZ" will set the header from "x-forwarded-for" to "XYZ".

Default is still "x-forwarded-for" if the header value isn't defined.
Also the suboption 'except a.b.c.d/z' still works on the same line.

So it's now: option forwardfor [except a.b.c.d[/z]] [header XYZ]
2008-08-03 10:51:45 +02:00
Willy Tarreau
dd64f8d394 [MEDIUM] acl: when possible, report the name and requirements of ACLs in warnings
When an ACL is referenced at a wrong place (eg: response during request, layer7
during layer4), try to indicate precisely the name and requirements of this ACL.

Only the first faulty ACL is returned. A small change consisting in iterating
that way may improve reports :
   cap = ACL_USE_any_unexpected
   while ((acl=cond_find_require(cond, cap))) {
     warning()
     cap &= ~acl->requires;
   }

This will report the first ACL of each unsupported type. But doing so will
mangle the error reporting a lot, so we need to rework error reports first.
2008-08-03 09:41:05 +02:00
Willy Tarreau
0ceba5af74 [MEDIUM] acl: set types on all currently known ACL verbs
All currently known ACL verbs have been assigned a type which makes
it possible to detect inconsistencies, such as response values used
in request rules.
2008-07-25 19:31:03 +02:00
Willy Tarreau
a9802633d8 [MEDIUM] acl: enforce ACL type checking
ACL now hold information on the availability of the data they rely
on. They can indicate which parts of the requests/responses they
require, and the rules parser may now report inconsistencies.

As an example, switching rules are now checked for response-specific
ACLs, though those are not still set. A warning is reported in case
of mismatch. ACLs keyword restrictions will now have to be specifically
set wherever a better control is expected.

The line number where an ACL condition is declared has been added to
the conditions in order to be able to report the faulty line number
during post-loading checks.
2008-07-25 19:13:19 +02:00
Willy Tarreau
b6fb420c7e [MINOR] acl: add the "wait_end" acl verb
The new "wait_end" acl delays evaluation of the rule (and the next ones)
to the end of the analysis period. This is intented to be used with TCP
content analysis. A rule referencing such an ACL will not match until
the delay is over. An equivalent default ACL "WAIT_END" has been created.
2008-07-20 11:18:28 +02:00
Willy Tarreau
58393e103f [MEDIUM] acl: get rid of dummy values in always_true/always_false
make use of last change in order to get rid of dummy values in
always_true/always_false.
2008-07-20 10:39:22 +02:00
Willy Tarreau
a79534fce1 [MEDIUM] acl: permit fetch() functions to set the result themselves
For protocol analysis, it's not always convenient to have to run through
a fetch then a match against dummy values. It's easier to let the fetch()
function set the result itself. This obviously works only for boolean
values.
2008-07-20 10:17:20 +02:00
Willy Tarreau
c6317703ce [MINOR] acl: add REQ_CONTENT to the list of default acls
With content inspection, checking the presence of data in the
request buffer is very important. It's getting boring to always
add such an ACL, so let's add it by default.
2008-07-20 09:29:50 +02:00
Willy Tarreau
62644770cf [DOC] document the new "tcp-request" keyword and associated ACLs
The update concerns :
  - tcp-request inspect-delay
  - tcp-request accept
  - tcp-request reject
  - acl ... req_len
  - acl ... req_ssl_ver
2008-07-16 18:36:06 +02:00
Willy Tarreau
177e2b0127 [CLEANUP] remove dependency on obsolete INTBITS macro
The INTBITS macro was found to be already defined on some platforms,
and to equal 32 (while INTBITS was 5 here). Due to pure luck, there
was no declaration conflict, but it's nonetheless a problem to fix.

Looking at the code showed that this macro was only used for left
shifts and nothing else anymore. So the replacement is obvious. The
new macro, BITS_PER_INT is more obviously correct.
2008-07-16 10:30:44 +02:00
Willy Tarreau
ec6c5df018 [CLEANUP] remove many #include <types/xxx> from C files
It should be stated as a rule that a C file should never
include types/xxx.h when proto/xxx.h exists, as it gives
less exposure to declaration conflicts (one of which was
caught and fixed here) and it complicates the file headers
for nothing.

Only types/global.h, types/capture.h and types/polling.h
have been found to be valid includes from C files.
2008-07-16 10:30:42 +02:00
Willy Tarreau
284648e079 [CLEANUP] remove unused include/types/client.h
This file is not used anymore.
2008-07-16 10:30:40 +02:00
Willy Tarreau
655e26af24 [MINOR] acl: add req_ssl_ver in TCP, to match an SSL version
This new keyword matches an dotted version mapped into an integer.
It permits to match an SSL message protocol version just as if it
was an integer, so that it is easy to map ranges, like this :

	acl obsolete_ssl  req_ssl_ver   lt 3
	acl correct_ssl   req_ssl_ver   3.0-3.1
	acl invalid_ssl   req_ssl_ver   gt 3.1

Both SSLv2 hello messages and SSLv3 messages are supported. The
test tries to be strict enough to avoid being easily fooled. In
particular, it waits for as many bytes as announced in the message
header if this header looks valid (bound to the buffer size).

The same decoder will be usable with minor changes to check the
response messages.
2008-07-16 10:30:06 +02:00
Willy Tarreau
4a26d2f2fa [MINOR] acl: add a new parsing function: parse_dotted_ver
This new function supports one major and one minor and makes an int of them.
It is very convenient to compare versions (eg: SSL) just as if they were plain
integers, as the comparison functions will still be based on integers.
2008-07-16 10:29:51 +02:00
Willy Tarreau
b686644ad8 [MAJOR] implement tcp request content inspection
Some people need to inspect contents of TCP requests before
deciding to forward a connection or not. A future extension
of this demand might consist in selecting a server farm
depending on the protocol detected in the request.

For this reason, a new state CL_STINSPECT has been added on
the client side. It is immediately entered upon accept() if
the statement "tcp-request inspect-delay <xxx>" is found in
the frontend configuration. Haproxy will then wait up to
this amount of time trying to find a matching ACL, and will
either accept or reject the connection depending on the
"tcp-request content <action> {if|unless}" rules, where
<action> is either "accept" or "reject".

Note that it only waits that long if no definitive verdict
can be found earlier. That generally implies calling a fetch()
function which does not have enough information to decode
some contents, or a match() function which only finds the
beginning of what it's looking for.

It is only at the ACL level that partial data may be processed
as such, because we need to distinguish between MISS and FAIL
*before* applying the term negation.

Thus it is enough to add "| ACL_PARTIAL" to the last argument
when calling acl_exec_cond() to indicate that we expect
ACL_PAT_MISS to be returned if some data is missing (for
fetch() or match()). This is the only case we may return
this value. For this reason, the ACL check in process_cli()
has become a lot simpler.

A new ACL "req_len" of type "int" has been added. Right now
it is already possible to drop requests which talk too early
(eg: for SMTP) or which don't talk at all (eg: HTTP/SSL).

Also, the acl fetch() functions have been extended in order
to permit reporting of missing data in case of fetch failure,
using the ACL_TEST_F_MAY_CHANGE flag.

The default behaviour is unchanged, and if no rule matches,
the request is accepted.

As a side effect, all layer 7 fetching functions have been
cleaned up so that they now check for the validity of the
layer 7 pointer before dereferencing it.
2008-07-16 10:29:07 +02:00
Willy Tarreau
9de1bbd004 [MEDIUM] modularize the "timeout" keyword configuration parser
The "timeout" keyword already relied on an external parser, let's
make use of the new keyword registration mechanism.
2008-07-09 20:34:27 +02:00
Willy Tarreau
39f23b6c7e [MINOR] cfgparse: add support for warnings in external functions
Some parsers will need to report warnings in some cases. Let's
use positive values for that.
2008-07-09 20:23:15 +02:00
Willy Tarreau
10522fd113 [MEDIUM] modularize the global "stats" keyword configuration parser
The "stats" keyword already relied on an external parser, let's
make use of the new keyword registration mechanism.
2008-07-09 20:12:41 +02:00
Willy Tarreau
5b2c33683b [MEDIUM] add support for configuration keyword registration
Any module which needs configuration keywords may now dynamically
register a keyword in a given section, and associate it with a
configuration parsing function using cfg_register_keywords() from
a constructor function. This makes the configuration parser more
modular because it is not required anymore to touch cfg_parse.c.
Example :

static int parse_global_blah(char **args, int section_type, struct proxy *curpx,
                             struct proxy *defpx, char *err, int errlen)
{
	printf("parsing blah in global section\n");
	return 0;
}

static int parse_listen_blah(char **args, int section_type, struct proxy *curpx,
		      struct proxy *defpx, char *err, int errlen)
{
	printf("parsing blah in listen section\n");
	if (*args[1]) {
		snprintf(err, errlen, "missing arg for listen_blah!!!");
		return -1;
	}
	return 0;
}

static struct cfg_kw_list cfg_kws = {{ },{
	{ CFG_GLOBAL, "blah", parse_global_blah },
	{ CFG_LISTEN, "blah", parse_listen_blah },
	{ 0, NULL, NULL },
}};

__attribute__((constructor))
static void __module_init(void)
{
	cfg_register_keywords(&cfg_kws);
}
2008-07-09 19:44:58 +02:00
Willy Tarreau
11382813a1 [TESTS] added test-acl.cfg to test some ACL combinations
various rules constructions can be tested with this test case.
2008-07-09 16:18:21 +02:00