Merge branch 'fix_405_error_and_other_methods' of https://github.com/mig5/onionshare into mig5-fix_405_error_and_other_methods

This commit is contained in:
Micah Lee 2021-05-25 16:27:26 -07:00
commit c682b7ec01
No known key found for this signature in database
GPG key ID: 403C2657CD994F73
12 changed files with 182 additions and 51 deletions

View file

@ -0,0 +1,21 @@
<!DOCTYPE html>
<html>
<head>
<title>OnionShare: An error occurred</title>
<meta charset="utf-8" />
<meta name="viewport" content="width=device-width, initial-scale=1">
<link href="{{ static_url_path }}/img/favicon.ico" rel="icon" type="image/x-icon">
<link rel="stylesheet" rel="subresource" type="text/css" href="{{ static_url_path }}/css/style.css" media="all">
</head>
<body>
<div class="info-wrapper">
<div class="info">
<p><img class="logo" src="{{ static_url_path }}/img/logo_large.png" title="OnionShare"></p>
<p class="info-header">Sorry, an unexpected error seems to have occurred, and your request didn't succeed.</p>
</div>
</div>
</body>
</html>

View file

@ -39,6 +39,12 @@ class ChatModeWeb:
# This tracks the history id # This tracks the history id
self.cur_history_id = 0 self.cur_history_id = 0
# Whether or not we can send REQUEST_INDIVIDUAL_FILE_STARTED
# and maybe other events when requests come in to this mode
# Chat mode has no concept of individual file requests that
# turn into history widgets in the GUI, so set it to False
self.supports_file_requests = False
self.define_routes() self.define_routes()
def define_routes(self): def define_routes(self):
@ -46,7 +52,7 @@ class ChatModeWeb:
The web app routes for chatting The web app routes for chatting
""" """
@self.web.app.route("/") @self.web.app.route("/", methods=["GET"], provide_automatic_options=False)
def index(): def index():
history_id = self.cur_history_id history_id = self.cur_history_id
self.cur_history_id += 1 self.cur_history_id += 1
@ -72,7 +78,7 @@ class ChatModeWeb:
) )
return self.web.add_security_headers(r) return self.web.add_security_headers(r)
@self.web.app.route("/update-session-username", methods=["POST"]) @self.web.app.route("/update-session-username", methods=["POST"], provide_automatic_options=False)
def update_session_username(): def update_session_username():
history_id = self.cur_history_id history_id = self.cur_history_id
data = request.get_json() data = request.get_json()

View file

@ -64,6 +64,10 @@ class ReceiveModeWeb:
# This tracks the history id # This tracks the history id
self.cur_history_id = 0 self.cur_history_id = 0
# Whether or not we can send REQUEST_INDIVIDUAL_FILE_STARTED
# and maybe other events when requests come in to this mode
self.supports_file_requests = True
self.define_routes() self.define_routes()
def define_routes(self): def define_routes(self):
@ -71,7 +75,7 @@ class ReceiveModeWeb:
The web app routes for receiving files The web app routes for receiving files
""" """
@self.web.app.route("/") @self.web.app.route("/", methods=["GET"], provide_automatic_options=False)
def index(): def index():
history_id = self.cur_history_id history_id = self.cur_history_id
self.cur_history_id += 1 self.cur_history_id += 1
@ -93,7 +97,7 @@ class ReceiveModeWeb:
) )
return self.web.add_security_headers(r) return self.web.add_security_headers(r)
@self.web.app.route("/upload", methods=["POST"]) @self.web.app.route("/upload", methods=["POST"], provide_automatic_options=False)
def upload(ajax=False): def upload(ajax=False):
""" """
Handle the upload files POST request, though at this point, the files have Handle the upload files POST request, though at this point, the files have
@ -225,7 +229,7 @@ class ReceiveModeWeb:
) )
return self.web.add_security_headers(r) return self.web.add_security_headers(r)
@self.web.app.route("/upload-ajax", methods=["POST"]) @self.web.app.route("/upload-ajax", methods=["POST"], provide_automatic_options=False)
def upload_ajax_public(): def upload_ajax_public():
if not self.can_upload: if not self.can_upload:
return self.web.error403() return self.web.error403()

