| Subject: | HTTP::GHTTP v1.06 set_body() bug |
In HTTP::GHTTP v1.06 (linked with libghttp-1.0.9), the set_body()
method doesn't correctly increment the reference count on the scalar
passed to it. Therefore, the body eventually sent in a HTTP request
(such as POST) may be junk, instead of the intended content.
Code much like the following produces the error on FreeBSD 4.5 with
perl 5.005_03:
sub set_post_body {
my ( $self, $value ) = @_;
$self->method_post();
$self->get_ghttp()->set_body( $value );
}
Attached is a patch which changes GHTTP.xs to correctly increment
and decrement the reference count on the scalar passed to set_body().
My limited testing shows that it works as expected now. However,
there may be other issues like this lurking in the libghttp library;
I'm not particularly familiar with it.
Another solution would malloc() and bcopy() the passed scalar, and
then free() it in the DESTROY method. Is this something libghttp
itself should be doing?
diff -u -r HTTP-GHTTP-1.06/GHTTP.xs HTTP-GHTTP-1.06-new/GHTTP.xs
--- HTTP-GHTTP-1.06/GHTTP.xs Thu Jan 18 09:42:58 2001
+++ HTTP-GHTTP-1.06-new/GHTTP.xs Tue Feb 26 14:42:52 2002
@@ -11,33 +11,50 @@
}
#endif
+typedef struct {
+ ghttp_request *ghttp;
+ SV *set_body_sv;
+} http_ghttp_class;
+
MODULE = HTTP::GHTTP PACKAGE = HTTP::GHTTP
-ghttp_request *
+http_ghttp_class *
_new(CLASS)
char * CLASS
+ PREINIT:
+ http_ghttp_class *self;
CODE:
- RETVAL = ghttp_request_new();
- if (RETVAL == NULL) {
+ self = malloc( sizeof(http_ghttp_class) );
+ if ( !self ) {
+ warn("Unable to allocate http_ghttp_class");
+ XSRETURN_UNDEF;
+ }
+ self->ghttp = ghttp_request_new();
+ if ( !self->ghttp ) {
warn("Unable to allocate ghttp_request");
XSRETURN_UNDEF;
}
+ self->set_body_sv = NULL;
+ RETVAL = self;
/* sv_bless(RETVAL, gv_stash_pv(CLASS, 1)); */
OUTPUT:
RETVAL
void
DESTROY(self)
- ghttp_request *self
+ http_ghttp_class *self
CODE:
- ghttp_request_destroy(self);
+ ghttp_request_destroy(self->ghttp);
+ if ( self->set_body_sv )
+ SvREFCNT_dec( self->set_body_sv );
+ free( self );
int
set_uri(self, uri)
- ghttp_request *self
+ http_ghttp_class *self
char *uri
CODE:
- if(ghttp_set_uri(self, uri) == ghttp_error) {
+ if(ghttp_set_uri(self->ghttp, uri) == ghttp_error) {
XSRETURN_UNDEF;
}
RETVAL = 1;
@@ -46,43 +63,43 @@
int
set_proxy(self, proxy)
- ghttp_request *self
+ http_ghttp_class *self
char *proxy
CODE:
- RETVAL = ghttp_set_proxy(self, proxy);
+ RETVAL = ghttp_set_proxy(self->ghttp, proxy);
OUTPUT:
RETVAL
void
set_header(self, hdr, val)
- ghttp_request *self
+ http_ghttp_class *self
const char *hdr
const char *val
CODE:
- ghttp_set_header(self, hdr, val);
+ ghttp_set_header(self->ghttp, hdr, val);
void
process_request(self)
- ghttp_request *self
+ http_ghttp_class *self
CODE:
- ghttp_prepare(self);
- ghttp_process(self);
+ ghttp_prepare(self->ghttp);
+ ghttp_process(self->ghttp);
int
prepare(self)
- ghttp_request *self
+ http_ghttp_class *self
CODE:
- RETVAL = ghttp_prepare(self);
+ RETVAL = ghttp_prepare(self->ghttp);
OUTPUT:
RETVAL
int
process(self)
- ghttp_request *self
+ http_ghttp_class *self
PREINIT:
ghttp_status process_status;
CODE:
- process_status = ghttp_process(self);
+ process_status = ghttp_process(self->ghttp);
if (process_status == ghttp_error) {
XSRETURN_UNDEF;
}
@@ -92,10 +109,10 @@
const char*
get_header(self, hdr)
- ghttp_request *self
+ http_ghttp_class *self
const char *hdr
CODE:
- RETVAL = ghttp_get_header(self, hdr);
+ RETVAL = ghttp_get_header(self->ghttp, hdr);
OUTPUT:
RETVAL
@@ -103,13 +120,13 @@
void
get_headers(self)
- ghttp_request *self
+ http_ghttp_class *self
PREINIT:
char **hdrs;
int num_hdrs;
int i;
PPCODE:
- if (ghttp_get_header_names(self, &hdrs, &num_hdrs) == -1) {
+ if (ghttp_get_header_names(self->ghttp, &hdrs, &num_hdrs) == -1) {
XSRETURN_UNDEF;
}
@@ -124,102 +141,107 @@
int
close(self)
- ghttp_request *self
+ http_ghttp_class *self
CODE:
- RETVAL = ghttp_close(self);
+ RETVAL = ghttp_close(self->ghttp);
OUTPUT:
RETVAL
SV *
get_body(self)
- ghttp_request *self
+ http_ghttp_class *self
PREINIT:
SV* buffer;
CODE:
buffer = newSVpvn("",0);
- sv_catpvn(buffer, ghttp_get_body(self), ghttp_get_body_len(self));
+ sv_catpvn(buffer, ghttp_get_body(self->ghttp), ghttp_get_body_len(self->ghttp));
RETVAL = buffer;
OUTPUT:
RETVAL
const char *
get_error(self)
- ghttp_request *self
+ http_ghttp_class *self
CODE:
- RETVAL = ghttp_get_error(self);
+ RETVAL = ghttp_get_error(self->ghttp);
OUTPUT:
RETVAL
int
set_authinfo(self, user, pass)
- ghttp_request *self
+ http_ghttp_class *self
const char *user
const char *pass
CODE:
- RETVAL = ghttp_set_authinfo(self, user, pass);
+ RETVAL = ghttp_set_authinfo(self->ghttp, user, pass);
OUTPUT:
RETVAL
int
set_proxy_authinfo(self, user, pass)
- ghttp_request *self
+ http_ghttp_class *self
const char *user
const char *pass
CODE:
- RETVAL = ghttp_set_proxy_authinfo(self, user, pass);
+ RETVAL = ghttp_set_proxy_authinfo(self->ghttp, user, pass);
OUTPUT:
RETVAL
int
set_type(self, type)
- ghttp_request *self
+ http_ghttp_class *self
int type
CODE:
- RETVAL = ghttp_set_type(self, type);
+ RETVAL = ghttp_set_type(self->ghttp, type);
OUTPUT:
RETVAL
int
set_body(self, body)
- ghttp_request *self
+ http_ghttp_class *self
SV *body
PREINIT:
STRLEN len;
char * str;
CODE:
+ if ( self->set_body_sv )
+ SvREFCNT_dec( self->set_body_sv );
+ self->set_body_sv = body;
+ SvREFCNT_inc( body );
str = SvPV(body, len);
- RETVAL = ghttp_set_body(self, str, len);
+ RETVAL = ghttp_set_body(self->ghttp, str, len);
+
OUTPUT:
RETVAL
int
_get_socket(self)
- ghttp_request *self
+ http_ghttp_class *self
CODE:
- RETVAL = ghttp_get_socket(self);
+ RETVAL = ghttp_get_socket(self->ghttp);
OUTPUT:
RETVAL
void
get_status(self)
- ghttp_request *self
+ http_ghttp_class *self
PREINIT:
int code;
const char *reason;
PPCODE:
- code = ghttp_status_code(self);
- reason = ghttp_reason_phrase(self);
+ code = ghttp_status_code(self->ghttp);
+ reason = ghttp_reason_phrase(self->ghttp);
EXTEND(SP, 2);
PUSHs(sv_2mortal(newSViv(code)));
PUSHs(sv_2mortal(newSVpv((char*)reason, 0)));
void
current_status(self)
- ghttp_request *self
+ http_ghttp_class *self
PREINIT:
ghttp_current_status status;
PPCODE:
- status = ghttp_get_status(self);
+ status = ghttp_get_status(self->ghttp);
EXTEND(SP, 3);
PUSHs(sv_2mortal(newSViv(status.proc)));
PUSHs(sv_2mortal(newSViv(status.bytes_read)));
@@ -227,18 +249,18 @@
int
set_async(self)
- ghttp_request *self
+ http_ghttp_class *self
CODE:
- RETVAL = ghttp_set_sync(self, ghttp_async);
+ RETVAL = ghttp_set_sync(self->ghttp, ghttp_async);
OUTPUT:
RETVAL
void
set_chunksize(self, size)
- ghttp_request *self
+ http_ghttp_class *self
int size
CODE:
- ghttp_set_chunksize(self, size);
+ ghttp_set_chunksize(self->ghttp, size);
#
# CONSTANTS
diff -u -r HTTP-GHTTP-1.06/typemap HTTP-GHTTP-1.06-new/typemap
--- HTTP-GHTTP-1.06/typemap Tue Nov 21 11:58:02 2000
+++ HTTP-GHTTP-1.06-new/typemap Tue Feb 26 14:42:14 2002
@@ -1,6 +1,6 @@
TYPEMAP
const char * T_PV
-ghttp_request * O_OBJECT
+http_ghttp_class * O_OBJECT
INPUT
O_OBJECT