Re: contrib.admin:list_editable - ForeignKey Performance is O(m*n)?

[Jeremy Dunck]

On Thu, Jul 1, 2010 at 12:40 AM, chadc  wrote:
...
> The jist of what I am wondering is if the changelist is going to allow
> editable ForeignKeys, should it be caching the choices rather than
> hitting the database every time?

Yep, your models and explanation are clear.  I'm not sure that
CachedSelect is a good solution; it seems like there's a general
problem with repeated queries within ModelFormSet, not just admin--
it's just cropping up here.  I'll try to look at it by tomorrow.

File a ticket and let me know the number?

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-develop...@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.

Re: What is the correct behavior? [was: new backend: unit test ...]

[Russell Keith-Magee]

On Thu, Jul 1, 2010 at 2:19 AM, Mark Bucciarelli  wrote:
> On Wed, Jun 30, 2010 at 7:56 AM, Mark Bucciarelli  wrote:
>> On Wed, Jun 30, 2010 at 7:14 AM, Mark Bucciarelli  wrote:
>>>
>>> Is this test look correct?
>>>
>>
>> It passes when using SQLite ...
>>
>
> It fails with a postgresql_pschopg2 backend.
>
> So what is the correct Django backend behavior?
>
> Here's the relevant part of test script I'm using.
> PostgreSQL fails on the get() call near the end.
>
>        names = ('a', 'b')
>        for name in names:
>                try:
>                        s = Source.objects.get(name=name)
>                        s.delete()
>                except Source.DoesNotExist:
>                        pass
>                s = Source(name=name, feedurl=name)
>                s.save()
>        try:
>                s = Source(name=names[0], feedurl=names[0])
>                s.save()
>        except IntegrityError:
>                pass
>        s = Source.objects.get(name=names[1])
>        print s
>
> The error is very similar to MonetDB:
>
>        psycopg2.InternalError: current transaction is
>        aborted, commands ignored until end of transaction
>        block

What is happening here is the surfacing of a difference between the
way PostgreSQL handles exceptions and the way SQLite handles
exceptions. As is to be expected, SQLite is a simple database, so it's
transaction control isn't as robust as a production-ready database.

When you generate an error under PostgreSQL, the cursor is put into an
error state, and in the interests of protecting you, it prevents you
from doing anything else with that cursor until the problem is
resolved. This means you have to roll back the cursor to the last
commit.

Strictly, the same problem exists with SQLite (i.e., you've
experienced an error that needs to be handled); but SQLite doesn't
force you to roll back the transaction and clarify that the problem
has been resolved.

It appears that MonetDB is behaving the same way as PostgreSQL. The
right approach in the test is to catch the exception and roll back the
cursor.

Yours,
Russ Magee %-)

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-develop...@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.

Re: Feedback for #13849 (csrf referer checking)

[Paul McLanahan]

On Wed, Jun 30, 2010 at 12:16 PM, Luke Plant  wrote:
> With Django's sessions and login method, you *do* have a session on the
> login page itself i.e. before the actual login step.  So you need to
> worry about session fixation etc. Even if not, and you are using your
> own login method, there is still "login CSRF" to worry about [1].
>
> Without the check for a HTTPS referer, you are wide open to a MITM
> attacker doing CSRF on your HTTPS connections.  The check isn't "erring
> on the side of caution" — that was what I *previously* thought it was,
> having not thought through the possible attacks, but in fact it is
> *absolutely essential*.  Without it, you should consider your site as
> having the same level of protection as an HTTP site.
>
> Yes, there are still some benefits to HTTPS (passwords not sent in
> plaintext), but you certainly don't have the kind of protection that
> would be expected in HTTPS, and in the general case you might find that
> all benefits are destroyed by what a successful attacker might be able
> to achieve (setting passwords etc.) unless you are extremely careful and
> have closed every possible loophole.  For these reasons, changing this
> behaviour — or even allowing it as an option — would be craziness for a
> general purpose framework.  An option is a bad idea in general, because
> it is global — and we certainly do *not* want this hole punched in the
> admin site.
>
> If you can really live with the security holes you want to make, that is
> absolutely up to you.  You can write your own middleware, and ensure its
> suitability for your needs.  But I cannot see an argument for supporting
> this as an option out of the box.

