You are viewing a plain text version of this content. The canonical link for it is here.
Posted to apreq-dev@httpd.apache.org by Jody Biggs <jo...@codegrok.com> on 2001/03/06 19:56:12 UTC
Apache::Request patch request (fwd)
I was having trouble setting a param to hold multiple values, and then
retrieving them using Apache Request, as
such:
sub handler
{
my $r = Apache::Request->new (shift);
$r->param ( 'foo' => [qw(a few element long list)] );
my @foolist = $r->param ( 'foo' );
# would get scalar (@foolist) == 1
# with that one element being an array ref
#
# actually, i'm not sure if it was an actual array ref,
# or just a string containing "ARRAY(0x.......)", but whatever
#...
}
anyhow, I think the problem was just that Request.pm holds:
sub param {
#...
if (defined $value) {
$tab->set($name, $value);
}
#...
}
however, $tab, which if I'm correct in assuming is an Apache::Table
object, only accepts string values to set, which means you get your array
ref converted to a string when it gets set.
I changed the set to add, which will accept strings or arrayrefs for the
value, and everything worked as I expected. However, there may be an
issue if the param was already set, and you wished to overwrite the
value(s) associated with it. Hence, it would make sense to call unset
first.
so:
sub param {
#...
if (defined $value) {
$tab->unset($name);
$tab->add($name, $value);
}
#...
}
anyhow, I hope that all made sense, and that I'm not missing some obvious
point in all of this. If it does make sense, I'd certainly like to see
the fix incorporated in the next version of libapreq (or however it
makes sense to release it).
thanks -
Jody Biggs
VP Product Engineering
CodeGrok Inc.
Re: Apache::Request patch request (fwd)
Posted by Joe Schaefer <jo...@sunstarsys.com>.
Jody Biggs <jo...@codegrok.com> writes:
> anyhow, I think the problem was just that Request.pm holds:
>
> sub param {
> #...
> if (defined $value) {
> $tab->set($name, $value);
> }
> #...
> }
>
> however, $tab, which if I'm correct in assuming is an Apache::Table
> object, only accepts string values to set, which means you get your array
> ref converted to a string when it gets set.
Yes- which is why I feel this is more of an Apache::Table issue than
an Apache::Request one. You are correct that libapreq holds the
parameter data in an apache table, which can be multi-valued. Hence
$table->add($name, [qw(a b c d)]);
performs some behind-the-scenes DWIMery (it creates multiple table
entries, one for each value in the anonymous array, rather than a
single key with a perl arrayref) that neither
$table->set($name, [qw(a b c d)]);
nor
$$table{$name} = [qw(a b c d)];
possess. IMHO they should be fixed, but that's best left to the
mod_perl developers. Perhaps you should post to the modperl
users (or developer) list instead?
> sub param {
> #...
> if (defined $value) {
> $tab->unset($name);
> $tab->add($name, $value);
> }
> #...
> }
The fix you suggest seems OK, in case the mod_perl folks can't/won't
change Apache::Table. Maybe we should include a similar work-around in
libapreq-0.32?
> anyhow, I hope that all made sense, and that I'm not missing some obvious
> point in all of this. If it does make sense, I'd certainly like to see
> the fix incorporated in the next version of libapreq (or however it
> makes sense to release it).
It did, and thanks for posting it.
--
Joe Schaefer
[RFC] Patch (Was: Apache::Request patch request (fwd))
Posted by Joe Schaefer <jo...@sunstarsys.com>.
Joe Schaefer <jo...@sunstarsys.com> writes:
> Jody Biggs <jo...@codegrok.com> writes:
> > sub param {
> > #...
> > if (defined $value) {
> > $tab->unset($name);
> > $tab->add($name, $value);
> > }
> > #...
> > }
>
> The fix you suggest seems OK, in case the mod_perl folks can't/won't
> change Apache::Table. Maybe we should include a similar work-around in
> libapreq-0.32?
Here's a patch that does the following:
1) rewrites Apache::Request::param as an xsub, incorporating Jody's
above fix. I'm not sure I did it right, since I'm still not
comfortable with my understanding of XS. It seems to work under
light testing though. My reason for XS'ing param() was mainly to
eliminate any need for $r->parms.
2) reworks ApacheRequest_tmpfile to provide access to the temp
file's name:
(a) adds (char *) upload->tmpnam to the ApacheUpload struct
(b) provides perl access to it via Apache::Upload::tmpnam
(c) adds new perl method Apache::Upload::link, used like this
$r->upload->link($linkname) or
die sprintf "link(%s, %s) failed",
$r->upload->tmpnam,
$linkname;
This makes a hard link to the temp file via link(2);
unfortunately I don't think it is portable to non-*nix
platforms. The implementation is in the patch to
Request.xs. Suggestions on a more portable method would
be most appreciated.
This patch doesn't include David's recent patch enabling upload
hooks yet (btw: David- how do you install per-upload object hooks
with your patch?)
3) includes the apache_cookie.c patch that I should have committed
already :(
Threat: I'll commit these in a few days unless someone objects.
I mean it this time :)
Thanks again.
--
Joe Schaefer
===================================================================
<joe httpd-apreq> % cvs diff -R .
cvs server: Diffing .
cvs server: Diffing Cookie
cvs server: Diffing Request
Index: Request/Request.pm
===================================================================
RCS file: /home/cvs/httpd-apreq/Request/Request.pm,v
retrieving revision 1.12
diff -u -r1.12 Request.pm
--- Request/Request.pm 2000/12/23 03:19:14 1.12
+++ Request/Request.pm 2001/03/11 01:46:05
@@ -12,19 +12,19 @@
}
#just prototype methods here, moving to xs later
-sub param {
- my $self = shift;
- my($name, $value) = @_;
- my $tab = $self->parms;
- unless ($name) {
- my %seen;
- return wantarray ? grep { !$seen{$_}++ } keys %$tab : $tab;
- }
- if (defined $value) {
- $tab->set($name, $value);
- }
- return wantarray ? ($tab->get($name)) : scalar $tab->get($name);
-}
+#sub param {
+# my $self = shift;
+# my($name, $value) = @_;
+# my $tab = $self->parms;
+# unless ($name) {
+# my %seen;
+# return wantarray ? grep { !$seen{$_}++ } keys %$tab : $tab;
+# }
+# if (defined $value) {
+# $tab->set($name, $value);
+# }
+# return wantarray ? ($tab->get($name)) : scalar $tab->get($name);
+#}
sub instance {
my $class = shift;
Index: Request/Request.xs
===================================================================
RCS file: /home/cvs/httpd-apreq/Request/Request.xs,v
retrieving revision 1.11
diff -u -r1.11 Request.xs
--- Request/Request.xs 2001/02/11 23:37:46 1.11
+++ Request/Request.xs 2001/03/11 01:46:06
@@ -76,6 +76,7 @@
#define ApacheUpload_name(upload) upload->name
#define ApacheUpload_filename(upload) upload->filename
#define ApacheUpload_next(upload) upload->next
+#define ApacheUpload_tmpnam(upload) upload->tmpnam
#ifndef PerlLIO_dup
#define PerlLIO_dup(fd) dup((fd))
@@ -246,6 +247,65 @@
ST(0) = mod_perl_tie_table(req->parms);
void
+ApacheRequest_param(req, key=NULL, sv=Nullsv)
+ Apache::Request req
+ const char *key
+ SV *sv
+
+ PPCODE:
+ ApacheRequest_parse(req);
+ if (key) {
+ if (SvROK(sv) && SvTYPE(SvRV(sv)) == SVt_PVAV) {
+ I32 i;
+ AV *av = (AV*)SvRV(sv);
+ const char *val;
+ ap_table_unset(req->parms, key);
+ for (i=0; i<=AvFILL(av); i++) {
+ val = (const char *)SvPV(*av_fetch(av, i, FALSE),na);
+ ap_table_add(req->parms, key, val);
+ }
+ }
+ else if (SvPOK(sv)) {
+ ap_table_set(req->parms, key, (const char *)SvPV(sv, na));
+ }
+ if(GIMME == G_SCALAR) {
+ const char *val = ap_table_get(req->parms, key);
+ if (val) XPUSHs(sv_2mortal(newSVpv((char*)val,0)));
+ else XSRETURN_UNDEF;
+ }
+ else {
+ I32 i;
+ array_header *arr = ap_table_elts(req->parms);
+ table_entry *elts = (table_entry *)arr->elts;
+ for (i = 0; i < arr->nelts; ++i) {
+ if (!elts[i].key || strcasecmp(elts[i].key, key)) continue;
+ XPUSHs(sv_2mortal(newSVpv(elts[i].val,0)));
+ }
+ }
+ }
+ else {
+ if(GIMME == G_SCALAR) {
+ XPUSHs(sv_2mortal(mod_perl_tie_table(req->parms)));
+ }
+ else {
+ I32 i;
+ array_header *arr = ap_table_elts(req->parms);
+ table_entry *elts = (table_entry *)arr->elts;
+ for (i = 0; i < arr->nelts; ++i) {
+ I32 j;
+ if (!elts[i].key) continue;
+ /* simple but inefficient uniqueness check */
+ for (j = 0; j < i; ++j) {
+ if (!strcasecmp(elts[i].key, elts[j].key))
+ break;
+ }
+ if ( i > j ) continue;
+ XPUSHs(sv_2mortal(newSVpv(elts[i].key,0)));
+ }
+ }
+ }
+
+void
ApacheRequest_upload(req, name=NULL)
Apache::Request req
char *name
@@ -323,6 +383,10 @@
ApacheUpload_filename(upload)
Apache::Upload upload
+char *
+ApacheUpload_tmpnam(upload)
+ Apache::Upload upload
+
Apache::Upload
ApacheUpload_next(upload)
Apache::Upload upload
@@ -330,6 +394,17 @@
const char *
ApacheUpload_type(upload)
Apache::Upload upload
+
+char *
+ApacheUpload_link(upload, name)
+ Apache::Upload upload
+ char *name
+
+ CODE:
+ RETVAL = (link(upload->tmpnam, name)) ? NULL : name;
+
+ OUTPUT:
+ RETVAL
void
ApacheUpload_info(upload, key=NULL)
cvs server: Diffing c
Index: c/apache_cookie.c
===================================================================
RCS file: /home/cvs/httpd-apreq/c/apache_cookie.c,v
retrieving revision 1.8
diff -u -r1.8 apache_cookie.c
--- c/apache_cookie.c 2001/01/05 00:05:23 1.8
+++ c/apache_cookie.c 2001/03/11 01:46:06
@@ -192,8 +192,40 @@
cookie_push_arr(arr, ap_pstrcat(p, name, "=", val, NULL)); \
}
-#define escape_url(val) \
-ap_os_escape_path(p, val?val:"", 1)
+static char * escape_url(pool *p, char *val)
+{
+ char *result = ap_os_escape_path(p, val?val:"", 1);
+ char *end = result + strlen(result);
+ char *seek;
+
+ for ( seek = end-1; seek >= result; --seek) {
+ char *ptr, *replacement;
+
+ switch (*seek) {
+
+ case '&':
+ replacement = "%26";
+ break;
+ case '=':
+ replacement = "%3d";
+ break;
+ /* additional cases here */
+
+ default:
+ continue; /* next for() */
+ }
+
+
+ for (ptr = end; ptr > seek; --ptr) {
+ ptr[2] = ptr[0];
+ }
+
+ strncpy(seek, replacement, 3);
+ end += 2;
+ }
+
+ return(result);
+}
char *ApacheCookie_as_string(ApacheCookie *c)
{
@@ -214,10 +246,10 @@
cookie_push_arr(values, "secure");
}
- cookie = ap_pstrcat(p, escape_url(c->name), "=", NULL);
+ cookie = ap_pstrcat(p, escape_url(p, c->name), "=", NULL);
for (i=0; i<c->values->nelts; i++) {
cookie = ap_pstrcat(p, cookie,
- escape_url(((char**)c->values->elts)[i]),
+ escape_url(p, ((char**)c->values->elts)[i]),
(i < (c->values->nelts-1) ? "&" : NULL),
NULL);
}
Index: c/apache_request.c
===================================================================
RCS file: /home/cvs/httpd-apreq/c/apache_request.c,v
retrieving revision 1.8
diff -u -r1.8 apache_request.c
--- c/apache_request.c 2000/12/17 00:36:38 1.8
+++ c/apache_request.c 2001/03/11 01:46:07
@@ -315,20 +315,37 @@
return OK;
}
-FILE *ApacheRequest_tmpfile(ApacheRequest *req)
+static void remove_tmpfile (void *data) {
+ char *name = (char *)data;
+ remove(data);
+}
+
+FILE *ApacheRequest_tmpfile(ApacheRequest *req, ApacheUpload *upload)
{
request_rec *r = req->r;
FILE *fp;
-
- if (!(fp = tmpfile())) {
- ap_log_rerror(REQ_ERROR,
- "[libapreq] could not create tmpfile()");
+ char name[L_tmpnam];
+ int fd, tries = 100;
+
+ while (--tries > 0) {
+ if (tmpnam(name) == NULL) continue;
+ fd = ap_popenf(r->pool, name, O_CREAT|O_EXCL|O_RDWR, 0600);
+ if ( fd >= 0 )
+ break; /* success */
}
- else {
- ap_note_cleanups_for_file(r->pool, fp);
+
+ if (tries == 0 || (fp = ap_pfdopen(r->pool, fd, "r+")) == NULL ) {
+ ap_log_rerror(REQ_ERROR,
+ "[libapreq] could not open temp file '%s'", name);
+ return NULL;
}
+ upload->fp = fp;
+ upload->tmpnam = ap_pstrdup(r->pool, name);
+ ap_register_cleanup(r->pool, (void *)upload->tmpnam,
+ remove_tmpfile, ap_null_cleanup);
return fp;
+
}
int ApacheRequest_parse_multipart(ApacheRequest *req)
@@ -418,7 +435,7 @@
req->upload = upload;
}
- if (!(upload->fp = ApacheRequest_tmpfile(req))) {
+ if (!(upload->fp = ApacheRequest_tmpfile(req, upload))) {
return HTTP_INTERNAL_SERVER_ERROR;
}
Index: c/apache_request.h
===================================================================
RCS file: /home/cvs/httpd-apreq/c/apache_request.h,v
retrieving revision 1.4
diff -u -r1.4 apache_request.h
--- c/apache_request.h 2001/01/03 03:58:56 1.4
+++ c/apache_request.h 2001/03/11 01:46:07
@@ -26,6 +26,7 @@
ApacheUpload *next;
char *filename;
char *name;
+ char *tmpnam;
table *info;
FILE *fp;
long size;
@@ -72,7 +73,7 @@
#define ApacheRequest_parse(req) \
(req->status = req->parsed ? req->status : ApacheRequest___parse(req))
-FILE *ApacheRequest_tmpfile(ApacheRequest *req);
+FILE *ApacheRequest_tmpfile(ApacheRequest *req, ApacheUpload *upload);
ApacheUpload *ApacheUpload_new(ApacheRequest *req);
ApacheUpload *ApacheUpload_find(ApacheUpload *upload, char *name);
cvs server: Diffing eg
cvs server: Diffing eg/c
cvs server: Diffing eg/c/testapreq
cvs server: Diffing eg/perl
cvs server: Diffing lib
cvs server: Diffing lib/Apache
<joe httpd-apreq> %
Re: Apache::Request patch request (fwd)
Posted by Joe Schaefer <jo...@sunstarsys.com>.
Jody Biggs <jo...@codegrok.com> writes:
> anyhow, I think the problem was just that Request.pm holds:
>
> sub param {
> #...
> if (defined $value) {
> $tab->set($name, $value);
> }
> #...
> }
>
> however, $tab, which if I'm correct in assuming is an Apache::Table
> object, only accepts string values to set, which means you get your array
> ref converted to a string when it gets set.
Yes- which is why I feel this is more of an Apache::Table issue than
an Apache::Request one. You are correct that libapreq holds the
parameter data in an apache table, which can be multi-valued. Hence
$table->add($name, [qw(a b c d)]);
performs some behind-the-scenes DWIMery (it creates multiple table
entries, one for each value in the anonymous array, rather than a
single key with a perl arrayref) that neither
$table->set($name, [qw(a b c d)]);
nor
$$table{$name} = [qw(a b c d)];
possess. IMHO they should be fixed, but that's best left to the
mod_perl developers. Perhaps you should post to the modperl
users (or developer) list instead?
> sub param {
> #...
> if (defined $value) {
> $tab->unset($name);
> $tab->add($name, $value);
> }
> #...
> }
The fix you suggest seems OK, in case the mod_perl folks can't/won't
change Apache::Table. Maybe we should include a similar work-around in
libapreq-0.32?
> anyhow, I hope that all made sense, and that I'm not missing some obvious
> point in all of this. If it does make sense, I'd certainly like to see
> the fix incorporated in the next version of libapreq (or however it
> makes sense to release it).
It did, and thanks for posting it.
--
Joe Schaefer