On Wed, 28 Nov 2007 13:19:34 +0200 (SAST)
"Graham Leggett" <[EMAIL PROTECTED]> wrote:
> On Wed, November 28, 2007 1:06 pm, Nick Kew wrote:
>
> > Agreed. As it happens, I started on exactly that this morning,
> > before having to turn my attention away from hacking. With a bit
> > of luck I'll find the time to come up with something working
> > this evening or tomorrow. I expect this to answer my own point
> > in http://marc.info/?l=apache-httpd-dev&m=119439091917387&w=2
> > and generalise to enable us to introduce new -Options.
>
> I sat for ages trying to decide on a suitable syntax for this. One
> option was to use the function(parameter) syntax used by programming
> languages, or SQL, but that meant possible major surgery to a piece
> of code that has been quiet and stable for years.
>
> Eventually I followed the thinking that the conditional parse mostly
> follows the conditional handling of /bin/sh, and using the -x syntax
> is an extension of /bin/sh, or test. The choice of -A was based on it
> not clashing with any other tests in /bin/sh, which has no concept of
> access or URLs (it has a concept of "read" bit set, but I figured
> that a future mod_include might want to care if the "read" bit was
> set on a file (you never know), so I kept it safe).
OK, fairy nuff I understand the shell args derivation, and that's
fine by me. I'd prefer "-A /foo/bar.txt < 400", but I'm happy
to accept your version.
What I don't like about the patch is the evaluation inline within
the parser. I'm updating my working version to pass in an evaluation
function for shell-stuff, but there's no way that's going to make 2.2.7.
So in the meantime, I attach a patch that puts the evaluation and
subrequest stuff into a separate function, in preparation for taking
the parser from mod_include to core at a future date. Patch attached.
If you're happy with this minor reorganisation of your proposal,
then you have my +1 on it.
--
Nick Kew
Application Development with Apache - the Apache Modules Book
http://www.apachetutor.org/
Index: mod_include.c
===================================================================
--- mod_include.c (revision 598296)
+++ mod_include.c (working copy)
@@ -1034,6 +1034,7 @@
case '-':
if (**parse == 'A' && (ctx->intern->accessenable)) {
TYPE_TOKEN(token, TOKEN_ACCESS);
+ token->value = *parse;
++*parse;
return 0;
}
@@ -1121,12 +1122,40 @@
return unmatched;
}
-
+static int process_flag(include_ctx_t *ctx, parse_node_t *current)
+{
+ request_rec *r = ctx->intern->r;
+ request_rec *rr;
+ if (current->token.value[0] != 'A') {
+ return 1; /* no such flag */
+ }
+ if (current->left || !current->right ||
+ (current->right->token.type != TOKEN_STRING &&
+ current->right->token.type != TOKEN_RE)) {
+ return 1;
+ }
+ current->right->token.value =
+ ap_ssi_parse_string(ctx, current->right->token.value, NULL, 0,
+ SSI_EXPAND_DROP_NAME);
+ rr = ap_sub_req_lookup_uri(current->right->token.value, r, NULL);
+ /* 400 and higher are considered access denied */
+ if (rr->status < HTTP_BAD_REQUEST) {
+ current->value = 1;
+ }
+ else {
+ current->value = 0;
+ ap_log_rerror(APLOG_MARK, APLOG_DEBUG, rr->status, r,
+ "mod_include: The tested "
+ "subrequest -A \"%s\" returned an error code.",
+ current->right->token.value);
+ }
+ ap_destroy_sub_req(rr);
+ return 0;
+}
static int parse_expr(include_ctx_t *ctx, const char *expr, int *was_error)
{
parse_node_t *new, *root = NULL, *current = NULL;
request_rec *r = ctx->intern->r;
- request_rec *rr = NULL;
const char *error = "Invalid expression \"%s\" in file %s";
const char *parse = expr;
int was_unmatched = 0;
@@ -1482,31 +1511,13 @@
break;
case TOKEN_ACCESS:
- if (current->left || !current->right ||
- (current->right->token.type != TOKEN_STRING &&
- current->right->token.type != TOKEN_RE)) {
+ *was_error = process_flag(ctx, current);
+ if (*was_error) {
ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
- "Invalid expression \"%s\" in file %s: Token '-A' must be followed by a URI string.",
- expr, r->filename);
- *was_error = 1;
+ "Invalid expression \"%s\" in file %s: Token '-A' must be followed by a URI string.",
+ expr, r->filename);
return 0;
}
- current->right->token.value =
- ap_ssi_parse_string(ctx, current->right->token.value, NULL, 0,
- SSI_EXPAND_DROP_NAME);
- rr = ap_sub_req_lookup_uri(current->right->token.value, r, NULL);
- /* 400 and higher are considered access denied */
- if (rr->status < HTTP_BAD_REQUEST) {
- current->value = 1;
- }
- else {
- current->value = 0;
- ap_log_rerror(APLOG_MARK, APLOG_DEBUG, rr->status, r,
- "mod_include: The tested "
- "subrequest -A \"%s\" returned an error code.",
- current->right->token.value);
- }
- ap_destroy_sub_req(rr);
break;
case TOKEN_RE:
