Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#15783 closed Bug (invalid)

Class FileProxyMixin has invalid attribute definition

Reported by: Sebastian Żurek Owned by: nobody
Component: File uploads/storage Version: 1.3
Severity: Normal Keywords:
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX:


In: django/core/files/ line 12, in the definition of class class FileProxyMixin used in
form fileds to handle file uploads, there is:

fileno = property(lambda self: self.file.fileno)

which proxies to self.file.fileno... *but* self.file has no fileno attribute, which
produces an error on the access try...

Change History (5)

comment:1 Changed 6 years ago by Sebastian Żurek

I think I was not to verbose with the ticket, so some more info below:

The error occurred when I was handling uploded file, that has not been saved and is in fact InMemoryUploadedFile object. Then, InMemoryUploadedFile instance does not have fileno attribute working correctly.

Last edited 6 years ago by Sebastian Żurek (previous) (diff)

comment:2 Changed 6 years ago by Jonas H.

Easy pickings: unset
Resolution: invalid
Status: newclosed

From the Python documentation:

File-like objects which do not have a real file descriptor should *not* provide
this method!

That's exactly what Django does: Raising an AttributeError for file-likes that have no fileno.

comment:3 Changed 6 years ago by Sebastian Żurek

OK, I understand that... but the case is: FileProxyMixin defines fileno property, which proxies to self.file.fileno .. which does not exist.

The fact that self.file.fileno does not exist (when file is e.g. InMemomyUploadedFile instance) is of course valid with respect to Python docs, but.. when I'm using an object (an instance of FileProxyMixin subclass) that is reporting (i.e. with dir) that it has fileno attribute, shouldn't it have it?

I found the current FileProxyMixin definition unclear. IMHO it confuses programmer, who has to know, that despite the existence of the attribute, he can not use it.

One can of course say, that this is somehow similar case to the case of abstract method raising NonImplementedError being at the same time the existing attribute of the class, but while the Python docs states that the method should not be provided in fact it is (at least as a 'reference' to AttributeError).

This is just to comment what worries me here. :D Maybe FileProxyMixin should introspect self.file to see if it provides fileno before putting it as a proxy attr?

comment:4 Changed 6 years ago by Jonas H.

From the runtime's point of view (hasattr/getattr), it doesn't have a fileno attribute.

    >>> class Foo(object):
    ...   @property
    ...   def fileno(self):
    ...     raise AttributeError
    >>> hasattr(Foo(), 'fileno')

Last edited 6 years ago by Jonas H. (previous) (diff)

comment:5 Changed 6 years ago by Sebastian Żurek

OKay - now I see it! This comment helps me a little. You're right the when You're looking on this with hasattr, there's no 'fileno', and everything is perfectly okay.

My 'fault' was, that I was looking on this firstly with dir, which simply listed 'fileno' as attr and than I was trying to use it. I was also looking into the code, where I also saw 'fileno' implemented.

So I thing it should be left as it is... but there's still a tiny bit of hesitation in me when I'm thinking abut it...

(fileno behaviour is like taken from the quantum mechanics: You know that it's not there only if You really try to touch it.) :D

Anyway - thanks for the discussion!

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