roxen.lists.roxen.general

Subject Author Date
Re: Lots of patches that can be integrated into Roxen 5.0 in a heartbeat Martin Stjernholm <mast[at]roxen[dot]com> 10-01-2009
Continuing:

>> 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.

As for this bit:

@@ -2878,7 +2880,15 @@ object clone_me()
   c->time = time;
   c->raw_url = raw_url;
 
-  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;
   c->variables = FakedVariables( c->real_variables );
   c->misc = copy_value( misc );
   c->misc->orig = t;

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).

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

Btw, a side note on:

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

I've seen this way of adding elements to mappings curiously often.
Pike can - under favorable conditions - optimize it to a destructive
operation on the first mapping so the whole thing isn't copied. But
sometimes it might not do that, and I'm also not sure if it can
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. :)

>> 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.

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

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

The pragma["no-cache"] test added in

-    if(misc->cacheable && !misc->no_proto_cache &&
-       (cv = conf->datacache->get(raw_url, this_object())) )
+    if(misc->cacheable && !misc->no_proto_cache && !pragma["no-cache"] &&
+       (cv = conf->datacache->get(cache_key, this_object())) )

is also unrelated afaics. Please motivate it separately.

As for the rest of it, I implemented it another way which should work
better with the config resolution procedure. Please test.

The rest still on the to-do..