Code

Opened 2 years ago

Closed 15 months ago

#17763 closed Bug (duplicate)

Problem with auth_permission name length

Reported by: stenius@… Owned by: anonymous
Component: contrib.auth Version: 1.0
Severity: Normal Keywords:
Cc: shai@… Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description

Django automatically generates a name that is longer than what the default field length can hold.

INSERT INTO "auth_permission" ("name", "content_type_id", "codename") VALUES (%s, %s, %s) (u'Can add study plan practice assessment tutorial question', 52, u'add_studyplanpracticeassessmenttutorialquestion')

This will fail with a error about it not being able to fit in a varchar(50)

                                     Table "public.auth_permission"
     Column      |          Type          |                          Modifiers                           
-----------------+------------------------+--------------------------------------------------------------
 id              | integer                | not null default nextval('auth_permission_id_seq'::regclass)
 name            | character varying(50 ) | not null
 content_type_id | integer                | not null
 codename        | character varying(100) | not null

This was under PostgreSQL and after modifying the field to have a length of 200, I was able to perform the insert.

Attachments (0)

Change History (7)

comment:1 Changed 2 years ago by aaugustin

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

Django should check the length of the permission name before attempting to save it.

If it's too long, I see two possible solutions:

1 - raise an error better error message, telling to use a shorter model name

Indeed, even the simplest of views is hard to read with such long model names:

def my_view(request):
    questions = StudyPlanPracticeAssessmentTutorialQuestion.objects.filter(tutorial=...)

Python encourages concise and readable code, I'm fine with not supporting "app + model" names above 50 chars.

2 - only raise a warning and automatically truncate the name to 50 chars

But then there's still a problem when two models have the same prefix, for instance:

  • StudyPlanPracticeAssessmentTutorialMultipleChoicesQuestionAnswer
  • StudyPlanPracticeAssessmentTutorialMultipleChoicesQuestionAnswerEvaluationElement

Technically, we could write something like if len(name) > 50: name = name[:46] + hash(name[46:])[:4], but that won't help end users manipulating permissions to distinguish the two permissions.

I prefer solution 1.

Last edited 2 years ago by aaugustin (previous) (diff)

comment:2 Changed 2 years ago by elbarto

  • Owner changed from nobody to elbarto

comment:3 Changed 2 years ago by anonymous

  • Owner changed from elbarto to anonymous
  • Status changed from new to assigned

comment:4 Changed 2 years ago by ricw

The problem is with regards to the "name" field, not the codename field (as described above). The codename field will be populated by a smaller string than the name field, as it is the same length minus whitespace and descriptive characters (e.g. "Can add"/"Can modify"/..). So the setup should take this into consideration such that len(name)>len(codename). Currently the opposite is the case (excerpt from contrib.auth.models).

class Permission(models.Model):
    ..
    name = models.CharField(_('name'), max_length=50)
    ..
    codename = models.CharField(_('codename'), max_length=100)
    ..

Two options here:

  1. name length should be increased to at least 120 to accommodate for potential white space and additional info (e.g. "Can change"/"Can delete"/"Can add"..) that it stores when compared to codename.
  2. Cut the length down to 50 when initialising the model.

Currently it is failing for me because this model name is too long: "Can change material variant distributor region country"
This is only length 54. I wouldn't say that the length is excessively long. It should certainly be supported by django.

comment:5 Changed 2 years ago by shai

  • Cc shai@… added
  • Version changed from 1.3 to 1.0

To strengthen ricw's point, the name value is based on the model's verbose name, which is not necessarily derived from the class name.

Having a field whose name indicates verbosity limited to 39 characters is strongly counter-intuitive.

Also, ricw's option 1 has been wontfix'd in #4748; the problem has been with us since before 1.0.

My own suggestion of a fix: Change django.contrib.auth.management._get_all_permissions to read:

def _get_all_permissions(opts):
    "Returns (codename, name) for all permissions in the given opts."
    perms = []
    for action in ('add', 'change', 'delete'):
        perm_name = u'Can %s %s' % (action, opts.verbose_name_raw)
        if len(perm_name)>50:
            perm_name= perm_name[:40] + u'...' + perm_name[-7:]
        perms.append((_get_permission_codename(action, opts), perm_name))
    return perms + list(opts.permissions)

Because the name is just used for UI anyway. Of course, you can play with the "40" and "7" there -- that's what I thought was reasonable. And perhaps the two-line snippet for "make display string fit in length" should be factored out and used elsewhere.

If a core developer finds the approach acceptable, but would rather have this as a patch, just say so.

comment:6 Changed 22 months ago by gaker

I brought this ticket up on django-developers, and it was indicated that it's best to alter your columns manually & change max_length on the model fields until native migrations are in Django.

https://groups.google.com/forum/?fromgroups#!topic/django-developers/Q5yELp1UV_s

Last edited 22 months ago by gaker (previous) (diff)

comment:7 Changed 15 months ago by wilsoniya

  • Resolution set to duplicate
  • Status changed from assigned to closed

Closed as duplicate of #8162

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.