Some more fixes for style and parser syntax.
Best,
S.
On Sun, Nov 15, 2015 at 07:46:35PM +0100, Reyk Floeter wrote:
> 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
? httpd
? httpd_url_rewrite.patch
? httpd_url_rewrite_v2.patch
? httpd_url_rewrite_v3.patch
? parse.c
? y.output
? y.tab.h
Index: config.c
===================================================================
RCS file: /cvs/src/usr.sbin/httpd/config.c,v
retrieving revision 1.42
diff -u -p -r1.42 config.c
--- config.c 19 Jul 2015 05:17:27 -0000 1.42
+++ config.c 16 Nov 2015 12:57:24 -0000
@@ -435,6 +435,12 @@ config_getserver_config(struct httpd *en
strdup(parent->return_uri)) == NULL)
goto fail;
}
+
+ /* Don't inherit rewrite settings */
+ /* f = SRVFLAG_REWRITE;
+ srv_conf->flags |= (parent->flags & f);
+ strlcpy(srv_conf->rewrite_uri, parent->rewrite_uri,
+ sizeof(srv_conf->rewrite_uri)); */
f = SRVFLAG_DEFAULT_TYPE;
if ((srv_conf->flags & f) == 0) {
Index: httpd.h
===================================================================
RCS file: /cvs/src/usr.sbin/httpd/httpd.h,v
retrieving revision 1.96
diff -u -p -r1.96 httpd.h
--- httpd.h 3 Aug 2015 11:45:17 -0000 1.96
+++ httpd.h 16 Nov 2015 12:57:24 -0000
@@ -70,6 +70,7 @@
#define SERVER_MAX_PREFETCH 256
#define SERVER_MIN_PREFETCHED 32
#define SERVER_HSTS_DEFAULT_AGE 31536000
+#define SERVER_MAX_RECUR_REWR 0
#define MEDIATYPE_NAMEMAX 128 /* file name extension */
#define MEDIATYPE_TYPEMAX 64 /* length of type/subtype */
@@ -354,13 +355,14 @@ SPLAY_HEAD(client_tree, client);
#define SRVFLAG_SERVER_MATCH 0x00200000
#define SRVFLAG_SERVER_HSTS 0x00400000
#define SRVFLAG_DEFAULT_TYPE 0x00800000
+#define SRVFLAG_REWRITE 0x01000000
#define SRVFLAG_BITS \
"\10\01INDEX\02NO_INDEX\03AUTO_INDEX\04NO_AUTO_INDEX" \
"\05ROOT\06LOCATION\07FCGI\10NO_FCGI\11LOG\12NO_LOG\13SOCKET" \
"\14SYSLOG\15NO_SYSLOG\16TLS\17ACCESS_LOG\20ERROR_LOG" \
"\21AUTH\22NO_AUTH\23BLOCK\24NO_BLOCK\25LOCATION_MATCH" \
- "\26SERVER_MATCH\27SERVER_HSTS\30DEFAULT_TYPE"
+ "\26SERVER_MATCH\27SERVER_HSTS\30DEFAULT_TYPE\31REWRITE"
#define TCPFLAG_NODELAY 0x01
#define TCPFLAG_NNODELAY 0x02
@@ -459,6 +461,7 @@ struct server_config {
int return_code;
char *return_uri;
off_t return_uri_len;
+ char rewrite_uri[PATH_MAX];
int hsts_max_age;
u_int8_t hsts_flags;
Index: parse.y
===================================================================
RCS file: /cvs/src/usr.sbin/httpd/parse.y,v
retrieving revision 1.73
diff -u -p -r1.73 parse.y
--- parse.y 19 Jul 2015 05:17:27 -0000 1.73
+++ parse.y 16 Nov 2015 12:57:25 -0000
@@ -134,7 +134,7 @@ typedef struct {
%token LOCATION LOG LOGDIR MATCH MAXIMUM NO NODELAY ON PORT PREFORK PROTOCOLS
%token REQUEST REQUESTS ROOT SACK SERVER SOCKET STRIP STYLE SYSLOG TCP TIMEOUT
%token TLS TYPE TYPES HSTS MAXAGE SUBDOMAINS DEFAULT PRELOAD
-%token ERROR INCLUDE AUTHENTICATE WITH BLOCK DROP RETURN PASS
+%token ERROR INCLUDE AUTHENTICATE WITH BLOCK DROP RETURN PASS REWRITE
%token <v.string> STRING
%token <v.number> NUMBER
%type <v.port> port
@@ -925,6 +925,17 @@ filter : block RETURN NUMBER optstring
/* Forbidden */
srv_conf->return_code = 403;
}
+ | PASS REWRITE STRING {
+ srv_conf->flags |= SRVFLAG_REWRITE;
+ if (strlcpy(srv_conf->rewrite_uri, $3,
+ sizeof(srv_conf->rewrite_uri)) >=
+ sizeof(srv_conf->rewrite_uri)) {
+ yyerror("url rewrite string too long");
+ free($3);
+ YYERROR;
+ }
+ free($3);
+ }
| PASS {
srv_conf->flags &= ~SRVFLAG_BLOCK;
srv_conf->flags |= SRVFLAG_NO_BLOCK;
@@ -1184,6 +1195,7 @@ lookup(char *s)
{ "request", REQUEST },
{ "requests", REQUESTS },
{ "return", RETURN },
+ { "rewrite", REWRITE },
{ "root", ROOT },
{ "sack", SACK },
{ "server", SERVER },
Index: server_http.c
===================================================================
RCS file: /cvs/src/usr.sbin/httpd/server_http.c,v
retrieving revision 1.96
diff -u -p -r1.96 server_http.c
--- server_http.c 31 Jul 2015 00:10:51 -0000 1.96
+++ server_http.c 16 Nov 2015 12:57:25 -0000
@@ -1054,6 +1054,8 @@ server_response(struct httpd *httpd, str
int portval = -1, ret;
char *hostval;
const char *errstr = NULL;
+ char buf[IBUF_READ_SIZE];
+ int n;
/* Canonicalize the request path */
if (desc->http_path == NULL ||
@@ -1158,6 +1160,44 @@ server_response(struct httpd *httpd, str
/* Now search for the location */
srv_conf = server_getlocation(clt, desc->http_path);
+
+ /* URL Rewriting logic */
+ DPRINTF("%s: URL Rewriting logic, flags: %08X, rewrite_uri: %s, "
+ "http_path: %s\n", __func__, srv_conf->flags,
+ srv_conf->rewrite_uri, desc->http_path);
+ for (n = 0; srv_conf->flags & SRVFLAG_REWRITE; n++) {
+ if (n > SERVER_MAX_RECUR_REWR) {
+ server_abort_http(clt, 500, "recursive "
+ "rewrite limit exceeded");
+ return (-1);
+ }
+ DPRINTF("%s: Rewrite from: %s to %s, n: %d\n", __func__,
+ desc->http_path, srv_conf->rewrite_uri, n);
+ if (server_expand_http(clt, srv_conf->rewrite_uri, buf,
+ sizeof(buf)) == NULL) {
+ server_abort_http(clt, 500, strerror(errno));
+ return (-1);
+ }
+ if (desc->http_path != NULL) {
+ free(desc->http_path);
+ desc->http_path = NULL;
+ }
+ desc->http_path = strdup(buf);
+ if (desc->http_path == NULL) {
+ server_abort_http(clt, 500, strerror(errno));
+ return (-1);
+ }
+ 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);
+ }
+
if (srv_conf->flags & SRVFLAG_BLOCK) {
server_abort_http(clt, srv_conf->return_code,
srv_conf->return_uri);
@@ -1200,8 +1240,10 @@ server_getlocation(struct client *clt, c
TAILQ_FOREACH(location, &srv->srv_hosts, entry) {
#ifdef DEBUG
if (location->flags & SRVFLAG_LOCATION) {
- DPRINTF("%s: location \"%s\" path \"%s\"",
- __func__, location->location, path);
+ DPRINTF("%s: location \"%s\" path \"%s\" flags: %08X "
+ "rewrite_uri: %s",
+ __func__, location->location, path, location->flags,
+ location->rewrite_uri);
}
#endif
if ((location->flags & SRVFLAG_LOCATION) &&
