MINOR: http: move redirect rule processing to its own function

We now have http_apply_redirect_rule() which does all the redirect-specific
job instead of having this inside http_process_req_common().

Also one of the benefit gained from uniformizing this code is that both
keep-alive and close response do emit the PR-- flags. The fix for the
flags could probably be backported to 1.4 though it's very minor.

The previous function http_perform_redirect() was becoming confusing
so it was renamed http_perform_server_redirect() since it only applies
to server-based redirection.

include/proto/proto_http.h

@@ -94,7 +94,7 @@ int http_find_header2(const char *name, int len,
 		      char *sol, struct hdr_idx *idx,
 		      struct hdr_ctx *ctx);
 void http_sess_log(struct session *s);
-void perform_http_redirect(struct session *s, struct stream_interface *si);
+void http_perform_server_redirect(struct session *s, struct stream_interface *si);
 void http_return_srv_error(struct session *s, struct stream_interface *si);
 void http_capture_bad_message(struct error_snapshot *es, struct session *s,
                               struct http_msg *msg,

src/proto_http.c

@@ -749,13 +749,13 @@ http_get_path(struct http_txn *txn)
 	return ptr;
 }
 
-/* Returns a 302 for a redirectable request. This may only be called just after
+/* Returns a 302 for a redirectable request that reaches a server working in
- * the stream interface has moved to SI_ST_ASS. Unprocessable requests are
+ * in redirect mode. This may only be called just after the stream interface
- * left unchanged and will follow normal proxy processing.
+ * has moved to SI_ST_ASS. Unprocessable requests are left unchanged and will
- * NOTE: this function is designed to support being called once data are scheduled
+ * follow normal proxy processing. NOTE: this function is designed to support
- * for forwarding.
+ * being called once data are scheduled for forwarding.
  */
