cms-dev / cms

Contest Management System
http://cms-dev.github.io/
GNU Affero General Public License v3.0
900 stars 362 forks source link

"url" is None during error template rendering #1069

Open andreyv opened 6 years ago

andreyv commented 6 years ago

Steps to reproduce:

  1. Assume CWS is running in multi-contest mode and listening on localhost:8888
  2. Run telnet localhost 8888
  3. Request a non-existing contest page:
    
    GET /zzz HTTP/1.1
    Host: localhost
4. Actual output:

HTTP/1.1 404 Not Found Server: TornadoServer/4.5.3 Content-Type: text/html; charset=UTF-8 Date: Tue, 06 Nov 2018 18:25:50 GMT Content-Length: 224 Set-Cookie: zzz_login=""; expires=Mon, 06 Nov 2017 18:25:50 GMT; Path=/ Set-Cookie: _xsrf=2|47f2b7e0|d92adbae5745d453d5769a29a88df401|1541528750; Path=/

<!DOCTYPE html>

Error 404***OUTPUT STOPS HERE*** ``` 5. After that the server is still listening and responds to further requests on the same connection. Environment: Ubuntu 18.04 with Python dependencies installed from the system; CMS is built from master and runs with Python 3. We can also see that the server tries to set cookies for the non-existent contest. </div> </div> <div class="comment"> <div class="user"> <a rel="noreferrer nofollow" target="_blank" href="https://github.com/andreyv"><img src="https://avatars.githubusercontent.com/u/397308?v=4" />andreyv</a> commented <strong> 6 years ago</strong> </div> <div class="markdown-body"> <p>The actual problem is visible in the logs: <code>Uncaught exception in write_error</code>.</p> <details><summary>ContestWebServer log with the actual exception</summary> ``` 2018-11-06 20:35:03,331 - INFO [Contest,0 12 authentication::_authenticate_request_from_cookie] Unsuccessful cookie authentication: no cookie provided 2018-11-06 20:35:03,333 - ERROR [Contest,0 12 web::send_error] Uncaught exception in write_error Traceback (most recent call last): File "/usr/lib/python3/dist-packages/tornado/web.py", line 1489, in _execute result = self.prepare() File "/usr/local/lib/python3.6/dist-packages/cms-1.5.dev0-py3.6.egg/cms/server/contest/handlers/contest.py", line 67, in prepare self.choose_contest() File "/usr/local/lib/python3.6/dist-packages/cms-1.5.dev0-py3.6.egg/cms/server/contest/handlers/contest.py", line 110, in choose_contest raise tornado.web.HTTPError(404) tornado.web.HTTPError: HTTP 404: Not Found During handling of the above exception, another exception occurred: Traceback (most recent call last): File "/usr/lib/python3/dist-packages/tornado/web.py", line 1037, in send_error self.write_error(status_code, **kwargs) File "/usr/local/lib/python3.6/dist-packages/cms-1.5.dev0-py3.6.egg/cms/server/contest/handlers/base.py", line 151, in write_error self.render("error.html", status_code=status_code, **self.r_params) File "/usr/local/lib/python3.6/dist-packages/cms-1.5.dev0-py3.6.egg/cms/server/contest/handlers/base.py", line 72, in render for chunk in t.generate(**params): File "/usr/lib/python3/dist-packages/jinja2/environment.py", line 1045, in generate yield self.environment.handle_exception(exc_info, True) File "/usr/lib/python3/dist-packages/jinja2/environment.py", line 780, in handle_exception reraise(exc_type, exc_value, tb) File "/usr/lib/python3/dist-packages/jinja2/_compat.py", line 37, in reraise raise value.with_traceback(tb) File "/usr/local/lib/python3.6/dist-packages/cms-1.5.dev0-py3.6.egg/cms/server/contest/templates/error.html", line 1, in top-level template code {% extends "base.html" %} File "/usr/local/lib/python3.6/dist-packages/cms-1.5.dev0-py3.6.egg/cms/server/contest/templates/base.html", line 9, in top-level template code <link rel="shortcut icon" href="{{ url("static", "favicon.ico") }}" /> TypeError: 'NoneType' object is not callable 2018-11-06 20:35:03,334 - WARNING [Contest,0 12 web::log_request] 404 GET /zzz (::ffff:127.0.0.1) 17.19ms ``` </details> <details><summary>A similar failure in AdminWebServer</summary> ``` 2018-11-23 09:47:38,999 - WARNING [Admin,0 50 web::log_exception] 403 POST /login (127.0.0.1): XSRF cookie does not match POST argument 2018-11-23 09:47:39,034 - ERROR [Admin,0 50 web::send_error] Uncaught exception in write_error Traceback (most recent call last): File "/usr/lib/python3/dist-packages/tornado/web.py", line 1487, in _execute self.check_xsrf_cookie() File "/usr/lib/python3/dist-packages/tornado/web.py", line 1334, in check_xsrf_cookie raise HTTPError(403, "XSRF cookie does not match POST argument") tornado.web.HTTPError: HTTP 403: Forbidden (XSRF cookie does not match POST argument) During handling of the above exception, another exception occurred: Traceback (most recent call last): File "/usr/lib/python3/dist-packages/tornado/web.py", line 1037, in send_error self.write_error(status_code, **kwargs) File "/usr/local/lib/python3.6/dist-packages/cms-1.5.dev0-py3.6.egg/cms/server/admin/handlers/base.py", line 336, in write_error self.render("error.html", status_code=status_code, **self.r_params) File "/usr/local/lib/python3.6/dist-packages/cms-1.5.dev0-py3.6.egg/cms/server/admin/handlers/base.py", line 281, in render for chunk in t.generate(**params): File "/usr/lib/python3/dist-packages/jinja2/environment.py", line 1045, in generate yield self.environment.handle_exception(exc_info, True) File "/usr/lib/python3/dist-packages/jinja2/environment.py", line 780, in handle_exception reraise(exc_type, exc_value, tb) File "/usr/lib/python3/dist-packages/jinja2/_compat.py", line 37, in reraise raise value.with_traceback(tb) File "/usr/local/lib/python3.6/dist-packages/cms-1.5.dev0-py3.6.egg/cms/server/admin/templates/error.html", line 1, in top-level template code {% extends "base.html" %} File "/usr/local/lib/python3.6/dist-packages/cms-1.5.dev0-py3.6.egg/cms/server/admin/templates/base.html", line 6, in top-level template code <link rel="shortcut icon" href="{{ url("static", "favicon.ico") }}" /> TypeError: 'NoneType' object is not callable 2018-11-23 09:47:39,048 - WARNING [Admin,0 50 web::log_request] 403 POST /login (127.0.0.1) 49.50ms ``` </details> </div> </div> <div class="comment"> <div class="user"> <a rel="noreferrer nofollow" target="_blank" href="https://github.com/andreyv"><img src="https://avatars.githubusercontent.com/u/397308?v=4" />andreyv</a> commented <strong> 5 years ago</strong> </div> <div class="markdown-body"> <p>The problem seems to be because when the template is rendered and <code>render_params()</code> is called, <code>self.url</code> is <code>None</code>. <code>self.url</code> is assigned in <code>CommonRequestHandler.prepare()</code>.</p> <p>Could it be that <code>prepare()</code> has not been called at the time the error template is rendered in <code>write_error()</code>?</p> </div> </div> <div class="comment"> <div class="user"> <a rel="noreferrer nofollow" target="_blank" href="https://github.com/andreyv"><img src="https://avatars.githubusercontent.com/u/397308?v=4" />andreyv</a> commented <strong> 5 years ago</strong> </div> <div class="markdown-body"> <p>Indeed, on both occasions <code>prepare()</code> has not been called at the time the error is processed.</p> <p>This change fixes the first failure:</p> <pre><code class="language-diff">diff --git a/cms/server/contest/handlers/contest.py b/cms/server/contest/handlers/contest.py index be2f333a..1e91c3de 100644 --- a/cms/server/contest/handlers/contest.py +++ b/cms/server/contest/handlers/contest.py @@ -64,6 +64,8 @@ class ContestHandler(BaseHandler): self.contest_url = None def prepare(self): + super().prepare() + self.choose_contest() if self.contest.allowed_localizations: @@ -74,8 +76,6 @@ class ContestHandler(BaseHandler): (k, v) for k, v in self.available_translations.items() if k in lang_codes) - super().prepare() - if self.is_multi_contest(): self.contest_url = self.url[self.contest.name] else:</code></pre> <p>The XSRF error is harder because the check is called just before <code>prepare()</code> in Tornado code:</p> <p><a href="https://github.com/tornadoweb/tornado/blob/975e9168560cc03f6210d0d3aab10c870bd47080/tornado/web.py#L1674-L1676">https://github.com/tornadoweb/tornado/blob/975e9168560cc03f6210d0d3aab10c870bd47080/tornado/web.py#L1674-L1676</a></p> <p>I see about three ways to solve this:</p> <ul> <li>Move <code>self.url</code> assignment from <code>prepare()</code> to <code>__init__()</code> (<code>self.request</code> is already defined at that time)</li> <li>Add a check in or around <code>self.render_params()</code> to raise an exception if <code>self.url</code> is <code>None</code>. This way <code>write_error()</code> can catch it and fall back to a simple error page.</li> <li>Simplify the error page or replace it with Tornado default</li> </ul> <p>Thoughts?</p> </div> </div> <div class="comment"> <div class="user"> <a rel="noreferrer nofollow" target="_blank" href="https://github.com/wil93"><img src="https://avatars.githubusercontent.com/u/1285361?v=4" />wil93</a> commented <strong> 1 year ago</strong> </div> <div class="markdown-body"> <p>If I understand correctly, this is now fixed in ContestWebServer but still broken in AdminWebServer.</p> </div> </div> <div class="page-bar-simple"> </div> <div class="footer"> <ul class="body"> <li>© <script> document.write(new Date().getFullYear()) </script> Githubissues.</li> <li>Githubissues is a development platform for aggregating issues.</li> </ul> </div> <script src="https://cdn.jsdelivr.net/npm/jquery@3.5.1/dist/jquery.min.js"></script> <script src="/githubissues/assets/js.js"></script> <script src="/githubissues/assets/markdown.js"></script> <script src="https://cdn.jsdelivr.net/gh/highlightjs/cdn-release@11.4.0/build/highlight.min.js"></script> <script src="https://cdn.jsdelivr.net/gh/highlightjs/cdn-release@11.4.0/build/languages/go.min.js"></script> <script> hljs.highlightAll(); </script> </body> </html>