#325 closed defect (fixed)
Glitch in web interface handler
| Reported by: | skaplun | Owned by: | jcaffaro |
|---|---|---|---|
| Priority: | major | Milestone: | |
| Component: | WebSearch | Version: | |
| Keywords: | opensearch | Cc: |
Description
[...]
The following problem occurred on <http://inspirebeta.net>; (CDS Invenio 0.99.90.20100823)
2010-10-26 07:31:34 -> AttributeError: 'function' object has no attribute '_traverse'
User details
uri: </opensearchdescription/?>
Traceback details
Traceback (most recent call last):
File "/usr/lib/python2.4/site-packages/invenio/webinterface_handler_wsgi.py", line 367, in application
ret = invenio_handler(req)
File "/usr/lib/python2.4/site-packages/invenio/webinterface_handler.py", line 331, in _profiler
return _handler(req)
File "/usr/lib/python2.4/site-packages/invenio/webinterface_handler.py", line 373, in _handler
return root._traverse(req, path, False, guest_p)
File "/usr/lib/python2.4/site-packages/invenio/webinterface_handler.py", line 223, in _traverse
return obj._traverse(req, path, do_head, guest_p)
AttributeError: 'function' object has no attribute '_traverse'
Locals by frame, innermost last
[...]
Attachments (2)
Change History (13)
comment:1 Changed 3 years ago by jcaffaro
- Priority changed from trivial to major
comment:2 Changed 3 years ago by skaplun
Ups! Definitively. This looks like a regression with WSGI then, because with mod_python this paths where handled correctly.
comment:3 Changed 3 years ago by jcaffaro
I wanted to test something of that kind:
diff --git a/modules/webstyle/lib/webinterface_handler.py b/modules/webstyle/lib/webinterface_handler.py
index c396e65..3a76f64 100644
--- a/modules/webstyle/lib/webinterface_handler.py
+++ b/modules/webstyle/lib/webinterface_handler.py
@@ -220,7 +220,11 @@ class WebInterfaceDirectory(object):
# resolving, otherwise call the method as it is our final
# renderer. We even pass it the parsed form arguments.
if path:
- return obj._traverse(req, path, do_head, guest_p)
+ if hasattr(obj, '_traverse'):
+ return obj._traverse(req, path, do_head, guest_p)
+ else:
+ raise apache.SERVER_RETURN, apache.HTTP_NOT_FOUND
but it looks more like a new behavior than a regression fix, so if you think that it got broken by the migration to WSGI, we might have to check somewhere else.
comment:4 Changed 3 years ago by jcaffaro
These URLs were already not properly handled before the migration to WSGI.
For eg (0.99.2): https://invenio-demo.cern.ch/youraccount/display/
comment:5 Changed 3 years ago by jcaffaro
- Status changed from new to in_merge
I have attached two patches, one for master and one for maint (0.99.2) branches.
(Note that http://invenio-demo.cern.ch has now been patched...)
comment:6 Changed 3 years ago by skaplun
I just wonder, with your patch you return a 404. Would indeed implicitly redirecting /foo/ to /foo (when _traverse does not exist) be the wrong thing to do? I guess it might be a problem for OAI, but are there other cases?
comment:7 Changed 3 years ago by jcaffaro
I also wondered about redirecting /foo/ to /foo, but I thought it was conceptually better to keep the distinction between pages (function) and directories (WebInterfaceDirectory). Indeed redirecting /foo/ to /foo would be similar to having a directory with an implicit "index" page inside (in these case "index" would be the only possible page).
In the case of a WebInterfaceDirectory, one might expect that /foo/index would be the same as /foo/ or /foo. That would not be the case with my patch, even with the redirection from /foo/ to /foo: we would have to also implement the redirection /foo/index to /foo (although we have not always been consistent regarding this across all modules...). I am not really foo about that.
Looking at the above list of URLs, they should be more like "leaf" pages than directories to me. Does one expect that /foo.html/ is redirected to /foo.html, or return a 404?
In the end, one or the other solution does not seen to change too much for the end-user, nor the developer. If we change our mind later, it will be easier to add the redirection, than removing it...
But I am sure there are plenty of reasons to choose one or the other way :-)
It is indeed the right moment to wonder if one of the solutions would have serious, annoying consequences for further developments...
comment:8 Changed 3 years ago by jcaffaro
- Summary changed from Glitch in opensearch web handler to Glitch in web interface handler
comment:9 Changed 3 years ago by simko
Note that /search/ works well and serves the same content as /search. Maybe we should do the same for others like /yourbaskets/add/. Though this introduces non-canonicality, which is not good.
In the case of /somefacility/someaction, I agree that it makes sense to return 404 for the trailing slash. In the case of /somefacility only, it may be better to redirect, because people are generally used to that, e.g. compare http://directory.google.com/Top/Computers/Software/ and http://directory.google.com/Top/Computers/Software. In other words, for our URLs /somefacility/someaction, like /yourbaskets/add or /yourbaskets/display, the first one looks like a directory and the second one looks like a page, and I guess I'd propose to interpret trailing slashes for all-but-last components of the URL, and return 404 for the last one.
Seems similar to Jerome's take on the matter.
comment:10 Changed 3 years ago by Jerome Caffaro <jerome.caffaro@…>
- Resolution set to fixed
- Status changed from in_merge to closed
Same would occur for URLs:
Should probably be fixed at the level of the URL handler.