Re: httpd URL rewrite support patch
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 - 1.73
> > +++ parse.y 13 Nov 2015 08:29:10 -
> > @@ -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 - 1.96
> > +++ server_http.c 13 Nov 2015 08:29:10 -
> > @@ -1054,6 +1054,7 @@ server_response(struct httpd *httpd, str
> > int portval = -1, ret;
> > char*hostval;
> > const char *errstr = NULL;
> > + charbuf[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_BLO
Re: httpd URL rewrite support patch
Please take a look at the modified patch. I've introduced the
"pass rewrite" syntax you proposed, added support for recursive
rewriting (disabled by default), fixed freeing of http_path and
created separate rewrite_uri field not to interfere with the
return_uri logic.
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 - 1.73
> > +++ parse.y 13 Nov 2015 08:29:10 -
> > @@ -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 - 1.96
> > +++ server_http.c 13 Nov 2015 08:29:10 -
> > @@ -1054,6 +1054,7 @@ server_response(struct httpd *httpd, str
> > int portval = -1, ret;
> > char*hostval;
> > const char *errstr = NULL;
> > + charbuf[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, de
Re: httpd URL rewrite support patch
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 - 1.73
> +++ parse.y 13 Nov 2015 08:29:10 -
> @@ -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 - 1.96
> +++ server_http.c 13 Nov 2015 08:29:10 -
> @@ -1054,6 +1054,7 @@ server_response(struct httpd *httpd, str
> int portval = -1, ret;
> char*hostval;
> const char *errstr = NULL;
> + charbuf[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 &&
...
>
Re: httpd URL rewrite support patch
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
> >
? httpd
? httpd_url_rewrite.patch
? parse.c
? y.tab.h
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 - 1.73
+++ parse.y 13 Nov 2015 08:29:10 -
@@ -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)) {
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 - 1.96
+++ server_http.c 13 Nov 2015 08:29:10 -
@@ -1054,6 +1054,7 @@ server_response(struct httpd *httpd, str
int portval = -1, ret;
char*hostval;
const char *errstr = NULL;
+ charbuf[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);
+#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);
+#endif
+ if (desc->http_path != NULL) {
+ free(desc->http_path);
+ desc->http_path = NULL;
+ }
+ 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) {
server_abort_http(clt, srv_conf->return_code,
srv_conf->return_uri);
return (-1);
Re: httpd URL rewrite support patch
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
httpd URL rewrite support patch
Hello everyone,
I'm new to OpenBSD and the tech@ list.
I need to run a WordPress-like URL rewriting scheme for my blog. Basically
URLs of the form /long-fancy-blog-post-title/ to be translated to
/index.php?q=long-fancy-blog-post-title.
I couldn't find this function in OpenBSD's new httpd which otherwise seems
like a great piece of s/w.
I spotted however the "block return code return_uri" directive which
corresponds almost perfectly to my needs. I concluded that extending it to
support code 200 which would signify an internal HTTP redirection is
logical, as the original request is still _blocked_.
The patch attached contains the necessary changes, whereas the httpd.conf
and rewr.php illustrate how the functionality is used.
Could I please ask the person responsible for httpd to integrate my patch
or something with equivalent functionality?
Thank you for your time.
Best regards,
Stanislaw Adaszewski
? httpd
? httpd_url_rewrite.patch
? parse.c
? y.tab.h
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 - 1.73
+++ parse.y 13 Nov 2015 08:29:10 -
@@ -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)) {
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 - 1.96
+++ server_http.c 13 Nov 2015 08:29:10 -
@@ -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);
+#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);
+#endif
+ if (desc->http_path != NULL) {
+ free(desc->http_path);
+ desc->http_path = NULL;
+ }
+ 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) {
server_abort_http(clt, srv_conf->return_code,
srv_conf->return_uri);
return (-1);
server "default" {
listen on * port 80
root "/htdocs/sadaszew/algoholic"
location "*.php" {
fastcgi socket "/run/php-fpm.sock"
}
location match "^/([%l%u%d%-]+)/$" {
block return 200 "/rewr.php?q=%1"
}
}