Fair enough. If the situation is potentially really this dire, as
you've convinced me that it is, then leaving it alone is the way to
go.

Thanks again for your time.

Paul

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-develop...@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.

test-refactor update

[Paul McMillan]

I've been ill, hence the late update and lack of progress. Last week I
only committed 2 test updates, but spent significant time tracking
down a fix for #13821 since it was preventing existing tests from
running on Postgres 8.3.

-Paul

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-develop...@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.

[solved] What is the correct behavior? [was: new backend: unit test ...]

[Mark Bucciarelli]

On Wed, Jun 30, 2010 at 2:19 PM, Mark Bucciarelli  wrote:
>
> So what is the correct Django backend behavior?
>

I'm going with backend-dependent for
pre-1.2 Django.

In 1.2 full_clean() + ValidationError is
to the rescue.

Thanks,

m

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-develop...@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.

What is the correct behavior? [was: new backend: unit test ...]

[Mark Bucciarelli]

On Wed, Jun 30, 2010 at 7:56 AM, Mark Bucciarelli  wrote:
> On Wed, Jun 30, 2010 at 7:14 AM, Mark Bucciarelli  wrote:
>>
>> Is this test look correct?
>>
>
> It passes when using SQLite ...
>

It fails with a postgresql_pschopg2 backend.

So what is the correct Django backend behavior?

Here's the relevant part of test script I'm using.
PostgreSQL fails on the get() call near the end.

names = ('a', 'b')
for name in names:
try:
s = Source.objects.get(name=name)
s.delete()
except Source.DoesNotExist:
pass
s = Source(name=name, feedurl=name)
s.save()
try:
s = Source(name=names[0], feedurl=names[0])
s.save()
except IntegrityError:
pass
s = Source.objects.get(name=names[1])
print s

The error is very similar to MonetDB:

psycopg2.InternalError: current transaction is
aborted, commands ignored until end of transaction
block

Thanks,

Mark

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-develop...@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.

Re: Feedback for #13849 (csrf referer checking)

[Luke Plant]

On Wed, 2010-06-30 at 11:08 -0400, Paul McLanahan wrote:

> I agree that not allowing them by default is good, but I think that
> having the option to allow them is also good. I like the default
> behavior as it is, as it's inline with our "secure by default"
> philosophy. But in our situation, I'm just not as concerned with this
> because our only other alternative is to turn off CSRF checking for
> these views, which I think is an even worse idea.
> 
> I will say that I'm not sure our situation even benefits from CSRF
> protection. We're doing this on a login page (so they shouldn't have a
> session at all yet), and on other pages on which we ask users to
> submit their password (password change, username change, etc.). The
> later all result in confirmation emails, which should prevent fraud.
> We're still hesitant to remove CSRF protection from these views though
> just in case we've done something else that may open a hole that the
> CSRF mechanism will close. Like you said, we're trying to "err on the
> side of caution."

With Django's sessions and login method, you *do* have a session on the
login page itself i.e. before the actual login step.  So you need to
worry about session fixation etc. Even if not, and you are using your
own login method, there is still "login CSRF" to worry about [1]. 

Without the check for a HTTPS referer, you are wide open to a MITM
attacker doing CSRF on your HTTPS connections.  The check isn't "erring
on the side of caution" — that was what I *previously* thought it was,
having not thought through the possible attacks, but in fact it is
*absolutely essential*.  Without it, you should consider your site as
having the same level of protection as an HTTP site.

