roxen.lists.roxen.general

Subject Author Date
POST variables (Re: Lots of patches that can be integrated into Roxen 5.0 in a heartbeat) Stephen R. van den Berg <srb[at]cuci[dot]nl> 12-01-2009
Martin Stjernholm wrote:
>>> commit 9d5f70747761a4e28cab901e2180a1174b073e39
>>>     Preserve POST variables upon redirect

>Did a bit different solution: Kept the POST vars in id->misc to avoid
>another member in every request object, and populated it separate from
>real_variables to ensure that other variables don't creep in there.

Sounds ok, will test that.

>@@ -2878,7 +2880,15 @@ object clone_me()
>-  c->real_variables = copy_value( real_variables );
>+  mapping nf=([]);
>+  if(RXML_CONTEXT)
>+  { mapping form=RXML_CONTEXT->get_scope ("form");
>+    foreach(indices(form),string idx)              
>+    { mixed t=form[idx];
>+      nf+=([idx:arrayp(t)?t:()]);
>+    }
>+  }
>+  c->real_variables = nf;

>I think this is wrong since it can cause variables to "bleed over" if
>a subrequest is made inside an essentually unrelated rxml parse
>session. If the subrequest is related then the form scope would be
>synched with real_variables already (in fact, the form scope is an
>object wrapper that accesses the same real_variables mapping, so the
>above only becomes a very convoluted mapping copy).

The bleed-over effect was never intended.  I think that this was the
closest I could get to a modified "copy_values".  It was a long time ago
when I added this.

>And besides, it doesn't look to me that it's necessary for the
>redirect thingy to work. Am I wrong?

As far as I can recall, it was necessary, something broke otherwise.
I have to admit, it's been over four years since I debugged and added
this, I forgot the details.  I'll take it out, and see what happens,
shouldn't be too difficult to reproduce.

>      nf+=([idx:arrayp(t)?t:()]);

>optimize away the creation of the small mapping. But anyway, to avoid
>such worries, why not just use a mapping assignment?

>      nf[idx] = arrayp(t)?t:();

>It's shorter too. :)

Thanks.  Well, quite possibly, whenever you see a construct like this,
it most likely is older code, but I'll quickly check the pgsql driver
to see if I'm still making this mistake.

>>> commit f91d0e08d845fd9c47fae961333d1fe47d16c7b1
>>>     Prevent spurious connection resets due to async sockets

>Too dangerous. That could make close() block on tls connections, and
>it mustn't block there. Need an analysis of the problem to decide on
>how to get this right.
>I don't understand your comment either.

Well, I know for a fact that something needs fixing here.
The problem is timing related, so it's not easily reproduced.
As far as I can remember, the problem involved the socket still having
unsent data in it, then being closed, the unsent data already in the kernel
buffers of the socket then gets dropped by the kernel instead of still
being sent.
The problem is a rare occurrence, and it was a pain to actually diagnose it.

>>> commit 362e25ea53b7569250da884daab0d7b7ae58941c
>>>     Differentiate cache keys for different hosts mapped to the default server

>Looks like the change in roxen.pike is unrelated. Disregarding it.

Probably true.  I forgot what it is for exactly, it most likely killed a
spurious error somewhere, I'll try running without it once more to watch
the effect.
-- 
Sincerely,
           Stephen R. van den Berg.
Every successful person has had failures but repeated failure is no
guarantee of eventual success.