twisted / twisted

Event-driven networking engine written in Python.
https://twisted.org
Other
5.56k stars 1.17k forks source link

twisted.web.resource.Resource.allowedMethods doesn't determine what methods are allowed #4958

Open twisted-trac opened 13 years ago

twisted-trac commented 13 years ago
washort's avatar @washort reported
Trac ID trac#4958
Type defect
Created 2011-03-15 19:20:44Z

Resource.render only consults allowedMethods after looking for a render_ method. If a render_FOO method exists, it's invoked; allowedMethods is only used to populate the Allow header in the response.

To quote wsanchez from #3684:

OK, so with this API, if you have `allowedMethods = ["GET"]`, then even though you are telling us that you don't want to allow HEAD (for whatever reason), we're going to ignore you and allow it anyway.

This strikes me as strangely inconsistent/exceptional API, so that bothers me. What I would prefer is that if you say allowedMethods = ["GET", "HEAD"], but you don't implement the HEAD method, then we'll use your GET implementation to provide HEAD.

That is, we should do what you ask us to do, and not pretend that we know better.

Searchable metadata ``` trac-id__4958 4958 type__defect defect reporter__washort washort priority__normal normal milestone__ branch__ branch_author__ status__new new resolution__None None component__web web keywords__ time__1300216844000000 1300216844000000 changetime__1300493988000000 1300493988000000 version__None None owner__ cc__jknight ```
twisted-trac commented 13 years ago
jyknight's avatar @jyknight commented

This doesn't seem like a bug, but rather an unfortunate attribute, and documentation that needs clarification.

The allowedMethods attribute's purpose is not to decide what methods are allowed -- render can do that by itself, I don't see why an additional filter for that is desirable.

twisted-trac commented 13 years ago
exarkun's avatar @exarkun commented

This doesn't seem like a bug, but rather an unfortunate attribute, and documentation that needs clarification.

Then maybe allowedMethods should simply not exist at all? The methods that are implemented can be discovered automatically.

twisted-trac commented 13 years ago
jyknight's avatar @jyknight commented

Assuming you don't do anything crazy like have a getattr hook, yea. In web2.resource it was a method instead of an attribute, which seemed a good idea so that just in case anyone had a reason to override it, it was easily done.