Yes, there are still some benefits to HTTPS (passwords not sent in
plaintext), but you certainly don't have the kind of protection that
would be expected in HTTPS, and in the general case you might find that
all benefits are destroyed by what a successful attacker might be able
to achieve (setting passwords etc.) unless you are extremely careful and
have closed every possible loophole.  For these reasons, changing this
behaviour — or even allowing it as an option — would be craziness for a
general purpose framework.  An option is a bad idea in general, because
it is global — and we certainly do *not* want this hole punched in the
admin site.

If you can really live with the security holes you want to make, that is
absolutely up to you.  You can write your own middleware, and ensure its
suitability for your needs.  But I cannot see an argument for supporting
this as an option out of the box.

Regards,

Luke

[1] http://www.adambarth.com/papers/2008/barth-jackson-mitchell-b.pdf

-- 
"The only skills I have the patience to learn are those that have 
no real application in life." (Calvin and Hobbes)

Luke Plant || http://lukeplant.me.uk/

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-develop...@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.

Re: Feedback for #13849 (csrf referer checking)

[Paul McLanahan]

Hi Luke,

Thanks for the reply. See my comments below.

On Wed, Jun 30, 2010 at 7:45 AM, Luke Plant  wrote:
> In addition, having looked at the links that Tino sent, I now think
> there is a very big advantage to not allowing HTTP -> HTTPS POST
> requests by default, even ignoring the CSRF issue.

I agree that not allowing them by default is good, but I think that
having the option to allow them is also good. I like the default
behavior as it is, as it's inline with our "secure by default"
philosophy. But in our situation, I'm just not as concerned with this
because our only other alternative is to turn off CSRF checking for
these views, which I think is an even worse idea.

I will say that I'm not sure our situation even benefits from CSRF
protection. We're doing this on a login page (so they shouldn't have a
session at all yet), and on other pages on which we ask users to
submit their password (password change, username change, etc.). The
later all result in confirmation emails, which should prevent fraud.
We're still hesitant to remove CSRF protection from these views though
just in case we've done something else that may open a hole that the
CSRF mechanism will close. Like you said, we're trying to "err on the
side of caution."

> BTW, if you are using Django's sessions over HTTPS, then you need to
> have SESSION_COOKIE_SECURE = True, otherwise you are wide open to an
> HTTP-snooper hijacking HTTPS sessions.  However, with
> SESSION_COOKIE_SECURE = True, sessions will not work over HTTP.  This
> means you need to serve your whole site over HTTPS.  I only just
> realised this for one of my own sites.  We probably need to document
> that more clearly, but I'm not sure how.

I can appreciate this as well, but we definitely don't need for the
whole site to be secure. No personal or sensitive data is transmitted
beyond the user's email address and password. So we'd really like to
only secure the transmission of that info. If an attacker were to
hijack one of our user's sessions, secure or otherwise, they'd just be
able to use our site w/o paying for a few hours. If they tried to
change the username or password, a confirmation email system would
prevent the change. As with most web security techniques, we're just
trying to be as secure as we can be without being overly inconvenient
to our users, while not being low-hanging fruit for would-be
attackers.

If you're all vehemently opposed to allowing the possibility of using
the CSRF middleware and submitting over SSL from a non-SSL page, then
we'll be fine. I just still think that this is a valid thing for a web
app to want to do, and thus would be a good option for a user of the
CSRF system to have.

Thanks again,

Paul

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-develop...@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.

Re: contrib.admin:list_editable - ForeignKey Performance is O(m*n)?

[chadc]

Oh, yah, sure. Sorry.

class Host(models.Model):
name = models.CharField(max_length=128, unique=True)

class Account(models.Model):
host = models.ForeignKey(Host, related_name="accounts")
name = models.CharField(max_length=128)

class AccountAdmin(admin.ModelAdmin):
list_display = ('name', 'host')
list_editable = ('host',)
list_per_page = 25
admin.site.register(Account, AccountAdmin)

Rendering the FK widgets here requires n*m=25*1=25 database queries:

