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 Joe Schaefer <jo...@sunstarsys.com> on 2003/10/24 06:38:27 UTC
Re: cvs commit: httpd-apreq-2/env/t/cgi-bin .cvsignore
randyk@apache.org writes:
> randyk 2003/10/23 16:58:15
>
> Modified: env Makefile.am .cvsignore
> Added: env test_cgi.c
> env/t cgi.t
> env/t/conf extra.conf.in
> env/t/cgi-bin .cvsignore
> Log:
> add libapreq_cgi tests.
After the Marlins big win tonight, I had some problems running the
tests with apr's current cvs- apr_env_get's behavior may have changed
slightly, and I was getting segfaults on cgi tests 1-9. Initializing
value = NULL took care of those. Also test 22 uses GET_BODY with a
"Content" argument; I'm not sure if that's legit or not, so I changed
it to POST_BODY.
The last criticism is that we should eventually get rid of the
env_ctx struct in test_cgi.c, and use a static global variable in
libapreq_cgi instead. There's no natural candidate for "env" in a cgi
context, so IMO we should probably ignore all the "env" arguments and
let users put anything they like in that slot.
Anyways, for now here's the diff from env/ that fixes the segfaults.
Let me know if it works ok.
Index: libapreq_cgi.c
===================================================================
RCS file: /home/cvs/httpd-apreq-2/env/libapreq_cgi.c,v
retrieving revision 1.13
diff -u -r1.13 libapreq_cgi.c
--- libapreq_cgi.c 21 Oct 2003 19:31:47 -0000 1.13
+++ libapreq_cgi.c 24 Oct 2003 04:22:27 -0000
@@ -122,7 +122,7 @@
{
dCTX;
char *key = apr_pstrdup(ctx->pool, name);
- char *k, *value, *http_key, http[] = "HTTP_";
+ char *k, *value=NULL, *http_key, http[] = "HTTP_";
for (k = key; *k; ++k) {
if (*k == '-')
*k = '_';
Index: test_cgi.c
===================================================================
RCS file: /home/cvs/httpd-apreq-2/env/test_cgi.c,v
retrieving revision 1.3
diff -u -r1.3 test_cgi.c
--- test_cgi.c 23 Oct 2003 23:58:14 -0000 1.3
+++ test_cgi.c 24 Oct 2003 04:22:27 -0000
@@ -61,14 +61,14 @@
#include "apr_lib.h"
#include "apr_tables.h"
-typedef struct {
+struct env_ctx {
apr_pool_t *pool;
apreq_request_t *req;
apreq_jar_t *jar;
apr_bucket_brigade *bb;
int loglevel;
apr_status_t status;
-} env_ctx;
+};
static int dump_table(void *count, const char *key, const char *value)
{
@@ -80,7 +80,7 @@
int main(int argc, char const * const * argv)
{
- env_ctx *ctx;
+ struct env_ctx *ctx;
apr_pool_t *pool;
const apreq_param_t *foo, *bar, *test, *key;
apr_table_t *params;
@@ -103,7 +103,7 @@
exit(-1);
}
- ctx = (env_ctx *) apr_palloc(pool, sizeof(*ctx));
+ ctx = (struct env_ctx *) apr_pcalloc(pool, sizeof *ctx);
ctx->loglevel = 0;
ctx->pool = pool;
apreq_log(APREQ_DEBUG 0, ctx, "%s", "Creating apreq_request");
Index: t/cgi.t
===================================================================
RCS file: /home/cvs/httpd-apreq-2/env/t/cgi.t,v
retrieving revision 1.1
diff -u -r1.1 cgi.t
--- t/cgi.t 23 Oct 2003 23:58:15 -0000 1.1
+++ t/cgi.t 24 Oct 2003 04:22:28 -0000
@@ -68,7 +68,7 @@
}
ok t_cmp("\tfoo => 1$line_end",
- GET_BODY("$script?foo=1", Content => $filler));
+ POST_BODY("$script?foo=1", Content => $filler), "simple post");
ok t_cmp("\tfoo => ?$line_end\tbar => hello world$line_end",
GET_BODY("$script?foo=%3F&bar=hello+world"), "simple get");
--
Joe Schaefer
Re: cvs commit: httpd-apreq-2/env/t/cgi-bin .cvsignore
Posted by Randy Kobes <ra...@theoryx5.uwinnipeg.ca>.
On Fri, 24 Oct 2003, Joe Schaefer wrote:
> Randy Kobes <ra...@theoryx5.uwinnipeg.ca> writes:
>
> [...]
> > If I understand this correctly, HTTP_ is being prepended to
> > the "name" being passed in ...
>
> Right, and the resulting pointer (key) is bumped past the "HTTP_"
> part for "HTTP_CONTENT_TYPE" and "HTTP_CONTENT_LENGTH". The
> advantage being we only do one pool allocation this way (instead of
> two). It should be functionally equivalent to the original.
Ah, right - very nice ... Thanks!
--
best regards,
randy
Re: cvs commit: httpd-apreq-2/env/t/cgi-bin .cvsignore
Posted by Joe Schaefer <jo...@sunstarsys.com>.
Randy Kobes <ra...@theoryx5.uwinnipeg.ca> writes:
[...]
> > APREQ_DECLARE(const char *)apreq_env_header_in(void *env,
> > const char *name)
> > {
> > - dCTX;
> > - char *key = apr_pstrdup(ctx->pool, name);
> > - char *k, *value=NULL, *http_key, http[] = "HTTP_";
> > + dP;
> > + char *key = apr_pstrcat(p, "HTTP_", name, NULL);
> > + char *k, *value = NULL;
> > for (k = key; *k; ++k) {
> > if (*k == '-')
> > *k = '_';
> > @@ -130,67 +128,63 @@
> > *k = apr_toupper(*k);
> > }
> >
> > - if (!strcmp(key, "CONTENT_TYPE") || !strcmp(key, "CONTENT_LENGTH")) {
> > - APREQ_ENV_STATUS(apr_env_get(&value, key, ctx->pool), key);
> > - }
> > - else {
> > - http_key = (char *) apr_palloc(ctx->pool, sizeof(http) + strlen(key));
> > - http_key = strcat(strcpy(http_key, http), key);
> > - APREQ_ENV_STATUS(apr_env_get(&value, http_key, ctx->pool), http_key);
> > + if (!strcmp(key, "HTTP_CONTENT_TYPE")
> > + || !strcmp(key, "HTTP_CONTENT_LENGTH")) {
> > +
> > + key += 5; /* strlen("HTTP_") */
> > }
> >
> > + APREQ_ENV_STATUS(apr_env_get(&value, key, p), key);
> > +
> > return value;
> > }
>
> If I understand this correctly, HTTP_ is being prepended to
> the "name" being passed in ...
Right, and the resulting pointer (key) is bumped past the "HTTP_"
part for "HTTP_CONTENT_TYPE" and "HTTP_CONTENT_LENGTH". The
advantage being we only do one pool allocation this way (instead of
two). It should be functionally equivalent to the original.
--
Joe Schaefer
Re: cvs commit: httpd-apreq-2/env/t/cgi-bin .cvsignore
Posted by Randy Kobes <ra...@theoryx5.uwinnipeg.ca>.
On Fri, 24 Oct 2003, Joe Schaefer wrote:
> Joe Schaefer <jo...@sunstarsys.com> writes:
>
> [...]
>
> > The last criticism is that we should eventually get rid of the
> > env_ctx struct in test_cgi.c, and use a static global variable in
> > libapreq_cgi instead. There's no natural candidate for "env" in a cgi
> > context, so IMO we should probably ignore all the "env" arguments and
> > let users put anything they like in that slot.
>
> After looking some more at test_cgi.c, there *is* one natural
> candidate there for an "env"- the pool! Here's the (longish) patch
> to libapreq_cgi.c and test_cgi.c that implements the above- let me
> know if it's ok to commit (all cgi tests still pass).
I'll look at this over the weekend on my Win32 machine, but
it looks fine, so please, go ahead - using the pool as the
env is more natural in this context ... I have one question
below related to
> Index: env/libapreq_cgi.c
> ===================================================================
> RCS file: /home/cvs/httpd-apreq-2/env/libapreq_cgi.c,v
> retrieving revision 1.14
> diff -u -r1.14 libapreq_cgi.c
[ ... ]
> APREQ_DECLARE(const char *)apreq_env_header_in(void *env,
> const char *name)
> {
> - dCTX;
> - char *key = apr_pstrdup(ctx->pool, name);
> - char *k, *value=NULL, *http_key, http[] = "HTTP_";
> + dP;
> + char *key = apr_pstrcat(p, "HTTP_", name, NULL);
> + char *k, *value = NULL;
> for (k = key; *k; ++k) {
> if (*k == '-')
> *k = '_';
> @@ -130,67 +128,63 @@
> *k = apr_toupper(*k);
> }
>
> - if (!strcmp(key, "CONTENT_TYPE") || !strcmp(key, "CONTENT_LENGTH")) {
> - APREQ_ENV_STATUS(apr_env_get(&value, key, ctx->pool), key);
> - }
> - else {
> - http_key = (char *) apr_palloc(ctx->pool, sizeof(http) + strlen(key));
> - http_key = strcat(strcpy(http_key, http), key);
> - APREQ_ENV_STATUS(apr_env_get(&value, http_key, ctx->pool), http_key);
> + if (!strcmp(key, "HTTP_CONTENT_TYPE")
> + || !strcmp(key, "HTTP_CONTENT_LENGTH")) {
> +
> + key += 5; /* strlen("HTTP_") */
> }
>
> + APREQ_ENV_STATUS(apr_env_get(&value, key, p), key);
> +
> return value;
> }
If I understand this correctly, HTTP_ is being prepended to
the "name" being passed in ... What I had before (which may
well have been wrong) was HTTP_ being prepended except for
CONTENT_TYPE or CONTENT_LENGTH - this I inferred from the
util_script.c under httpd-2.0.47/server/, in the
ap_add_common_vars() sub, where http2env() is called (to
prepend HTTP_) except for Content-Type and Content-Length
(and a couple of others). But maybe I'm not interpreting
this right? Thanks.
--
best regards,
randy
Re: cvs commit: httpd-apreq-2/env/t/cgi-bin .cvsignore
Posted by Joe Schaefer <jo...@sunstarsys.com>.
Joe Schaefer <jo...@sunstarsys.com> writes:
[...]
> The last criticism is that we should eventually get rid of the
> env_ctx struct in test_cgi.c, and use a static global variable in
> libapreq_cgi instead. There's no natural candidate for "env" in a cgi
> context, so IMO we should probably ignore all the "env" arguments and
> let users put anything they like in that slot.
After looking some more at test_cgi.c, there *is* one natural
candidate there for an "env"- the pool! Here's the (longish) patch
to libapreq_cgi.c and test_cgi.c that implements the above- let me
know if it's ok to commit (all cgi tests still pass).
Index: env/libapreq_cgi.c
===================================================================
RCS file: /home/cvs/httpd-apreq-2/env/libapreq_cgi.c,v
retrieving revision 1.14
diff -u -r1.14 libapreq_cgi.c
--- env/libapreq_cgi.c 24 Oct 2003 05:25:11 -0000 1.14
+++ env/libapreq_cgi.c 24 Oct 2003 08:30:51 -0000
@@ -64,7 +64,7 @@
#include <stdlib.h>
#include <stdio.h>
-#define dCTX struct env_ctx *ctx = (struct env_ctx *)env
+#define dP apr_pool_t *p = (apr_pool_t *)env
/**
* @file libapreq_cgi.c
@@ -82,47 +82,45 @@
*/
struct env_ctx {
- apr_pool_t *pool;
apreq_request_t *req;
apreq_jar_t *jar;
- apr_bucket_brigade *bb;
- int loglevel;
apr_status_t status;
};
+static struct env_ctx ctx;
+
const char apreq_env_name[] = "CGI";
-const unsigned int apreq_env_magic_number = 20031014;
+const unsigned int apreq_env_magic_number = 20031024;
#define CRLF "\015\012"
-#define APREQ_ENV_STATUS(rc_run, k) do { \
- apr_status_t rc = rc_run; \
- if (rc != APR_SUCCESS) { \
- apreq_log(APREQ_DEBUG 0, ctx, \
- "Lookup of %s failed: status=%d", k, rc); \
- } \
+#define APREQ_ENV_STATUS(rc_run, k) do { \
+ apr_status_t rc = rc_run; \
+ if (rc != APR_SUCCESS) { \
+ apreq_log(APREQ_DEBUG 0, p, \
+ "Lookup of %s failed: status=%d", k, rc); \
+ } \
} while (0)
APREQ_DECLARE(apr_pool_t *)apreq_env_pool(void *env)
{
- dCTX;
- return ctx->pool;
+ return (apr_pool_t *)env;
}
APREQ_DECLARE(const char *)apreq_env_query_string(void *env)
{
- dCTX;
- char *value, qs[] = "QUERY_STRING";
- APREQ_ENV_STATUS(apr_env_get(&value, qs, ctx->pool), qs);
+ dP;
+ char *value = NULL, qs[] = "QUERY_STRING";
+ APREQ_ENV_STATUS(apr_env_get(&value, qs, p), qs);
return value;
}
APREQ_DECLARE(const char *)apreq_env_header_in(void *env,
const char *name)
{
- dCTX;
- char *key = apr_pstrdup(ctx->pool, name);
- char *k, *value=NULL, *http_key, http[] = "HTTP_";
+ dP;
+ char *key = apr_pstrcat(p, "HTTP_", name, NULL);
+ char *k, *value = NULL;
for (k = key; *k; ++k) {
if (*k == '-')
*k = '_';
@@ -130,67 +128,63 @@
*k = apr_toupper(*k);
}
- if (!strcmp(key, "CONTENT_TYPE") || !strcmp(key, "CONTENT_LENGTH")) {
- APREQ_ENV_STATUS(apr_env_get(&value, key, ctx->pool), key);
- }
- else {
- http_key = (char *) apr_palloc(ctx->pool, sizeof(http) + strlen(key));
- http_key = strcat(strcpy(http_key, http), key);
- APREQ_ENV_STATUS(apr_env_get(&value, http_key, ctx->pool), http_key);
+ if (!strcmp(key, "HTTP_CONTENT_TYPE")
+ || !strcmp(key, "HTTP_CONTENT_LENGTH")) {
+
+ key += 5; /* strlen("HTTP_") */
}
+ APREQ_ENV_STATUS(apr_env_get(&value, key, p), key);
+
return value;
}
APREQ_DECLARE(apr_status_t)apreq_env_header_out(void *env, const char *name,
char *value)
{
- dCTX;
+ dP;
apr_file_t *out;
int bytes;
- apr_file_open_stdout(&out, ctx->pool);
+ apr_file_open_stdout(&out, p);
bytes = apr_file_printf(out, "%s: %s" CRLF, name, value);
- apreq_log(APREQ_DEBUG 0, ctx, "Setting header: %s => %s", name, value);
+ apreq_log(APREQ_DEBUG 0, p, "Setting header: %s => %s", name, value);
return bytes > 0 ? APR_SUCCESS : APR_EGENERAL;
}
APREQ_DECLARE(apreq_jar_t *) apreq_env_jar(void *env, apreq_jar_t *jar)
{
- dCTX;
+
if (jar != NULL) {
- apreq_jar_t *old_jar = ctx->jar;
- ctx->jar = jar;
+ apreq_jar_t *old_jar = ctx.jar;
+ ctx.jar = jar;
return old_jar;
}
- return ctx->jar;
+ return ctx.jar;
}
APREQ_DECLARE(apreq_request_t *)apreq_env_request(void *env,
apreq_request_t *req)
{
- dCTX;
if (req != NULL) {
- apreq_request_t *old_req = ctx->req;
- ctx->req = req;
+ apreq_request_t *old_req = ctx.req;
+ ctx.req = req;
return old_req;
}
- return ctx->req;
+ return ctx.req;
}
APREQ_DECLARE_LOG(apreq_log)
{
- dCTX;
+ dP;
va_list vp;
- if (level < ctx->loglevel)
- return;
va_start(vp, fmt);
fprintf(stderr, "[%s(%d)] %s\n", file, line,
- apr_pvsprintf(ctx->pool,fmt,vp));
+ apr_pvsprintf(p,fmt,vp));
va_end(vp);
}
@@ -198,20 +192,21 @@
apr_read_type_e block,
apr_off_t bytes)
{
- dCTX;
- if (ctx->bb == NULL) {
- apr_bucket_alloc_t *alloc = apr_bucket_alloc_create(ctx->pool);
- apr_file_t *in;
- apr_bucket *stdin_pipe, *b;
+ dP;
+ apreq_request_t *req = apreq_request(env, NULL);
- ctx->bb = apr_brigade_create(ctx->pool, alloc);
- apr_file_open_stdin(&in, ctx->pool);
+ if (req->body == NULL) {
+ apr_bucket_alloc_t *alloc = apr_bucket_alloc_create(p);
+ apr_bucket_brigade *bb = apr_brigade_create(p, alloc);
+ apr_bucket *stdin_pipe, *eos = apr_bucket_eos_create(alloc);
+ apr_file_t *in;
+ apr_file_open_stdin(&in, p);
stdin_pipe = apr_bucket_pipe_create(in,alloc);
- APR_BRIGADE_INSERT_HEAD(ctx->bb, stdin_pipe);
- b = apr_bucket_eos_create(alloc);
- APR_BRIGADE_INSERT_TAIL(ctx->bb, b);
+ APR_BRIGADE_INSERT_HEAD(bb, stdin_pipe);
+ APR_BRIGADE_INSERT_TAIL(bb, eos);
+ ctx.status = apreq_parse_request(req, bb);
}
- return apreq_parse_request(apreq_request(env, NULL), ctx->bb);
+ return ctx.status;
}
/** @} */
Index: env/test_cgi.c
===================================================================
RCS file: /home/cvs/httpd-apreq-2/env/test_cgi.c,v
retrieving revision 1.4
diff -u -r1.4 test_cgi.c
--- env/test_cgi.c 24 Oct 2003 05:25:11 -0000 1.4
+++ env/test_cgi.c 24 Oct 2003 08:30:51 -0000
@@ -61,36 +61,20 @@
#include "apr_lib.h"
#include "apr_tables.h"
-struct env_ctx {
- apr_pool_t *pool;
- apreq_request_t *req;
- apreq_jar_t *jar;
- apr_bucket_brigade *bb;
- int loglevel;
- apr_status_t status;
-};
-
static int dump_table(void *count, const char *key, const char *value)
{
int *c = (int *) count;
- int value_len = strlen(value);
- if(value_len) *c = *c + strlen(key) + value_len;
+ int value_len = apreq_strlen(value);
+ if (value_len)
+ *c += strlen(key) + value_len;
return 1;
}
int main(int argc, char const * const * argv)
{
- struct env_ctx *ctx;
apr_pool_t *pool;
+ apreq_request_t *req;
const apreq_param_t *foo, *bar, *test, *key;
- apr_table_t *params;
- int count = 0;
- apr_status_t s;
- const apreq_jar_t *jar;
- apreq_cookie_t *cookie;
- apr_ssize_t ssize;
- apr_size_t size;
- char *dest;
atexit(apr_terminate);
if (apr_app_initialize(&argc, &argv, NULL) != APR_SUCCESS) {
@@ -103,61 +87,63 @@
exit(-1);
}
- ctx = (struct env_ctx *) apr_pcalloc(pool, sizeof *ctx);
- ctx->loglevel = 0;
- ctx->pool = pool;
- apreq_log(APREQ_DEBUG 0, ctx, "%s", "Creating apreq_request");
- ctx->req = apreq_request(ctx, NULL);
+ apreq_log(APREQ_DEBUG 0, pool, "%s", "Creating apreq_request");
+ req = apreq_request(pool, NULL);
printf("%s", "Content-Type: text/plain\n\n");
- apreq_log(APREQ_DEBUG 0, ctx, "%s", "Fetching the parameters");
+ apreq_log(APREQ_DEBUG 0, pool, "%s", "Fetching the parameters");
- foo = apreq_param(ctx->req, "foo");
- bar = apreq_param(ctx->req, "bar");
+ foo = apreq_param(req, "foo");
+ bar = apreq_param(req, "bar");
- test = apreq_param(ctx->req, "test");
- key = apreq_param(ctx->req, "key");
+ test = apreq_param(req, "test");
+ key = apreq_param(req, "key");
if (foo || bar) {
- if(foo) {
+ if (foo) {
printf("\t%s => %s\n", "foo", foo->v.data);
- apreq_log(APREQ_DEBUG 0, ctx, "%s => %s", "foo", foo->v.data);
+ apreq_log(APREQ_DEBUG 0, pool, "%s => %s", "foo", foo->v.data);
}
- if(bar) {
+ if (bar) {
printf("\t%s => %s\n", "bar", bar->v.data);
- apreq_log(APREQ_DEBUG 0, ctx, "%s => %s", "bar", bar->v.data);
+ apreq_log(APREQ_DEBUG 0, pool, "%s => %s", "bar", bar->v.data);
}
}
else if (test && key) {
- jar = apreq_jar(ctx, NULL);
- apreq_log(APREQ_DEBUG 0, ctx, "Fetching Cookie %s", key->v.data);
+ const apreq_jar_t *jar = apreq_jar(pool, NULL);
+ apreq_cookie_t *cookie;
+
+ apreq_log(APREQ_DEBUG 0, pool, "Fetching Cookie %s", key->v.data);
cookie = apreq_cookie(jar, key->v.data);
+
if (cookie == NULL) {
- apreq_log(APREQ_DEBUG 0, ctx,
+ apreq_log(APREQ_DEBUG 0, pool,
"No cookie for %s found!", key->v.data);
exit(-1);
}
if (strcmp(test->v.data, "bake") == 0) {
- s = apreq_cookie_bake(cookie, ctx);
+ apreq_cookie_bake(cookie, pool);
}
else if (strcmp(test->v.data, "bake2") == 0) {
- s = apreq_cookie_bake2(cookie, ctx);
+ apreq_cookie_bake2(cookie, pool);
}
else {
- size = strlen(cookie->v.data);
- dest = apr_palloc(ctx->pool, size + 1);
- ssize = apreq_decode(dest, cookie->v.data, size);
- printf("%s", dest);
+ char *dest = apr_palloc(pool, cookie->v.size + 1);
+ if (apreq_decode(dest, cookie->v.data, cookie->v.size) >= 0)
+ printf("%s", dest);
}
}
else {
- apreq_log(APREQ_DEBUG 0, ctx, "Fetching all parameters");
- params = apreq_params(ctx->pool, ctx->req);
+ apr_table_t *params = apreq_params(pool, req);
+ int count = 0;
+
+ apreq_log(APREQ_DEBUG 0, pool, "Fetching all parameters");
+
if (params == NULL) {
- apreq_log(APREQ_DEBUG 0, ctx, "No parameters found!");
+ apreq_log(APREQ_DEBUG 0, pool, "No parameters found!");
exit(-1);
}
apr_table_do(dump_table, &count, params, NULL);
Re: cvs commit: httpd-apreq-2/env/t/cgi-bin .cvsignore
Posted by Stas Bekman <st...@stason.org>.
Joe Schaefer wrote:
> Stas Bekman <st...@stason.org> writes:
>
>
>>Joe Schaefer wrote:
>>
>>
>>>... I had some problems running the
>>>tests with apr's current cvs- apr_env_get's behavior may have changed
>>>slightly, and I was getting segfaults on cgi tests 1-9. Initializing
>>>value = NULL took care of those.
>>
>>What was the exact problem?
>
>
> Segfaults on RH8, because apreq_env_header_in returns an uninitialized
> value whenever apr_env_get failed. On a lucky platform, that
> unitialized value was zero, which is fine. On my (unlucky) platform,
> the returned value was 1.
[...]
>>As long as you check for APR_SUCCESS the value should be OK to you
>>without initializing it. I think the workaround of initting it to NULL
>>hides the real problem.
>
>
> It's just fine so long as apr_env_get only mangles its first argument
> on APR_SUCCESS. apreq_env_header_in() must return NULL when apr_env_get
> fails; initializing value = NULL ensures that will happen.
what's the definition of 'failed'? There is no env var, or it has failed to
get it (not implemented, etc)?
>>I did suggested to use a macro APREQ_ENV_STATUS to make the testing
>
> I'm not a fan of that particular macro.
I just hate when those error checks clutter the code. As for APREQ_ENV_STATUS,
I just suggested it as an idea, at the moment it's broken, as it lets the
failures through (only warns).
__________________________________________________________________
Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org http://ticketmaster.com
Re: cvs commit: httpd-apreq-2/env/t/cgi-bin .cvsignore
Posted by Joe Schaefer <jo...@sunstarsys.com>.
Stas Bekman <st...@stason.org> writes:
> Joe Schaefer wrote:
>
> > ... I had some problems running the
> > tests with apr's current cvs- apr_env_get's behavior may have changed
> > slightly, and I was getting segfaults on cgi tests 1-9. Initializing
> > value = NULL took care of those.
>
> What was the exact problem?
Segfaults on RH8, because apreq_env_header_in returns an uninitialized
value whenever apr_env_get failed. On a lucky platform, that
unitialized value was zero, which is fine. On my (unlucky) platform,
the returned value was 1.
> apr_env_get hasn't changed and its implementation on unix (assuming
> that's where you had this problem) is as simple as:
>
> APR_DECLARE(apr_status_t) apr_env_get(char **value,
> const char *envvar,
> apr_pool_t *pool)
> {
> #ifdef HAVE_GETENV
>
> char *val = getenv(envvar);
> if (!val)
> return APR_ENOENT;
> *value = val;
> return APR_SUCCESS;
>
> #else
> return APR_ENOTIMPL;
> #endif
> }
>
> As long as you check for APR_SUCCESS the value should be OK to you
> without initializing it. I think the workaround of initting it to NULL
> hides the real problem.
It's just fine so long as apr_env_get only mangles its first argument
on APR_SUCCESS. apreq_env_header_in() must return NULL when apr_env_get
fails; initializing value = NULL ensures that will happen.
> I did suggested to use a macro APREQ_ENV_STATUS to make the testing
I'm not a fan of that particular macro.
--
Joe Schaefer
Re: cvs commit: httpd-apreq-2/env/t/cgi-bin .cvsignore
Posted by Stas Bekman <st...@stason.org>.
Joe Schaefer wrote:
> ... I had some problems running the
> tests with apr's current cvs- apr_env_get's behavior may have changed
> slightly, and I was getting segfaults on cgi tests 1-9. Initializing
> value = NULL took care of those.
What was the exact problem? apr_env_get hasn't changed and its implementation
on unix (assuming that's where you had this problem) is as simple as:
APR_DECLARE(apr_status_t) apr_env_get(char **value,
const char *envvar,
apr_pool_t *pool)
{
#ifdef HAVE_GETENV
char *val = getenv(envvar);
if (!val)
return APR_ENOENT;
*value = val;
return APR_SUCCESS;
#else
return APR_ENOTIMPL;
#endif
}
As long as you check for APR_SUCCESS the value should be OK to you without
initializing it. I think the workaround of initting it to NULL hides the real
problem. I did suggested to use a macro APREQ_ENV_STATUS to make the testing
concise,
but the problem with it is that it just warns about the problem, whereas it
should die.
I wasn't sure what's the right way to do that. But the problem is definitely
there. I'd suggest to revert it and figure out what's broken for real on the
way fixing APREQ_ENV_STATUS to be fatal.
__________________________________________________________________
Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org http://ticketmaster.com
Re: cvs commit: httpd-apreq-2/env/t/cgi-bin .cvsignore
Posted by Randy Kobes <ra...@theoryx5.uwinnipeg.ca>.
On Thu, 24 Oct 2003, Joe Schaefer wrote:
> randyk@apache.org writes:
>
> > randyk 2003/10/23 16:58:15
> >
> > Modified: env Makefile.am .cvsignore
> > Added: env test_cgi.c
> > env/t cgi.t
> > env/t/conf extra.conf.in
> > env/t/cgi-bin .cvsignore
> > Log:
> > add libapreq_cgi tests.
>
> After the Marlins big win tonight,
It's not the same since the Blue Jays fell into disrepair :)
> I had some problems running the
> tests with apr's current cvs- apr_env_get's behavior may have changed
> slightly, and I was getting segfaults on cgi tests 1-9. Initializing
> value = NULL took care of those. Also test 22 uses GET_BODY with a
> "Content" argument; I'm not sure if that's legit or not, so I changed
> it to POST_BODY.
Sorry about that - that was a remnant of me switching back
and forth betwen GET and POST when there were problems under
env/t/, which you fixed ...
> The last criticism is that we should eventually get rid of the
> env_ctx struct in test_cgi.c, and use a static global variable in
> libapreq_cgi instead. There's no natural candidate for "env" in a cgi
> context, so IMO we should probably ignore all the "env" arguments and
> let users put anything they like in that slot.
>
> Anyways, for now here's the diff from env/ that fixes the segfaults.
> Let me know if it works ok.
It works fine for me under Win32 ... Thanks.
--
best regards,
randy