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 Jim Winstead <ji...@trainedmonkey.com> on 2000/12/12 02:07:01 UTC
Re: libapreq (preliminary comments on the multipart.c code)
On Dec 11, Joe Schaefer wrote:
> I just got an account with apache.org (user joes). Have you
> been able to get a CVS tree going for apreq at apache.org?
close. i've got doug's cvs tree, now i just have to wait for brian
to fix the permissions on a directory and then integrate the stuff
we've been working on.
there's also new apreq-dev and apreq-cvs mailing lists (@httpd.apache.org,
ezmlm-managed like the mod_perl lists). i've cc'd this there so it
appears in the ezmlm archives.
> I put together a preliminary list.c/list.h API for
> dealing with dynamically resizing buffers. It's tailor
> made for handling most of the cumbersome problems that
> we face with implementing multipart/form-data posts. I'd
> appreciate feedback on how to improve it (one thing
> I'm considering is a direct BUFF interface.) The
> files are located at
cool. i haven't had a chance to dig in yet, but it sounds great at
first glance. i wouldn't worry about a direct BUFF interface, since
one of the ToDo items is to port to apache-2.0, and BUFF is gone
in 2.0. :)
it may be worth looking at the 'ring' code from apr-util at
http://www.apache.org/websrc/viewcvs.cgi/apr-util/include/ap_ring.h.
> 2) There seems to be an accounting error at the end of
> multipart_buffer_read. On a terminal CRLF, it should
> reduce the return value by one more (--len):
good catch.
> Also, one serious problem with your last patch: the arrows
> in your linked list point the wrong way :) I have a patch
> to fix this, but obviously I'd prefer it if we incorporated
> the list.[ch] code I wrote to specfically deal with these
> issues.
d'oh! i made sure i had the arguments to memcpy() the right way
'round (they weren't initially), but i missed that. shows you how
much testing i did to check it was correct versus uses less memory. :)
jim
Re: libapreq (preliminary comments on the multipart.c code)
Posted by Joe Schaefer <jo...@sunstarsys.com>.
Joe Schaefer <jo...@sunstarsys.com> writes:
> There's still something that I haven't groked yet about how the
> multipart_buffer_read code works on boundary reads. As I can't come
> up with a real test page that breaks it, perhaps you could set me
> straight on how it handles partial boundary reads.
[ snip slightly misleading picture ]
> As I said, I can't produce this behavior with a real example, so
> I'm probably missing something. I guess I'm just skeptical
> that partial boundary matches actually work right in exceptional
> cases like this.
I got it now- my picture was wrong. You don't modify self->buffer
at all- just the copied version in buf. Very slick and efficient-
I'm no longer a skeptic :)
--
Joe Schaefer
Re: libapreq (preliminary comments on the multipart.c code)
Posted by Joe Schaefer <jo...@sunstarsys.com>.
Jim Winstead <ji...@trainedmonkey.com> writes:
> On Dec 11, Joe Schaefer wrote:
> > I just got an account with apache.org (user joes). Have you
> > been able to get a CVS tree going for apreq at apache.org?
>
> close. i've got doug's cvs tree, now i just have to wait for brian
> to fix the permissions on a directory and then integrate the stuff
> we've been working on.
>
> there's also new apreq-dev and apreq-cvs mailing lists (@httpd.apache.org,
> ezmlm-managed like the mod_perl lists). i've cc'd this there so it
> appears in the ezmlm archives.
Got it- this one was mailed only to the list.
>
> it may be worth looking at the 'ring' code from apr-util at
> http://www.apache.org/websrc/viewcvs.cgi/apr-util/include/ap_ring.h.
Thanks for the url- looks nice (and very relevant). I should
have a reference implementation for the multipart_buffer code
using list.c,h ready for testing in a day or two. (I've got
one working now, but I'm still tinkering with it a little.)
>
> > 2) There seems to be an accounting error at the end of
> > multipart_buffer_read. On a terminal CRLF, it should
> > reduce the return value by one more (--len):
>
> good catch.
>
There's still something that I haven't groked yet about how the
multipart_buffer_read code works on boundary reads. As I can't come
up with a real test page that breaks it, perhaps you could set me
straight on how it handles partial boundary reads.
Say there's a textarea posted that looks like this (string form):
...blah blah.CRLF---- section 12 of this lovely document ----CRLF...
^^^^^^^^^^^^^^^^^^^
self->buffer ends here
i.e. self->buffer = ...blah blah.CRLF--\0 (strlen = 5119)
What I think happens is that multipart_buffer_read would recognize
the (false) partial boundary in self->buffer and change this to
...blah blah.\0\0--\0
^
(self->buf_begin)
Subsequently, I believe the ultimate string would become
...blah blah.---- section 12 of this lovely document ----CRLF...
^^
CRLF dropped
As I said, I can't produce this behavior with a real example, so
I'm probably missing something. I guess I'm just skeptical
that partial boundary matches actually work right in exceptional
cases like this.
--
Joe Schaefer
Re: Making Apache::Request a singleton
Posted by Jim Winstead <ji...@apache.org>.
On Dec 12, Joe Schaefer wrote:
> Matt Sergeant <ma...@sergeant.org> writes:
> > Can we make Apache::Request a singleton? Currently if you try to create
> > one after it has been initialised once it fails to see anything in the
> > POST data (because its already been read).
>
> I think that was one of the improvements Jim has planned - persistence
> throughout the entire request, so later handlers can access post data
> should they wish.
right, this is definitely something i want to address, but preferably
at the level of the c code so that everyone benefits. i really want
to get out libapreq-0.32 that at least fixes the outstanding bugs
against 0.31 first, though.
jim
Re: Making Apache::Request a singleton
Posted by Joe Schaefer <jo...@sunstarsys.com>.
(I narrowed cc's to the apreq-dev list).
Matt Sergeant <ma...@sergeant.org> writes:
> Can we make Apache::Request a singleton? Currently if you try to create
> one after it has been initialised once it fails to see anything in the
> POST data (because its already been read).
I think that was one of the improvements Jim has planned - persistence
throughout the entire request, so later handlers can access post data
should they wish.
--
Joe Schaefer
Re: Making Apache::Request a singleton
Posted by Matt Sergeant <ma...@sergeant.org>.
On Tue, 12 Dec 2000, Doug MacEachern wrote:
> On Tue, 12 Dec 2000, Matt Sergeant wrote:
>
> > Can we make Apache::Request a singleton? Currently if you try to create
> > one after it has been initialised once it fails to see anything in the
> > POST data (because its already been read).
>
> i think the new() method should stay as-is. if somebody wants singleton
> they should ask for it, via a singleton() method (or whatever name), or by
> subclassing Apache::Request.
> and, pnotes() should probably used rather than a global. i don't think
> subrequests should inherit the Apache::Request object for the main
> request.
OK thats fine I'll get a new patch together. Is the code in CVS yet?
--
<Matt/>
/|| ** Director and CTO **
//|| ** AxKit.com Ltd ** ** XML Application Serving **
// || ** http://axkit.org ** ** XSLT, XPathScript, XSP **
// \\| // ** Personal Web Site: http://sergeant.org/ **
\\//
//\\
// \\
Re: Making Apache::Request a singleton
Posted by Doug MacEachern <do...@covalent.net>.
On Tue, 12 Dec 2000, Matt Sergeant wrote:
> Can we make Apache::Request a singleton? Currently if you try to create
> one after it has been initialised once it fails to see anything in the
> POST data (because its already been read).
i think the new() method should stay as-is. if somebody wants singleton
they should ask for it, via a singleton() method (or whatever name), or by
subclassing Apache::Request.
and, pnotes() should probably used rather than a global. i don't think
subrequests should inherit the Apache::Request object for the main
request.
Making Apache::Request a singleton
Posted by Matt Sergeant <ma...@sergeant.org>.
Can we make Apache::Request a singleton? Currently if you try to create
one after it has been initialised once it fails to see anything in the
POST data (because its already been read).
This patch should fix that, though I've not actually tested it yet:
--- Request.pm.old Tue Dec 12 16:53:38 2000
+++ Request.pm Tue Dec 12 16:59:31 2000
@@ -11,6 +11,15 @@
__PACKAGE__->mod_perl::boot($VERSION);
}
+sub new {
+ my ($class, $r, @extras) = @_;
+ if ($Apache::Request::_instance) {
+ return $Apache::Request::_instance;
+ }
+ $r->register_cleanup( sub { undef $Apache::Request::_instance; } );
+ return $Apache::Request::_instance = $class->new_instance($r,
@extras);
+}
+
#just prototype methods here, moving to xs later
sub param {
my $self = shift;
--- Request.xs.old Tue Dec 12 16:52:52 2000
+++ Request.xs Tue Dec 12 16:53:31 2000
@@ -105,7 +105,7 @@
av_push(perl_get_av("Apache::Request::ISA",TRUE),
newSVpv("Apache",6));
Apache::Request
-ApacheRequest_new(class, r, ...)
+ApacheRequest_new_instance(class, r, ...)
SV *class
Apache r
--
<Matt/>
/|| ** Director and CTO **
//|| ** AxKit.com Ltd ** ** XML Application Serving **
// || ** http://axkit.org ** ** XSLT, XPathScript, XSP **
// \\| // ** Personal Web Site: http://sergeant.org/ **
\\//
//\\
// \\