Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#14398 closed (fixed)

runfcgi umask option should use literal value

Reported by: petteyg Owned by: nobody
Component: Core (Management commands) Version: 1.2
Severity: Keywords:
Cc: anthony@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: yes Patch needs improvement: no
Easy pickings: UI/UX:

Description

The umask option is converted to an int, instead of taken as typed.

./manage.py runfcgi umask=0111 (decimal int) creates a socket with umask 0157 (octal). umask=0022 creates a socket with umask 0026. umask=0027 creares a socket with umask 0033.

A user on IRC suggested use of ast.literal_eval() instead.

Attachments (1)

t14398.diff (7.9 KB) - added by ramiro 5 years ago.
Patch for the issue

Download all attachments as: .zip

Change History (11)

comment:1 in reply to: ↑ description Changed 5 years ago by ramiro

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

Replying to petteyg:

A user on IRC suggested use of ast.literal_eval() instead.

The ast module was added to the stdlib in python 2.6 and Django (currently) needs to be compatible with Python 2.4.

Changed 5 years ago by ramiro

Patch for the issue

comment:2 Changed 5 years ago by ramiro

  • Has patch set
  • Needs tests set
  • Triage Stage changed from Unreviewed to Accepted

Patch for this issue using the radix parameter to int() taht allows to pass octal numbers in the usual 0nnn notation. Plus associated yak shaving: Intended to clarificate of the notations accepted for the umask runfcgi command option docs -> Addition of docs for the command -> Modification of our Sphinx customization to accept command options not prefixed with '--' so the :djadminopt: xrefs work as expected.

comment:3 follow-up: Changed 5 years ago by aptiko

You are using int(umask, 0). This means that the interpretation will not always be octal. int('755', 0) is interpreted as decimal. Wouldn't it be better to use int(umask, 8)?

comment:4 in reply to: ↑ 3 Changed 5 years ago by ramiro

Replying to aptiko:

You are using int(umask, 0). This means that the interpretation will not always be octal. int('755', 0) is interpreted as decimal. Wouldn't it be better to use int(umask, 8)?

The historical notation for octal numbers in C and Unix (and others) is prefixing them with a '0'. So IMHO we should respect that and interpret the argument as an octal value only if it has such prefix, that's why I'm using the radix=0 argument to int() (although this means we will also allow base 10 values and even hexadecimal '0xn' values)

I will ask other Django developers for opinions.

comment:5 Changed 5 years ago by aptiko

In C yes, but not in Unix generally. The chmod and umask commands accept octals, like chmod 755 or umask 222. Here we are talking about a command, ./manage.py ... umask=... As a Unix user, I would definitely expect to write an octal there. As a programmer, I would expect the leading zero convention only when writing a program.

comment:6 Changed 5 years ago by ramiro

You are completely right, I realize now I've been passing zero-prefixed arguments to chmod for years. Thanks.

comment:7 Changed 5 years ago by ramiro

  • Resolution set to fixed
  • Status changed from new to closed

(In [14360]) Fixed #14398 -- Changed runfcgi command to interpret the umask option argument as an octal value.
Thanks petteyg for report and aptiko for help with the fix.

comment:8 Changed 5 years ago by ramiro

(In [14361]) Documented options accepted by the runfcgi management command.
Also, modified our custom management command option docs xref parser
to also process those without a -- prefix. -- Refs #14398.

comment:9 Changed 5 years ago by ramiro

(In [14362]) [1.2.X] Fixed #14398 -- Changed runfcgi command to interpret the umask option argument as an octal value.
Thanks petteyg for report and aptiko for help with the fix.

Backport of [14360] from trunk

comment:10 Changed 5 years ago by ramiro

(In [14363]) [1.2.X] Documented options accepted by the runfcgi management command.
Also, modified our custom management command option docs xref parser
to also process those without a -- prefix. -- Refs #14398.

Backport of [14361] from trunk

Note: See TracTickets for help on using tickets.
Back to Top