-void perform_http_redirect(struct session *s, struct stream_interface *si)
+void http_perform_server_redirect(struct session *s, struct stream_interface *si)
 {
 	struct http_txn *txn;
 	struct server *srv;
@@ -3129,6 +3129,224 @@ http_req_get_intercept_rule(struct proxy *px, struct list *rules, struct session
 	return NULL;
 }
 
+
+/* Perform an HTTP redirect based on the information in <rule>. The function
+ * returns non-zero on success, or zero in case of a, irrecoverable error such
+ * as too large a request to build a valid response.
+ */
+static int http_apply_redirect_rule(struct redirect_rule *rule, struct session *s, struct http_txn *txn)
+{
+	struct http_msg *msg = &txn->req;
+	const char *msg_fmt;
+
+	/* build redirect message */
+	switch(rule->code) {
+	case 303:
+		msg_fmt = HTTP_303;
+		break;
+	case 301:
+		msg_fmt = HTTP_301;
+		break;
+	case 302:
+	default:
+		msg_fmt = HTTP_302;
+		break;
+	}
+
+	if (unlikely(!chunk_strcpy(&trash, msg_fmt)))
+		return 0;
+
+	switch(rule->type) {
+	case REDIRECT_TYPE_SCHEME: {
+		const char *path;
+		const char *host;
+		struct hdr_ctx ctx;
+		int pathlen;
+		int hostlen;
+
+		host = "";
+		hostlen = 0;
+		ctx.idx = 0;
+		if (http_find_header2("Host", 4, txn->req.chn->buf->p + txn->req.sol, &txn->hdr_idx, &ctx)) {
+			host = ctx.line + ctx.val;
+			hostlen = ctx.vlen;
+		}
+
+		path = http_get_path(txn);
+		/* build message using path */
+		if (path) {
+			pathlen = txn->req.sl.rq.u_l + (txn->req.chn->buf->p + txn->req.sl.rq.u) - path;
+			if (rule->flags & REDIRECT_FLAG_DROP_QS) {
+				int qs = 0;
+				while (qs < pathlen) {
+					if (path[qs] == '?') {
+						pathlen = qs;
+						break;
+					}
+					qs++;
+				}
+			}
+		} else {
+			path = "/";
+			pathlen = 1;
+		}
+
+		/* check if we can add scheme + "://" + host + path */
+		if (trash.len + rule->rdr_len + 3 + hostlen + pathlen > trash.size - 4)
+			return 0;
+
+		/* add scheme */
+		memcpy(trash.str + trash.len, rule->rdr_str, rule->rdr_len);
+		trash.len += rule->rdr_len;
+
+		/* add "://" */
+		memcpy(trash.str + trash.len, "://", 3);
+		trash.len += 3;
+
+		/* add host */
+		memcpy(trash.str + trash.len, host, hostlen);
+		trash.len += hostlen;
+
+		/* add path */
+		memcpy(trash.str + trash.len, path, pathlen);
+		trash.len += pathlen;
+
+		/* append a slash at the end of the location is needed and missing */
+		if (trash.len && trash.str[trash.len - 1] != '/' &&
+		    (rule->flags & REDIRECT_FLAG_APPEND_SLASH)) {
+			if (trash.len > trash.size - 5)
+				return 0;
+			trash.str[trash.len] = '/';
+			trash.len++;
+		}
+
+		break;
+	}
+	case REDIRECT_TYPE_PREFIX: {
+		const char *path;
+		int pathlen;
+
+		path = http_get_path(txn);
+		/* build message using path */
+		if (path) {
+			pathlen = txn->req.sl.rq.u_l + (txn->req.chn->buf->p + txn->req.sl.rq.u) - path;
+			if (rule->flags & REDIRECT_FLAG_DROP_QS) {
+				int qs = 0;
+				while (qs < pathlen) {
+					if (path[qs] == '?') {
+						pathlen = qs;
+						break;
+					}
+					qs++;
+				}
+			}
+		} else {
+			path = "/";
+			pathlen = 1;
+		}
+
+		if (trash.len + rule->rdr_len + pathlen > trash.size - 4)
+			return 0;
+
+		/* add prefix. Note that if prefix == "/", we don't want to
+		 * add anything, otherwise it makes it hard for the user to
+		 * configure a self-redirection.
+		 */
+		if (rule->rdr_len != 1 || *rule->rdr_str != '/') {
+			memcpy(trash.str + trash.len, rule->rdr_str, rule->rdr_len);
+			trash.len += rule->rdr_len;
+		}
+
+		/* add path */
+		memcpy(trash.str + trash.len, path, pathlen);
+		trash.len += pathlen;
+
+		/* append a slash at the end of the location is needed and missing */
+		if (trash.len && trash.str[trash.len - 1] != '/' &&
+		    (rule->flags & REDIRECT_FLAG_APPEND_SLASH)) {
+			if (trash.len > trash.size - 5)
+				return 0;
+			trash.str[trash.len] = '/';
+			trash.len++;
+		}
+
+		break;
+	}
+	case REDIRECT_TYPE_LOCATION:
+	default:
+		if (trash.len + rule->rdr_len > trash.size - 4)
+			return 0;
+
+		/* add location */
+		memcpy(trash.str + trash.len, rule->rdr_str, rule->rdr_len);
+		trash.len += rule->rdr_len;
+		break;
+	}
+
+	if (rule->cookie_len) {
+		memcpy(trash.str + trash.len, "\r\nSet-Cookie: ", 14);
+		trash.len += 14;
+		memcpy(trash.str + trash.len, rule->cookie_str, rule->cookie_len);
+		trash.len += rule->cookie_len;
+		memcpy(trash.str + trash.len, "\r\n", 2);
+		trash.len += 2;
+	}
+
+	/* add end of headers and the keep-alive/close status.
+	 * We may choose to set keep-alive if the Location begins
+	 * with a slash, because the client will come back to the
+	 * same server.
+	 */
+	txn->status = rule->code;
+	/* let's log the request time */
+	s->logs.tv_request = now;
+
+	if (rule->rdr_len >= 1 && *rule->rdr_str == '/' &&
+	    (msg->flags & HTTP_MSGF_XFER_LEN) &&
+	    !(msg->flags & HTTP_MSGF_TE_CHNK) && !txn->req.body_len &&
+	    ((txn->flags & TX_CON_WANT_MSK) == TX_CON_WANT_SCL ||
+	     (txn->flags & TX_CON_WANT_MSK) == TX_CON_WANT_KAL)) {
+		/* keep-alive possible */
+		if (!(msg->flags & HTTP_MSGF_VER_11)) {
+			if (unlikely(txn->flags & TX_USE_PX_CONN)) {
+				memcpy(trash.str + trash.len, "\r\nProxy-Connection: keep-alive", 30);
+				trash.len += 30;
+			} else {
+				memcpy(trash.str + trash.len, "\r\nConnection: keep-alive", 24);
+				trash.len += 24;
+			}
+		}
+		memcpy(trash.str + trash.len, "\r\n\r\n", 4);
+		trash.len += 4;
+		bo_inject(txn->rsp.chn, trash.str, trash.len);
+		/* "eat" the request */
+		bi_fast_delete(txn->req.chn->buf, msg->sov);
+		msg->sov = 0;
+		txn->req.chn->analysers = AN_REQ_HTTP_XFER_BODY;
+		s->rep->analysers = AN_RES_HTTP_XFER_BODY;
+		txn->req.msg_state = HTTP_MSG_CLOSED;
+		txn->rsp.msg_state = HTTP_MSG_DONE;
+	} else {
+		/* keep-alive not possible */
+		if (unlikely(txn->flags & TX_USE_PX_CONN)) {
+			memcpy(trash.str + trash.len, "\r\nProxy-Connection: close\r\n\r\n", 29);
+			trash.len += 29;
+		} else {
+			memcpy(trash.str + trash.len, "\r\nConnection: close\r\n\r\n", 23);
+			trash.len += 23;
+		}
+		stream_int_retnclose(txn->req.chn->prod, &trash);
+		txn->req.chn->analysers = 0;
+	}
+
+	if (!(s->flags & SN_ERR_MASK))
+		s->flags |= SN_ERR_PRXCOND;
+	if (!(s->flags & SN_FINST_MASK))
+		s->flags |= SN_FINST_R;
+
+	return 1;
+}
+
 /* This stream analyser runs all HTTP request processing which is common to
  * frontends and backends, which means blocking ACLs, filters, connection-close,
  * reqadd, stats and redirects. This is performed for the designated proxy.
@@ -3346,219 +3564,21 @@ int http_process_req_common(struct session *s, struct channel *req, int an_bit,
 
 	/* check whether we have some ACLs set to redirect this request */
 	list_for_each_entry(rule, &px->redirect_rules, list) {
-		int ret = ACL_PAT_PASS;
-
 		if (rule->cond) {
+			int ret;
+
 			ret = acl_exec_cond(rule->cond, px, s, txn, SMP_OPT_DIR_REQ|SMP_OPT_FINAL);
 			ret = acl_pass(ret);
 			if (rule->cond->pol == ACL_COND_UNLESS)
 				ret = !ret;
+			if (!ret)
+				continue;
 		}
-
+		if (!http_apply_redirect_rule(rule, s, txn))
-		if (ret) {
-			const char *msg_fmt;
-
-			/* build redirect message */
-			switch(rule->code) {
-			case 303:
-				msg_fmt = HTTP_303;
-				break;
-			case 301:
-				msg_fmt = HTTP_301;
-				break;
-			case 302:
-			default:
-				msg_fmt = HTTP_302;
-				break;
-			}
-
-			if (unlikely(!chunk_strcpy(&trash, msg_fmt)))
 			goto return_bad_req;
 
-			switch(rule->type) {
+		req->analyse_exp = TICK_ETERNITY;
-			case REDIRECT_TYPE_SCHEME: {
+		return 1;
-				const char *path;
-				const char *host;
-				struct hdr_ctx ctx;
-				int pathlen;
-				int hostlen;
-
-				host = "";
-				hostlen = 0;
-				ctx.idx = 0;
-				if (http_find_header2("Host", 4, txn->req.chn->buf->p + txn->req.sol, &txn->hdr_idx, &ctx)) {
-					host = ctx.line + ctx.val;
-					hostlen = ctx.vlen;
-				}
-
-				path = http_get_path(txn);
-				/* build message using path */
-				if (path) {
-					pathlen = txn->req.sl.rq.u_l + (req->buf->p + txn->req.sl.rq.u) - path;
-					if (rule->flags & REDIRECT_FLAG_DROP_QS) {
-						int qs = 0;
-						while (qs < pathlen) {
-							if (path[qs] == '?') {
-								pathlen = qs;
-								break;
-							}
-							qs++;
-						}
-					}
-				} else {
-					path = "/";
-					pathlen = 1;
-				}
-
-				/* check if we can add scheme + "://" + host + path */
-				if (trash.len + rule->rdr_len + 3 + hostlen + pathlen > trash.size - 4)
-					goto return_bad_req;
-
-				/* add scheme */
-				memcpy(trash.str + trash.len, rule->rdr_str, rule->rdr_len);
-				trash.len += rule->rdr_len;
-
-				/* add "://" */
-				memcpy(trash.str + trash.len, "://", 3);
-				trash.len += 3;
-
-				/* add host */
-				memcpy(trash.str + trash.len, host, hostlen);
-				trash.len += hostlen;
-
-				/* add path */
-				memcpy(trash.str + trash.len, path, pathlen);
-				trash.len += pathlen;
-
-				/* append a slash at the end of the location is needed and missing */
-				if (trash.len && trash.str[trash.len - 1] != '/' &&
-				    (rule->flags & REDIRECT_FLAG_APPEND_SLASH)) {
-					if (trash.len > trash.size - 5)
-						goto return_bad_req;
-					trash.str[trash.len] = '/';
-					trash.len++;
-				}
-
-				break;
-			}
-			case REDIRECT_TYPE_PREFIX: {
-				const char *path;
-				int pathlen;
-
-				path = http_get_path(txn);
-				/* build message using path */
-				if (path) {
-					pathlen = txn->req.sl.rq.u_l + (req->buf->p + txn->req.sl.rq.u) - path;
-					if (rule->flags & REDIRECT_FLAG_DROP_QS) {
-						int qs = 0;
-						while (qs < pathlen) {
-							if (path[qs] == '?') {
-								pathlen = qs;
-								break;
-							}
-							qs++;
-						}
-					}
-				} else {
-					path = "/";
-					pathlen = 1;
-				}
-
-				if (trash.len + rule->rdr_len + pathlen > trash.size - 4)
-					goto return_bad_req;
-
-				/* add prefix. Note that if prefix == "/", we don't want to
-				 * add anything, otherwise it makes it hard for the user to
-				 * configure a self-redirection.
-				 */
-				if (rule->rdr_len != 1 || *rule->rdr_str != '/') {
-					memcpy(trash.str + trash.len, rule->rdr_str, rule->rdr_len);
-					trash.len += rule->rdr_len;
-				}
-
-				/* add path */
-				memcpy(trash.str + trash.len, path, pathlen);
-				trash.len += pathlen;
-
-				/* append a slash at the end of the location is needed and missing */
-				if (trash.len && trash.str[trash.len - 1] != '/' &&
-				    (rule->flags & REDIRECT_FLAG_APPEND_SLASH)) {
-					if (trash.len > trash.size - 5)
-						goto return_bad_req;
-					trash.str[trash.len] = '/';
-					trash.len++;
-				}
-
-				break;
-			}
-			case REDIRECT_TYPE_LOCATION:
-			default:
-				if (trash.len + rule->rdr_len > trash.size - 4)
-					goto return_bad_req;
-
-				/* add location */
-				memcpy(trash.str + trash.len, rule->rdr_str, rule->rdr_len);
-				trash.len += rule->rdr_len;
-				break;
-			}
-
-			if (rule->cookie_len) {
-				memcpy(trash.str + trash.len, "\r\nSet-Cookie: ", 14);
-				trash.len += 14;
-				memcpy(trash.str + trash.len, rule->cookie_str, rule->cookie_len);
-				trash.len += rule->cookie_len;
-				memcpy(trash.str + trash.len, "\r\n", 2);
-				trash.len += 2;
-			}
-
-			/* add end of headers and the keep-alive/close status.
-			 * We may choose to set keep-alive if the Location begins
-			 * with a slash, because the client will come back to the
-			 * same server.
-			 */
-			txn->status = rule->code;
-			/* let's log the request time */
-			s->logs.tv_request = now;
-
-			if (rule->rdr_len >= 1 && *rule->rdr_str == '/' &&
-			    (msg->flags & HTTP_MSGF_XFER_LEN) &&
-			    !(msg->flags & HTTP_MSGF_TE_CHNK) && !txn->req.body_len &&
-			    ((txn->flags & TX_CON_WANT_MSK) == TX_CON_WANT_SCL ||
-			     (txn->flags & TX_CON_WANT_MSK) == TX_CON_WANT_KAL)) {
-				/* keep-alive possible */
-				if (!(msg->flags & HTTP_MSGF_VER_11)) {
-					if (unlikely(txn->flags & TX_USE_PX_CONN)) {
-						memcpy(trash.str + trash.len, "\r\nProxy-Connection: keep-alive", 30);
-						trash.len += 30;
-					} else {
-						memcpy(trash.str + trash.len, "\r\nConnection: keep-alive", 24);
-						trash.len += 24;
-					}
-				}
-				memcpy(trash.str + trash.len, "\r\n\r\n", 4);
-				trash.len += 4;
-				bo_inject(req->prod->ob, trash.str, trash.len);
-				/* "eat" the request */
-				bi_fast_delete(req->buf, msg->sov);
-				msg->sov = 0;
-				req->analysers = AN_REQ_HTTP_XFER_BODY;
-				s->rep->analysers = AN_RES_HTTP_XFER_BODY;
-				txn->req.msg_state = HTTP_MSG_CLOSED;
-				txn->rsp.msg_state = HTTP_MSG_DONE;
-				break;
-			} else {
-				/* keep-alive not possible */
-				if (unlikely(txn->flags & TX_USE_PX_CONN)) {
-					memcpy(trash.str + trash.len, "\r\nProxy-Connection: close\r\n\r\n", 29);
-					trash.len += 29;
-				} else {
-					memcpy(trash.str + trash.len, "\r\nConnection: close\r\n\r\n", 23);
-					trash.len += 23;
-				}
-				stream_int_retnclose(req->prod, &trash);
-				goto return_prx_cond;
-			}
-		}
 	}
 
 	/* POST requests may be accompanied with an "Expect: 100-Continue" header.

src/session.c

@@ -2198,7 +2198,7 @@ struct task *process_session(struct task *t)
 
 			srv = objt_server(s->target);
 			if (s->si[1].state == SI_ST_ASS && srv && srv->rdr_len && (s->flags & SN_REDIRECTABLE))
-				perform_http_redirect(s, &s->si[1]);
+				http_perform_server_redirect(s, &s->si[1]);
 		} while (s->si[1].state == SI_ST_ASS);
 
 		/* Now we can add the server name to a header (if requested) */