View file

@ -52,6 +52,10 @@ class SendBaseModeWeb:
# This tracks the history id # This tracks the history id
self.cur_history_id = 0 self.cur_history_id = 0
# Whether or not we can send REQUEST_INDIVIDUAL_FILE_STARTED
# and maybe other events when requests come in to this mode
self.supports_file_requests = True
self.define_routes() self.define_routes()
self.init() self.init()
@ -208,10 +212,6 @@ class SendBaseModeWeb:
history_id = self.cur_history_id history_id = self.cur_history_id
self.cur_history_id += 1 self.cur_history_id += 1
# Only GET requests are allowed, any other method should fail
if request.method != "GET":
return self.web.error405(history_id)
self.web.add_request( self.web.add_request(
self.web.REQUEST_INDIVIDUAL_FILE_STARTED, self.web.REQUEST_INDIVIDUAL_FILE_STARTED,
path, path,

View file

@ -134,8 +134,8 @@ class ShareModeWeb(SendBaseModeWeb):
The web app routes for sharing files The web app routes for sharing files
""" """
@self.web.app.route("/", defaults={"path": ""}) @self.web.app.route("/", defaults={"path": ""}, methods=["GET"], provide_automatic_options=False)
@self.web.app.route("/<path:path>") @self.web.app.route("/<path:path>", methods=["GET"], provide_automatic_options=False)
def index(path): def index(path):
""" """
Render the template for the onionshare landing page. Render the template for the onionshare landing page.
@ -160,7 +160,7 @@ class ShareModeWeb(SendBaseModeWeb):
return self.render_logic(path) return self.render_logic(path)
@self.web.app.route("/download") @self.web.app.route("/download", methods=["GET"], provide_automatic_options=False)
def download(): def download():
""" """
Download the zip file. Download the zip file.

View file

@ -229,6 +229,20 @@ class Web:
mode.cur_history_id += 1 mode.cur_history_id += 1
return self.error404(history_id) return self.error404(history_id)
@self.app.errorhandler(405)
def method_not_allowed(e):
mode = self.get_mode()
history_id = mode.cur_history_id
mode.cur_history_id += 1
return self.error405(history_id)
@self.app.errorhandler(500)
def method_not_allowed(e):
mode = self.get_mode()
history_id = mode.cur_history_id
mode.cur_history_id += 1
return self.error500(history_id)
@self.app.route("/<password_candidate>/shutdown") @self.app.route("/<password_candidate>/shutdown")
def shutdown(password_candidate): def shutdown(password_candidate):
""" """
@ -280,6 +294,8 @@ class Web:
return self.add_security_headers(r) return self.add_security_headers(r)
def error404(self, history_id): def error404(self, history_id):
mode = self.get_mode()
if mode.supports_file_requests:
self.add_request( self.add_request(
self.REQUEST_INDIVIDUAL_FILE_STARTED, self.REQUEST_INDIVIDUAL_FILE_STARTED,
request.path, request.path,
@ -293,6 +309,8 @@ class Web:
return self.add_security_headers(r) return self.add_security_headers(r)
def error405(self, history_id): def error405(self, history_id):
mode = self.get_mode()
if mode.supports_file_requests:
self.add_request( self.add_request(
self.REQUEST_INDIVIDUAL_FILE_STARTED, self.REQUEST_INDIVIDUAL_FILE_STARTED,
request.path, request.path,
@ -305,6 +323,21 @@ class Web:
) )
return self.add_security_headers(r) return self.add_security_headers(r)
def error500(self, history_id):
mode = self.get_mode()
if mode.supports_file_requests:
self.add_request(
self.REQUEST_INDIVIDUAL_FILE_STARTED,
request.path,
{"id": history_id, "status_code": 500},
)
self.add_request(Web.REQUEST_OTHER, request.path)
r = make_response(
render_template("500.html", static_url_path=self.static_url_path), 500
)
return self.add_security_headers(r)
def add_security_headers(self, r): def add_security_headers(self, r):
""" """
Add security headers to a request Add security headers to a request

View file

@ -37,8 +37,8 @@ class WebsiteModeWeb(SendBaseModeWeb):
The web app routes for sharing a website The web app routes for sharing a website
""" """
@self.web.app.route("/", defaults={"path": ""}) @self.web.app.route("/", defaults={"path": ""}, methods=["GET"], provide_automatic_options=False)
@self.web.app.route("/<path:path>") @self.web.app.route("/<path:path>", methods=["GET"], provide_automatic_options=False)
def path_public(path): def path_public(path):
return path_logic(path) return path_logic(path)

View file

@ -465,6 +465,20 @@ class GuiBaseTest(unittest.TestCase):
# We should have timed out now # We should have timed out now
self.assertEqual(tab.get_mode().server_status.status, 0) self.assertEqual(tab.get_mode().server_status.status, 0)
def hit_405(self, url, expected_resp, data = {}, methods = [] ):
"""Test various HTTP methods and the response"""
for method in methods:
if method == "put":
r = requests.put(url, data = data)
if method == "post":
r = requests.post(url, data = data)
if method == "delete":
r = requests.delete(url)
if method == "options":
r = requests.options(url)
self.assertTrue(expected_resp in r.text)
self.assertFalse('Werkzeug' in r.headers)
# Grouped tests follow from here # Grouped tests follow from here
def run_all_common_setup_tests(self): def run_all_common_setup_tests(self):

View file

@ -30,7 +30,7 @@ class TestChat(GuiBaseTest):
def change_username(self, tab): def change_username(self, tab):
"""Test that we can change our username""" """Test that we can change our username"""
url = f"http://127.0.0.1:{tab.app.port}/update-session-username" url = f"http://127.0.0.1:{tab.app.port}/update-session-username"
data = {"username":"oniontest"} data = {"username": "oniontest"}
if tab.settings.get("general", "public"): if tab.settings.get("general", "public"):
r = requests.post(url, json=data) r = requests.post(url, json=data)
else: else:
@ -47,28 +47,7 @@ class TestChat(GuiBaseTest):
self.assertTrue(jsonResponse["success"]) self.assertTrue(jsonResponse["success"])
self.assertEqual(jsonResponse["username"], "oniontest") self.assertEqual(jsonResponse["username"], "oniontest")
def change_username_too_long(self, tab): def run_all_chat_mode_started_tests(self, tab):
"""Test that we can't set our username to something 128 chars or longer"""
url = f"http://127.0.0.1:{tab.app.port}/update-session-username"
bad_username = "sduBB9yEMkyQpwkMM4A9nUbQwNUbPU2PQuJYN26zCQ4inELpB76J5i5oRUnD3ESVaE9NNE8puAtBj2DiqDaZdVqhV8MonyxSSGHRv87YgM5dzwBYPBxttoQSKZAUkFjo"
data = {"username":bad_username}
if tab.settings.get("general", "public"):
r = requests.post(url, json=data)
else:
r = requests.post(
url,
json=data,
auth=requests.auth.HTTPBasicAuth(
"onionshare", tab.get_mode().server_status.web.password
),
)
QtTest.QTest.qWait(500, self.gui.qtapp)
jsonResponse = r.json()
self.assertFalse(jsonResponse["success"])
self.assertNotEqual(jsonResponse["username"], bad_username)
def run_all_chat_mode_tests(self, tab):
"""Tests in chat mode after starting a chat""" """Tests in chat mode after starting a chat"""
self.server_working_on_start_button_pressed(tab) self.server_working_on_start_button_pressed(tab)
self.server_status_indicator_says_starting(tab) self.server_status_indicator_says_starting(tab)
@ -79,9 +58,9 @@ class TestChat(GuiBaseTest):
self.have_copy_url_button(tab) self.have_copy_url_button(tab)
self.have_show_qr_code_button(tab) self.have_show_qr_code_button(tab)
self.server_status_indicator_says_started(tab) self.server_status_indicator_says_started(tab)
self.view_chat(tab)
self.change_username(tab) def run_all_chat_mode_stopping_tests(self, tab):
self.change_username_too_long(tab) """Tests stopping a chat"""
self.server_is_stopped(tab) self.server_is_stopped(tab)
self.web_server_is_stopped(tab) self.web_server_is_stopped(tab)
self.server_status_indicator_says_closed(tab) self.server_status_indicator_says_closed(tab)
@ -93,5 +72,27 @@ class TestChat(GuiBaseTest):
Test chat mode Test chat mode
""" """
tab = self.new_chat_tab() tab = self.new_chat_tab()
self.run_all_chat_mode_tests(tab) self.run_all_chat_mode_started_tests(tab)
self.view_chat(tab)
self.change_username(tab)
self.run_all_chat_mode_stopping_tests(tab)
self.close_all_tabs()
def test_405_page_returned_for_invalid_methods(self):
"""
Our custom 405 page should return for invalid methods
"""
tab = self.new_chat_tab()
tab.get_mode().mode_settings_widget.public_checkbox.click()
self.run_all_chat_mode_started_tests(tab)
url = f"http://127.0.0.1:{tab.app.port}/"
self.hit_405(
url,
expected_resp="OnionShare: 405 Method Not Allowed",
data={"foo": "bar"},
methods=["put", "post", "delete", "options"],
)
self.run_all_chat_mode_stopping_tests(tab)
self.close_all_tabs() self.close_all_tabs()

View file

@ -286,3 +286,22 @@ class TestReceive(GuiBaseTest):
self.run_all_upload_non_writable_dir_tests(tab) self.run_all_upload_non_writable_dir_tests(tab)
self.close_all_tabs() self.close_all_tabs()
def test_405_page_returned_for_invalid_methods(self):
"""
Our custom 405 page should return for invalid methods
"""
tab = self.new_receive_tab()
tab.get_mode().mode_settings_widget.public_checkbox.click()
self.run_all_common_setup_tests()
self.run_all_receive_mode_setup_tests(tab)
self.upload_file(tab, self.tmpfile_test, "test.txt")
url = f"http://127.0.0.1:{tab.app.port}/"
self.hit_405(url, expected_resp="OnionShare: 405 Method Not Allowed", data = {'foo':'bar'}, methods = ["put", "post", "delete", "options"])
self.server_is_stopped(tab)
self.web_server_is_stopped(tab)
self.server_status_indicator_says_closed(tab)
self.close_all_tabs()

View file

@ -608,3 +608,20 @@ class TestShare(GuiBaseTest):
self.hit_401(tab) self.hit_401(tab)
self.close_all_tabs() self.close_all_tabs()
def test_405_page_returned_for_invalid_methods(self):
"""
Our custom 405 page should return for invalid methods
"""
tab = self.new_share_tab()
tab.get_mode().autostop_sharing_checkbox.click()
tab.get_mode().mode_settings_widget.public_checkbox.click()
self.run_all_common_setup_tests()
self.run_all_share_mode_setup_tests(tab)
self.run_all_share_mode_started_tests(tab)
url = f"http://127.0.0.1:{tab.app.port}/"
self.hit_405(url, expected_resp="OnionShare: 405 Method Not Allowed", data = {'foo':'bar'}, methods = ["put", "post", "delete", "options"])
self.history_widgets_present(tab)
self.close_all_tabs()

View file

@ -99,3 +99,19 @@ class TestWebsite(GuiBaseTest):
tab.get_mode().disable_csp_checkbox.click() tab.get_mode().disable_csp_checkbox.click()
self.run_all_website_mode_download_tests(tab) self.run_all_website_mode_download_tests(tab)
self.close_all_tabs() self.close_all_tabs()
def test_405_page_returned_for_invalid_methods(self):
"""
Our custom 405 page should return for invalid methods
"""
tab = self.new_website_tab()
tab.get_mode().mode_settings_widget.public_checkbox.click()
self.run_all_common_setup_tests()
self.run_all_website_mode_setup_tests(tab)
self.run_all_website_mode_started_tests(tab)
url = f"http://127.0.0.1:{tab.app.port}/"
self.hit_405(url, expected_resp="OnionShare: 405 Method Not Allowed", data = {'foo':'bar'}, methods = ["put", "post", "delete", "options"])
self.close_all_tabs()