On Sun, Nov 15, 2015 at 06:41:49PM +0100, Stanislaw Adaszewski wrote:
> Sorry again, I'm trying now with with patch as an attachment -
> when I did it the first time I think it wasn't accepted by the
> list.
>
> On Fri, Nov 13, 2015 at 05:22:22PM +0100, Bret Lambert wrote:
> > tech@ accepts attachments; don't make this harder for us than
> > it needs to be, please.
> >
> > On Fri, Nov 13, 2015 at 05:06:47PM +0100, Stanislaw Adaszewski wrote:
> > > Excuse me, the indentation was removed because message wasn't plain text.
> > >
> > > Patch: http://pastebin.com/XnZLEPEk
> > >
> > > httpd.conf: http://pastebin.com/gGp3NqCD
> > >
> > > rewr.php: http://pastebin.com/prA1NJXC
> > >
OK, thanks, now I got the diff.
It is an interesting and simple approach, but neither florian@ nor me
had the time to look at it yet. See comments below.
> Index: parse.y
> ===================================================================
> RCS file: /cvs/src/usr.sbin/httpd/parse.y,v
> retrieving revision 1.73
> diff -u -p -u -r1.73 parse.y
> --- parse.y 19 Jul 2015 05:17:27 -0000 1.73
> +++ parse.y 13 Nov 2015 08:29:10 -0000
> @@ -907,7 +907,7 @@ filter : block RETURN NUMBER optstring
>
> if ($4 != NULL) {
> /* Only for 3xx redirection headers */
> - if ($3 < 300 || $3 > 399) {
> + if ($3 != 200 && ($3 < 300 || $3 > 399)) {
I'd add it as "pass rewrite" instead. It is not blocking the connection.
> yyerror("invalid return code for "
> "location URI");
> free($4);
> Index: server_http.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/httpd/server_http.c,v
> retrieving revision 1.96
> diff -u -p -u -r1.96 server_http.c
> --- server_http.c 31 Jul 2015 00:10:51 -0000 1.96
> +++ server_http.c 13 Nov 2015 08:29:10 -0000
> @@ -1054,6 +1054,7 @@ server_response(struct httpd *httpd, str
> int portval = -1, ret;
> char *hostval;
> const char *errstr = NULL;
> + char buf[IBUF_READ_SIZE];
>
> /* Canonicalize the request path */
> if (desc->http_path == NULL ||
> @@ -1154,11 +1155,44 @@ server_response(struct httpd *httpd, str
> resp->http_method = desc->http_method;
> if ((resp->http_version = strdup(desc->http_version)) == NULL)
> goto fail;
> +
> +#ifdef DEBUG
> + DPRINTF("%s: http_path: %s, http_path_alias: %s\n", __func__,
> desc->http_path, desc->http_path_alias);
No need to put DPRINTF in DEBUG - it is already only compiled with DEBUG.
> +#endif
>
> /* Now search for the location */
> srv_conf = server_getlocation(clt, desc->http_path);
>
> - if (srv_conf->flags & SRVFLAG_BLOCK) {
> +
> + if ((srv_conf->flags & SRVFLAG_BLOCK) && srv_conf->return_code == 200) {
> +#ifdef DEBUG
> + DPRINTF("%s: Rewrite from: %s to %s\n", __func__,
> desc->http_path, srv_conf->return_uri);
Same here.
Hint: when sending patches, developers appreciate it when they already
follow style(9). This makes it easier for us to read. So wrap the
lines at 80 chars, don't use '//' comments as below, ...
> +#endif
> + if (desc->http_path != NULL) {
> + free(desc->http_path);
> + desc->http_path = NULL;
Oh, this will break some of the macros in server_expand_http()!
> + }
> + if (server_expand_http(clt, srv_conf->return_uri, buf,
> sizeof(buf)) == NULL) {
> + server_abort_http(clt, 500, strerror(errno));
> + return (-1);
> + }
> + desc->http_path = strdup(buf);
> + if (desc->http_path == NULL) {
> + server_abort_http(clt, 500, strerror(errno));
> + }
> + desc->http_query = strchr(desc->http_path, '?');
> + if (desc->http_query != NULL) {
> + *desc->http_query++ = '\0';
> + desc->http_query = strdup(desc->http_query);
> + if (desc->http_query == NULL) {
> + server_abort_http(clt, 500, strerror(errno));
> + }
> + }
> + srv_conf = server_getlocation(clt, desc->http_path); // again
> + }
> +
> +
> + if ((srv_conf->flags & SRVFLAG_BLOCK) && srv_conf->return_code != 200) {
This logic is not so nice.
But what about
if (srv_conf->flags & SRVFLAG_BLOCK) {
/* block with optional url */
...
} else if (srv_conf->return_uri) {
/* pass with uri: rewrite */
...
}
if (srv_conf->flags & SRVFLAG_AUTH &&
...
> server_abort_http(clt, srv_conf->return_code,
> srv_conf->return_uri);
> return (-1);
This patch needs more work, but it also shows that your concept, if
correct, is relatively simple to implement. Thanks.
Reyk
