Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent uploading a new file or text message if the autostop timer has run out, but another existing upload is still in progress #1356

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

mig5
Copy link
Collaborator

@mig5 mig5 commented May 11, 2021

I think this fixes #1254

  1. Start Receive mode with an autostop timer

  2. Visit the OnionShare service in two tabs

  3. Start uploading a large-ish file shortly before the timer is due to stop, in one of the tabs

  4. Once the autostop timer has stopped, it will keep the service running til the upload in 3) is complete. Meanwhile, try and upload another file in the second tab. You should get an error (in ajax mode) or in non-ajax mode you should get the 'Thank you for using OnionShare, you may now close this window' page.

I tried setting a 403 forbidden rather than use the Thank You page, but that affects the original upload: when it completes, the user would then see a 403 Forbidden.. I think. Better to see a Thank you page. Because I am relying on the can_upload' attribute having been set to True once the autostop timer is run out, I can't really distinguish between the two cases.

The diff looks complicated but really it's just indentation: I add if self.can_upload: on line 102 of receive_mode.py in the /upload route handler, and then indent most of the rest of the logic, with an 'else' condition at the end that throws the thankyou page if can_upload was False.

I also removed the return self.web.error403() in the ajax upload route because that doesn't give any real visual indication to the user, and the other logic can be evaluated instead then in the main upload route.

I'm not 100% confident on this PR but please give it a go and see if there are any issues.. hopefully no regressions.. at the very least though, there should be no fatal errors on the backend and 500 Internal Server errors thrown to the browser anymore..

…s run out, but another existing upload is still in progress
@mig5 mig5 requested a review from micahflee as a code owner May 11, 2021 06:36
@mig5
Copy link
Collaborator Author

mig5 commented May 11, 2021

Copy link
Collaborator

@micahflee micahflee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm so a few things. This isn't quite working for me like I expected.

First, when the auto-stop timer runs out and goes into the negative, for some it jumps back to -2d remaining and starts counting up a second at a time until it gets to 0, and I have no idea why. If the auto-stop timer runs out, it should probably then go to -1s remaining, then -2s, etc. and keep counting down. Do you think you could fix that in this PR too?

When I set Tor Browser to safest to disable javascript:

  • I upload a large file
  • Wait for the autostop timer to run out
  • In another tab try uploading another file

It fails with an internal server error. Here's the verbose output I see:

[May 25 2021 05:04:30 PM] ReceiveModeRequest.__init__
[2021-05-25 17:04:30,711] ERROR in app: Exception on /upload [POST]
Traceback (most recent call last):
  File "/home/user/code/onionshare/desktop/venv/lib/python3.9/site-packages/flask/app.py", line 2447, in wsgi_app
    response = self.full_dispatch_request()
  File "/home/user/code/onionshare/desktop/venv/lib/python3.9/site-packages/flask/app.py", line 1952, in full_dispatch_request
    rv = self.handle_user_exception(e)
  File "/home/user/code/onionshare/desktop/venv/lib/python3.9/site-packages/flask/app.py", line 1821, in handle_user_exception
    reraise(exc_type, exc_value, tb)
  File "/home/user/code/onionshare/desktop/venv/lib/python3.9/site-packages/flask/_compat.py", line 39, in reraise
    raise value
  File "/home/user/code/onionshare/desktop/venv/lib/python3.9/site-packages/flask/app.py", line 1950, in full_dispatch_request
    rv = self.dispatch_request()
  File "/home/user/code/onionshare/desktop/venv/lib/python3.9/site-packages/flask/app.py", line 1936, in dispatch_request
    return self.view_functions[rule.endpoint](**req.view_args)
  File "/home/user/code/onionshare/desktop/venv/lib/python3.9/site-packages/onionshare_cli/web/receive_mode.py", line 106, in upload
    message_received = request.includes_message
  File "/home/user/code/onionshare/desktop/venv/lib/python3.9/site-packages/werkzeug/local.py", line 347, in __getattr__
    return getattr(self._get_current_object(), name)
AttributeError: 'ReceiveModeRequest' object has no attribute 'includes_message'
[May 25 2021 05:04:30 PM] ReceiveModeRequest.close
127.0.0.1 - - [25/May/2021 17:04:30] "POST /upload HTTP/1.1" 500 -
Error on request:
Traceback (most recent call last):
  File "/home/user/code/onionshare/desktop/venv/lib/python3.9/site-packages/werkzeug/serving.py", line 323, in run_wsgi
    execute(self.server.app)
  File "/home/user/code/onionshare/desktop/venv/lib/python3.9/site-packages/werkzeug/serving.py", line 312, in execute
    application_iter = app(environ, start_response)
  File "/home/user/code/onionshare/desktop/venv/lib/python3.9/site-packages/flask/app.py", line 2464, in __call__
    return self.wsgi_app(environ, start_response)
  File "/home/user/code/onionshare/desktop/venv/lib/python3.9/site-packages/onionshare_cli/web/receive_mode.py", line 264, in __call__
    return self.app(environ, start_response)
  File "/home/user/code/onionshare/desktop/venv/lib/python3.9/site-packages/flask/app.py", line 2458, in wsgi_app
    ctx.auto_pop(error)
  File "/home/user/code/onionshare/desktop/venv/lib/python3.9/site-packages/flask/ctx.py", line 452, in auto_pop
    self.pop(exc)
  File "/home/user/code/onionshare/desktop/venv/lib/python3.9/site-packages/flask/ctx.py", line 426, in pop
    request_close()
  File "/home/user/code/onionshare/desktop/venv/lib/python3.9/site-packages/onionshare_cli/web/receive_mode.py", line 557, in close
    if self.told_gui_about_request:
AttributeError: 'ReceiveModeRequest' object has no attribute 'told_gui_about_request'

Are you getting this too?

@mig5
Copy link
Collaborator Author

mig5 commented May 26, 2021

Weird, that was precisely the behaviour that this PR fixed for me. I will test it again as soon as I get a chance..

@mig5
Copy link
Collaborator Author

mig5 commented May 26, 2021

@micahflee did you definitely rebuild your CLI etc to contain the changes from this branch? As it works ok for me: I get the 'You may now close this window' (OnionShare is closed) page render when the second upload tries to occur if the autostop timer has run out and the first upload in the first tab is still uploading. And I get no trackback in the terminal.

Screenshot_2021-05-26_18-28-56

Nevertheless there is a git conflict here and also the countdown timer thing which I will try to work on soon, but I don't know if I will get to it this week (I'll try though!). So either don't block release on this (it's not a new issue) or feel free to fix yourself if you have time and it's obvious

@mig5
Copy link
Collaborator Author

mig5 commented May 31, 2021

@micahflee I've fixed the conflict and added something that changes the server button's wording when an autostop timer has run out but a transfer is still in progress - so it doesn't display any time that has 'entered the negative'. Let me know what you think.

Still confused about your seeing of the 'told gui about request' trackback - that is precisely what this PR is designed to fix, and I didn't reproduce it myself.. let me know if you still get it after running scripts/rebuild_cli.py on this branch?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

500 Internal Server Error when uploading after the auto-stop timer has expired
2 participants