Form: SELECT "hosts_host"."id",  "hosts_host"."name" FROM "hosts_host"
ORDER BY "hosts_host"."name" ASC
Total time: 330 ms
Numer of queries: 25

Twenty five queries is, of course, not a big deal. However, the
default changelist is n=100, and with m=4 this becomes a real problem.

If you comment out the list_editable line or use the fix below
( formfield_overrides = {models.ForeignKey:{'widget':
CachedSelect()}} ), the number of queries returns to O(1).

The jist of what I am wondering is if the changelist is going to allow
editable ForeignKeys, should it be caching the choices rather than
hitting the database every time?



On Jun 29, 7:14 pm, Jeremy Dunck  wrote:
> On Wed, Jun 30, 2010 at 2:04 AM,chadc wrote:
>
> ...
>
> > 3. Overriding the ForeignKey widget with a custom CachedSelect widget.
>
> ...
>
> > PS: For the sake of posterity, I have included the code for my
> > CachedSelect widget. Yes, I am aware that using regular expressions to
> > parse the key is a nasty hack. If you have any better ideas, please
> > let me know!
>
> For clarity of what's going on here, would you mind posting an example model?

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-develop...@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.

Re: new backend: unit test for "autocommit-like" behavior

[Mark Bucciarelli]

On Wed, Jun 30, 2010 at 7:14 AM, Mark Bucciarelli  wrote:
>
> Is this test look correct?
>

It passes when using SQLite backend, so I
think the test is good.

>
> Currently it fails on the get() with the error
>
>      current transaction is aborted
>      (please ROLLBACK)
>

Any suggestions for what code is missing
from the MonetDB driver?

Thanks,

m

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-develop...@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.

new backend: unit test for "autocommit-like" behavior

[Mark Bucciarelli]

Hi,

I've been working on a Django backend for MonetDB.
I'm trying to write a unit test that verifieds I curently
have a bug in how transactions are handled.

I have a couple questions about the unit test.

Here, Simple is a model with a single field "name"
that has a unique index.

s = Simple(name='mark')
s.save()
s = Simple(name='mark')
try:
s.save()
except OperationalError:
pass

# first one should be saved.
s = Simple.objects.get(name='mark')
self.assertTrue(s is not None)

Is this test look correct?

Currently it fails on the get() with the error

  current transaction is aborted
  (please ROLLBACK)

I should not need to call rollbak call in the
test's except clause, right?

Thanks,

Mark

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-develop...@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.

Re: contrib.admin:list_editable - ForeignKey Performance is O(m*n)?

[Patryk Zawadzki]

On Tue, Jun 29, 2010 at 6:04 PM, chadc  wrote:
> Hi there,
>
> I have been experiencing poor performance with django-admin when
> list_editable includes ForeignKey fields. In particular, rendering the
> changelist requires O(m*n) database queries where m is the number of
> ForeignKey fields included in list_editable and n is the number of
> rows in the changelist. I have searched extensively for possible
> causes and, after finding nothing and receiving no response on either
> django-users ('list_editable duplicate queries') or IRC, I am starting
> to think that it is a legitimate bug.

While this is worth to cache, you need to index the cache using a
serialized query, not the model.

It is possible to have a different set of choices for each model
instance (for example applying extra filter using another field's
value).

-- 
Patryk Zawadzki

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-develop...@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.

Re: Class based generic views in 1.3?

[Waldemar Kornewald]

On Thu, Jun 17, 2010 at 10:45 PM, Jacob Kaplan-Moss  wrote:
> On Thu, Jun 17, 2010 at 3:41 PM, Alex Gaynor  wrote:
>> So here's an idea, of dubious quality ;)
>
> Okay, folks: at this point the discussion has pretty much descended
> into bikeshedding territory. I'm going to read through everything
> posted so far and try to post a summary and round-up to help us get
> refocused; gimme a few hours to pull that together and then let's try
> to reach towards a consensus.

Any results?

Bye,
Waldemar Kornewald

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-develop...@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.