[Zope-dev] funky side-effects, possible bug in HTTPRequest.py

2003-06-20 Thread Jamie Heilman
The last few days I've been working on patch to Zope 2 that will clean
up the textarea size preference handling in the ZMI.  Right now, its a
mess.  I kept running into this really irritating behavior such that,
when ever I'd go to pull something out of a request object, it'd end
up being found in the 'other' dictionary instead of where I expected
to be (which happened to be 'form').  After getting really curious as
to why this was happening I've managed to track it down to subtle a
change in the way that the HTTPRequest.get method worked between
revisions 1.75 and 1.74 (1.75 being mj's merge of the value tainting
code).  The code responsible (assuming the value isn't tainted):

# Untrusted data *after* trusted data
v = self.form.get(key, _marker)
if v is not _marker:
other[key] = v  # *boom*
return v

That magical promotion of the key & value to the other dictionary is
what tripped me up.  This technique, while originally used only for
known convenience variables like URLx or BASEx and for promoting lazy
data, now applies to all variables after revision 1.75.  (This isn't
the only spot it gets used now, the tainting changes made it affect
form values, cookies, tainted form values, etc.)  It would increase the
speed at which variables are found--but I'm not sure its really an
intended side effect, and now I'll explain how I was bitten by it.

Its not unusual to see people write methods to be published that are
similar too:

 manage_main = DTMLFile("edit", globals())
 def manage_changeFoo(self, REQUEST, something=None, or_other=628):
# stuff
return self.manage_main(self, REQUEST)

They're a dime a dozen, yay neat.  What sometimes makes them more
interesting is when people modify the REQUEST dictionaries to pull
off various behind the scenes fake-like-we-requested-it-that-way
stuff.  This is what several of the methods that handled the Taller,
Wider, Narrower, etc. buttons on text area prefs did, and thats why I
ran into it.  One of these methods did:
REQUEST.form.update({'something':'x', 'or_other':'y'})
and then it returned a DTMLFile plied with the snazzy new request that
had been tricked out with these fake values now stuck into the form
dictionary.  This explains the TALES I saw that was
"request/form/something | request/something | default" which when I
first saw made wonder if the author was on acid.  It was clear to me
that 'something' would never be in other, it wasn't a special variable
and it wasn't being explicitly stuck into the other dictionary by the
code, so why the redundant TALES I wondered?  I removed the first
statement, and it broke the functionality.  Hmmm!

Until about 20 minutes ago I didn't understand exactly how object
methods got published, but I tracked down the code in Publish.py and
found the mapply() function and now its all clear to me what was
happening.  The request object gets mapply'd to the published method,
this means that any positional arguments or any keyword arguments will
get HTTPRequest.get'd out of the request object when they are applied
to the published method, and thanks to the side-effects from revision
1.75 on, it also means any named variables in a method definition will
be promoted into the HTTPRequest.other dictionary.  Now, let me just say
- if thats how its supposed to be, so be it - but spin my nipple nuts
and call me Frank, this does NOT strike me as terribly intuitive
behavior.

Anyway, it was a learning experience for me, but I'm not convinced
this isn't a bug.  What do you think?

(the patches I spoke of should be ready sometime tomorrowish assuming
I don't run into any other funkyness, I'll post them to the collector)

-- 
Jamie Heilman   http://audible.transient.net/~jamie/
"We must be born with an intuition of mortality.  Before we know the words
 for it, before we know there are words, out we come bloodied and squalling
 with the knowledge that for all the compasses in the world, there's only
 one direction, and time is its only measure."  -Rosencrantz

___
Zope-Dev maillist  -  [EMAIL PROTECTED]
http://mail.zope.org/mailman/listinfo/zope-dev
**  No cross posts or HTML encoding!  **
(Related lists - 
 http://mail.zope.org/mailman/listinfo/zope-announce
 http://mail.zope.org/mailman/listinfo/zope )


Re: [Zope-dev] funky side-effects, possible bug in HTTPRequest.py

2003-06-20 Thread Paul Winkler
On Fri, Jun 20, 2003 at 03:07:18AM -0700, Jamie Heilman wrote:
> Anyway, it was a learning experience for me, but I'm not convinced
> this isn't a bug.  What do you think?

>From the POV of a poor web monkey, my opinion is that this is 
purely awful. It's the kind of thing that can easily waste hours of
debug time.

I was just saying to Andy McKay yesterday that sometimes I think
zope 2 is designed around the principle of greatest consternation
rather than the principle of least suprise. Case in point.

-- 

Paul Winkler
http://www.slinkp.com
Look! Up in the sky! It's ANTHROPOLOGIST MAN!
(random hero from isometric.spaceninja.com)

___
Zope-Dev maillist  -  [EMAIL PROTECTED]
http://mail.zope.org/mailman/listinfo/zope-dev
**  No cross posts or HTML encoding!  **
(Related lists - 
 http://mail.zope.org/mailman/listinfo/zope-announce
 http://mail.zope.org/mailman/listinfo/zope )


Re: [Zope-dev] funky side-effects, possible bug in HTTPRequest.py

2003-06-20 Thread Oliver Bleutgen
Jamie Heilman wrote:
[major snippage]
Hmmm, that means that this changes break exactly these applications, 
which, in order to be on the secure side, explicitly use 
REQUEST.form['bla'] more than once in a request, right.

Ironic.

cheers,
oliver
___
Zope-Dev maillist  -  [EMAIL PROTECTED]
http://mail.zope.org/mailman/listinfo/zope-dev
**  No cross posts or HTML encoding!  **
(Related lists - 
http://mail.zope.org/mailman/listinfo/zope-announce
http://mail.zope.org/mailman/listinfo/zope )


Re: [Zope-dev] funky side-effects, possible bug in HTTPRequest.py

2003-06-20 Thread Dieter Maurer
Jamie Heilman wrote at 2003-6-20 03:07 -0700:
 > ...
 > That magical promotion of the key & value to the other dictionary is
 > what tripped me up.  This technique, while originally used only for
 > known convenience variables like URLx or BASEx and for promoting lazy
 > data, now applies to all variables after revision 1.75.

Zope promotes the variables from "cookies" and "form" to "other"
at least since Zope 2.1.6 (the first version I worked with).

I think this is for efficiency reasons. You have a single
dictionary lookup instead of three ("other", "form", "cookie").


Dieter

___
Zope-Dev maillist  -  [EMAIL PROTECTED]
http://mail.zope.org/mailman/listinfo/zope-dev
**  No cross posts or HTML encoding!  **
(Related lists - 
 http://mail.zope.org/mailman/listinfo/zope-announce
 http://mail.zope.org/mailman/listinfo/zope )


Re: [Zope-dev] funky side-effects, possible bug in HTTPRequest.py

2003-06-20 Thread Jamie Heilman
Oliver Bleutgen wrote:
> Jamie Heilman wrote:
> >[major snippage]
> 
> Hmmm, that means that this changes break exactly these applications, 
> which, in order to be on the secure side, explicitly use 
> REQUEST.form['bla'] more than once in a request, right.

Naw, it doesn't break them, promotion doesn't mean the vars are
actually moved from the form dictionary to the other dictionary, only
copied.  What it does is humble the poor shmuck who thought he knew
the dangers of REQUEST.get well enough to use it without shooting
himself in the foot.

-- 
Jamie Heilman   http://audible.transient.net/~jamie/
"You came all this way, without saying squat, and now you're trying
 to tell me a '56 Chevy can beat a '47 Buick in a dead quarter mile?
 I liked you better when you weren't saying squat kid." -Buddy

___
Zope-Dev maillist  -  [EMAIL PROTECTED]
http://mail.zope.org/mailman/listinfo/zope-dev
**  No cross posts or HTML encoding!  **
(Related lists - 
 http://mail.zope.org/mailman/listinfo/zope-announce
 http://mail.zope.org/mailman/listinfo/zope )


Re: [Zope-dev] funky side-effects, possible bug in HTTPRequest.py

2003-06-20 Thread Jamie Heilman
Dieter Maurer wrote:
> Zope promotes the variables from "cookies" and "form" to "other"
> at least since Zope 2.1.6 (the first version I worked with).
> 
> I think this is for efficiency reasons. You have a single
> dictionary lookup instead of three ("other", "form", "cookie").

Yeah most promotion is for efficiency, but since 2.1.6 eh?
Interesting, that means its happening elsewhere too.  Hmm.  Yeah, I
see, it happens in __init__ for cookies, mmm and in processInputs for
form variables I guess.  The difference I can see is two fold, first,
before 1.75 I believe promotion of form and cookies was only done
during request object creation, interestingly, its not done there
anymore from the looks of it; cookies and form vars are kept in their
seperate tainted & non-tainted buckets until they are fetched.  Second
the promotion of cookies wouldn't clobber pre-existing keys in
other, interestingly, in 2.1.6 times, form vars would clober normal
other vars (URLx, BASEx, etc.).

I must admit, this isn't the biggest deal to me, I just blew quite a
bit of time tracking it down because I couldn't understand why the
promotion was happening and I don't like surprises, and nothing about
"def foo(self, bar, baz):" immediately jumped out and said to me,
"promote bar and baz form vars and cookies to the other dictionary."
Now that I have an appreciation for exactly how methods are published,
I see things in a different light of course.

-- 
Jamie Heilman   http://audible.transient.net/~jamie/
"...thats the metaphorical equivalent of flopping your wedding tackle 
 into a lion's mouth and flicking his lovespuds with a wet towel, pure 
 insanity..."   -Rimmer

___
Zope-Dev maillist  -  [EMAIL PROTECTED]
http://mail.zope.org/mailman/listinfo/zope-dev
**  No cross posts or HTML encoding!  **
(Related lists - 
 http://mail.zope.org/mailman/listinfo/zope-announce
 http://mail.zope.org/mailman/listinfo/zope )


Re: [Zope-dev] funky side-effects, possible bug in HTTPRequest.py

2003-06-23 Thread Florent Guillaume
In article <[EMAIL PROTECTED]> you write:
> 
> # Untrusted data *after* trusted data
> v = self.form.get(key, _marker)
> if v is not _marker:
> other[key] = v  # *boom*
> return v
> 
> That magical promotion of the key & value to the other dictionary is
> what tripped me up.

Wouldn't
  other.setdefault(key, v)
be better? 
So a variable already existing in other wouldn't get clobbered.

Florent

-- 
Florent Guillaume, Nuxeo (Paris, France)
+33 1 40 33 79 87  http://nuxeo.com  mailto:[EMAIL PROTECTED]

___
Zope-Dev maillist  -  [EMAIL PROTECTED]
http://mail.zope.org/mailman/listinfo/zope-dev
**  No cross posts or HTML encoding!  **
(Related lists - 
 http://mail.zope.org/mailman/listinfo/zope-announce
 http://mail.zope.org/mailman/listinfo/zope )