I ran into a problem converting some of my code to run with Hunchentoot 1.0. This patch fixes it and -- as far as I can tell after a bit of poking around -- is possibly righteous. What do you think? The basic problem that I ran into is explained in the comment in the patch.
-Peter
diff -r 635083ac9419 hunchentoot/request.lisp --- a/hunchentoot/request.lisp Sun May 31 11:31:25 2009 -0700 +++ b/hunchentoot/request.lisp Mon Jun 01 14:19:19 2009 -0700 @@ -346,7 +346,12 @@ (get-parameters request))
(defmethod post-parameters :before ((request request)) - (maybe-read-post-parameters :request request)) + ;; Force here because if someone calls POST-PARAMETERS they actually + ;; want them, regardless of why the RAW-POST-DATA has been filled + ;; in. (For instance, if SEND-HEADERS has been called, filling in + ;; RAW-POST-DATA, and then subsequent code calls POST-PARAMETERS, + ;; without the :FORCE flag POST-PARAMETERS would return NIL.) + (maybe-read-post-parameters :request request :force t))
(defun post-parameters* (&optional (request *request*)) "Returns an alist of the POST parameters associated with the REQUEST
Peter Seibel wrote:
I ran into a problem converting some of my code to run with Hunchentoot 1.0. This patch fixes it and -- as far as I can tell after a bit of poking around -- is possibly righteous. What do you think? The basic problem that I ran into is explained in the comment in the patch.
-Peter
diff -r 635083ac9419 hunchentoot/request.lisp --- a/hunchentoot/request.lisp Sun May 31 11:31:25 2009 -0700 +++ b/hunchentoot/request.lisp Mon Jun 01 14:19:19 2009 -0700 @@ -346,7 +346,12 @@ (get-parameters request))
(defmethod post-parameters :before ((request request))
- (maybe-read-post-parameters :request request))
- ;; Force here because if someone calls POST-PARAMETERS they actually
- ;; want them, regardless of why the RAW-POST-DATA has been filled
- ;; in. (For instance, if SEND-HEADERS has been called, filling in
- ;; RAW-POST-DATA, and then subsequent code calls POST-PARAMETERS,
- ;; without the :FORCE flag POST-PARAMETERS would return NIL.)
- (maybe-read-post-parameters :request request :force t))
(defun post-parameters* (&optional (request *request*)) "Returns an alist of the POST parameters associated with the REQUEST
Peter, did you ever get any response to this?
Jeff
On Fri, Jun 12, 2009 at 6:05 AM, Jeff Cunninghamj.k.cunningham@comcast.net wrote:
Peter Seibel wrote:
I ran into a problem converting some of my code to run with Hunchentoot 1.0. This patch fixes it and -- as far as I can tell after a bit of poking around -- is possibly righteous. What do you think? The basic problem that I ran into is explained in the comment in the patch.
-Peter
diff -r 635083ac9419 hunchentoot/request.lisp --- a/hunchentoot/request.lisp Sun May 31 11:31:25 2009 -0700 +++ b/hunchentoot/request.lisp Mon Jun 01 14:19:19 2009 -0700 @@ -346,7 +346,12 @@ (get-parameters request))
(defmethod post-parameters :before ((request request))
- (maybe-read-post-parameters :request request))
- ;; Force here because if someone calls POST-PARAMETERS they actually
- ;; want them, regardless of why the RAW-POST-DATA has been filled
- ;; in. (For instance, if SEND-HEADERS has been called, filling in
- ;; RAW-POST-DATA, and then subsequent code calls POST-PARAMETERS,
- ;; without the :FORCE flag POST-PARAMETERS would return NIL.)
- (maybe-read-post-parameters :request request :force t))
(defun post-parameters* (&optional (request *request*)) "Returns an alist of the POST parameters associated with the REQUEST
Peter, did you ever get any response to this?
Nope.
-Peter
On Fri, Jun 12, 2009 at 3:31 PM, Peter Seibelpeter@gigamonkeys.com wrote:
Nope.
Yes, sorry. That I didn't reply yet doesn't mean that I'm not interested or that I don't like the patch or whatever, it just means that I'm pretty busy with other things and have been so for quite some time. I'll get around to look at all the unanswered emails related to Hunchentoot, CL-WHO, etc. but this will take some more time... :(
Cheers, Edi.
I sent this almost two months ago when Edi was very busy. I'm hoping someone can tell me if this is a righteous patch.
-Peter
---------- Forwarded message ---------- From: Peter Seibel peter@gigamonkeys.com Date: Mon, Jun 1, 2009 at 2:21 PM Subject: Problem with late calls to POST-PARAMETERS To: tbnl-devel@common-lisp.net
I ran into a problem converting some of my code to run with Hunchentoot 1.0. This patch fixes it and -- as far as I can tell after a bit of poking around -- is possibly righteous. What do you think? The basic problem that I ran into is explained in the comment in the patch.
-Peter
diff -r 635083ac9419 hunchentoot/request.lisp --- a/hunchentoot/request.lisp Sun May 31 11:31:25 2009 -0700 +++ b/hunchentoot/request.lisp Mon Jun 01 14:19:19 2009 -0700 @@ -346,7 +346,12 @@ (get-parameters request))
(defmethod post-parameters :before ((request request)) - (maybe-read-post-parameters :request request)) + ;; Force here because if someone calls POST-PARAMETERS they actually + ;; want them, regardless of why the RAW-POST-DATA has been filled + ;; in. (For instance, if SEND-HEADERS has been called, filling in + ;; RAW-POST-DATA, and then subsequent code calls POST-PARAMETERS, + ;; without the :FORCE flag POST-PARAMETERS would return NIL.) + (maybe-read-post-parameters :request request :force t))
(defun post-parameters* (&optional (request *request*)) "Returns an alist of the POST parameters associated with the REQUEST
-- Peter Seibel http://www.codersatwork.com/ http://www.gigamonkeys.com/blog/
Peter Seibel wrote:
I sent this almost two months ago when Edi was very busy. I'm hoping someone can tell me if this is a righteous patch.
I'll try to make a start:
What's the use case for this? Why set RAW-POST-DATA and then attempt to read it again using POST-PARAMETERS?
Leslie
Because I didn't call RAW-POST-DATA; it was called by the Hunchentoot internals when I called SEND-HEADERS. And after that without my patch you can no longer get anything via POST-PARAMETERS. I don't see any reason that calling SEND-HEADERS should make it impossible to get at POST-PARAMETERS (and pre-1.0 it didn't which is how I ran into this problem--my previously working code stopped working when I upgraded to 1.0)
-Peter
On Wed, Jul 22, 2009 at 12:45 AM, Leslie P. Polzersky@viridian-project.de wrote:
Peter Seibel wrote:
I sent this almost two months ago when Edi was very busy. I'm hoping someone can tell me if this is a righteous patch.
I'll try to make a start:
What's the use case for this? Why set RAW-POST-DATA and then attempt to read it again using POST-PARAMETERS?
Leslie
-- http://www.linkedin.com/in/polzer
tbnl-devel site list tbnl-devel@common-lisp.net http://common-lisp.net/mailman/listinfo/tbnl-devel
Peter Seibel wrote:
Because I didn't call RAW-POST-DATA; it was called by the Hunchentoot internals when I called SEND-HEADERS. And after that without my patch you can no longer get anything via POST-PARAMETERS. I don't see any reason that calling SEND-HEADERS should make it impossible to get at POST-PARAMETERS (and pre-1.0 it didn't which is how I ran into this problem--my previously working code stopped working when I upgraded to 1.0)
I see. So is there a pressing reason to call SEND-HEADERS before collecting post parameters?
Leslie
On Wed, Jul 22, 2009 at 7:33 AM, Leslie P. Polzersky@viridian-project.de wrote:
Peter Seibel wrote:
Because I didn't call RAW-POST-DATA; it was called by the Hunchentoot internals when I called SEND-HEADERS. And after that without my patch you can no longer get anything via POST-PARAMETERS. I don't see any reason that calling SEND-HEADERS should make it impossible to get at POST-PARAMETERS (and pre-1.0 it didn't which is how I ran into this problem--my previously working code stopped working when I upgraded to 1.0)
I see. So is there a pressing reason to call SEND-HEADERS before collecting post parameters?
Well, a big pile of previously working code. And if you're not allowed to call POST-PARAMETERS after SEND-HEADERS then the docs should say so. So is it easier to patch the code to allow the old (and sometimes useful behavior) or patch the docs to reflect a somewhat arbitrary restriction?
-Peter
I think the actual problem I had was some code that did essentially:
(with-foo-output ((send-headers)) (expand-template-file))
where expand-template-file calls code that uses post-parameters to get at, well, post parameters. It's not clear how to easily turn that code inside out so all the parameters are gathered in advance of calling send-headers.
-Peter
On Wed, Jul 22, 2009 at 7:44 AM, Peter Seibelpeter@gigamonkeys.com wrote:
On Wed, Jul 22, 2009 at 7:33 AM, Leslie P. Polzersky@viridian-project.de wrote:
Peter Seibel wrote:
Because I didn't call RAW-POST-DATA; it was called by the Hunchentoot internals when I called SEND-HEADERS. And after that without my patch you can no longer get anything via POST-PARAMETERS. I don't see any reason that calling SEND-HEADERS should make it impossible to get at POST-PARAMETERS (and pre-1.0 it didn't which is how I ran into this problem--my previously working code stopped working when I upgraded to 1.0)
I see. So is there a pressing reason to call SEND-HEADERS before collecting post parameters?
Well, a big pile of previously working code. And if you're not allowed to call POST-PARAMETERS after SEND-HEADERS then the docs should say so. So is it easier to patch the code to allow the old (and sometimes useful behavior) or patch the docs to reflect a somewhat arbitrary restriction?
-Peter
-- Peter Seibel http://www.codersatwork.com/ http://www.gigamonkeys.com/blog/
Peter Seibel wrote:
Well, a big pile of previously working code. And if you're not allowed to call POST-PARAMETERS after SEND-HEADERS then the docs should say so. So is it easier to patch the code to allow the old (and sometimes useful behavior) or patch the docs to reflect a somewhat arbitrary restriction?
I'd rather have the former. In fact I'd been in favor of that patch before, but it seemed necessary in any case to discuss the benefits.
I can't commit your patch but I hope this discussion helps Hans and Edi to do so.
Leslie
Peter, I have committed your fix to the development version of Hunchentoot. Leslie, thank you for reviewing it!
Apologies for the long time that this took.
-Hans
On Wed, Jul 22, 2009 at 17:09, Leslie P. Polzersky@viridian-project.de wrote:
Peter Seibel wrote:
Well, a big pile of previously working code. And if you're not allowed to call POST-PARAMETERS after SEND-HEADERS then the docs should say so. So is it easier to patch the code to allow the old (and sometimes useful behavior) or patch the docs to reflect a somewhat arbitrary restriction?
I'd rather have the former. In fact I'd been in favor of that patch before, but it seemed necessary in any case to discuss the benefits.
I can't commit your patch but I hope this discussion helps Hans and Edi to do so.
Leslie
-- http://www.linkedin.com/in/polzer
tbnl-devel site list tbnl-devel@common-lisp.net http://common-lisp.net/mailman/listinfo/tbnl-devel
Cool, thanks guys.
-Peter
On Wed, Jul 22, 2009 at 9:25 AM, Hans Hübnerhans.huebner@gmail.com wrote:
Peter, I have committed your fix to the development version of Hunchentoot. Leslie, thank you for reviewing it!
Apologies for the long time that this took.
-Hans
On Wed, Jul 22, 2009 at 17:09, Leslie P. Polzersky@viridian-project.de wrote:
Peter Seibel wrote:
Well, a big pile of previously working code. And if you're not allowed to call POST-PARAMETERS after SEND-HEADERS then the docs should say so. So is it easier to patch the code to allow the old (and sometimes useful behavior) or patch the docs to reflect a somewhat arbitrary restriction?
I'd rather have the former. In fact I'd been in favor of that patch before, but it seemed necessary in any case to discuss the benefits.
I can't commit your patch but I hope this discussion helps Hans and Edi to do so.
Leslie
-- http://www.linkedin.com/in/polzer
tbnl-devel site list tbnl-devel@common-lisp.net http://common-lisp.net/mailman/listinfo/tbnl-devel
Hans Hübner hans.huebner@gmail.com writes:
Peter, I have committed your fix to the development version of Hunchentoot. Leslie, thank you for reviewing it!
As of r4438 (Peter's patch), I believe this wipes out post-parameters of multipart/form-data POST requests and causes the relevant requests to time out.
Specifically, on a multipart/form-data request, the first call to `post-parameters` yields a sensible result, whereas future calls yield NIL after timing out.
The solution proper is in the first hunk of the patch below, which removes the ability to FORCE maybe-read-post-parameters to reread a multipart/form-data request, because it can't do that, having emptied the stream of the necessary data.
The second hunk, which incidentally fixes the problem as well (only by avoiding the confusing behavior of maybe-read-post-parameters in the common case of POSTing a non-empty set of parameters, rather than solving it), is a specific followup to r4438, which in combination with the first hunk merely prevents post-parameters from recalculating the alist from the raw data on every call.
Were the first hunk to be changed to warn in the specific case it avoids, the second hunk would also serve to eliminate spurious warnings about being unable to reread multipart posts. So I believe including both would be ideal.
The patch looks good, I have committed it to the development version. We really need more and better test cases, to state the obvious.
Thanks! Hans
On Thu, Aug 20, 2009 at 23:28, Stephen Compalls11@member.fsf.org wrote:
As of r4438 (Peter's patch), I believe this wipes out post-parameters of multipart/form-data POST requests and causes the relevant requests to time out.
Specifically, on a multipart/form-data request, the first call to `post-parameters` yields a sensible result, whereas future calls yield NIL after timing out.
The solution proper is in the first hunk of the patch below, which removes the ability to FORCE maybe-read-post-parameters to reread a multipart/form-data request, because it can't do that, having emptied the stream of the necessary data.
The second hunk, which incidentally fixes the problem as well (only by avoiding the confusing behavior of maybe-read-post-parameters in the common case of POSTing a non-empty set of parameters, rather than solving it), is a specific followup to r4438, which in combination with the first hunk merely prevents post-parameters from recalculating the alist from the raw data on every call.
Were the first hunk to be changed to warn in the specific case it avoids, the second hunk would also serve to eliminate spurious warnings about being unable to reread multipart posts.
On Wed, Jul 22, 2009 at 5:09 PM, Leslie P. Polzersky@viridian-project.de wrote:
I can't commit your patch but I hope this discussion helps Hans and Edi to do so.
It did. And it reminded us to finally do something... :)
I agree that Peter's patch looks OK and is generally to be preferred over disallowing to use POST-PARAMETERS after having called SEND-HEADERS. However, as a general style rule, I'd recommend to do as much work as possible /before/ calling SEND-HEADERS (i.e. while errors can still be handled gracefully).
Patch will be applied to the dev version.
Thanks, Edi.
-
Edi Weitz -
Hans Hübner -
Jeff Cunningham -
Leslie P. Polzer -
Peter Seibel -
Stephen Compall