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

Transparently resume and complete stalled or failed submission downloads. #1994

Closed
zenmonkeykstop opened this issue May 8, 2024 · 4 comments · Fixed by #2006
Closed

Transparently resume and complete stalled or failed submission downloads. #1994

zenmonkeykstop opened this issue May 8, 2024 · 4 comments · Fixed by #2006
Assignees
Labels

Comments

@zenmonkeykstop
Copy link
Contributor

zenmonkeykstop commented May 8, 2024

Description

Large file downloads over Tor fail more often than over public Internet, and the client and server do not yet support resuming failed downloads or even provide good feedback as to download progress - users just have to start over after an indeterminate period.

With range request support in the server, the client would be able to a) be more assertive about timeouts, and b) send multiple requests to complete downloads, ideally without requiring user intervention.

There have been multiple related issues touching on different aspects of this problem:

How will this impact SecureDrop users?

UX improvement for journalists using SDW.

How would this affect the SecureDrop Workstation threat model?

If download segments are stored in the same way and on the same AppVM as full downloads are currently, this should not have any threat model implications.

User Stories

@micahflee
Copy link
Contributor

I've been studying the securedrop-client code to get a handle on how it works and how I can start implementing range requests.

In the dev environment, you can choose to use the proxy or not use the proxy and send http requests directly to the dev server. I should start by making this work without using the proxy first, and then later make sure it works with the proxy. This makes sense anyway while proxy v2 is still under development.

Another thing to consider is providing download progress feedback to the user. It's related but I think it makes sense to start with just auto-resuming failed downloads. This way I can focus on the core functionality of supporting range requests, and we can spend more time thinking through the changes in the UI later.

So the work should be in this order:

  • auto-resume downloads without the proxy, and without changing the UI
  • show download status feedback in the UI (unless we want to punt this to a future issue instead to give it more thought)
  • support auto-resume downloads through the v2 proxy

It looks like all of the download logic is in client/securedrop_client/sdk/__init__.py, and specifically the API.download_submission method (and _send_json_request and _send_http_json_request). I'll start by updating this so that it:

  • streams the download
  • on failure, retries from where it left off using a range request
  • it retries a maximum of 3 times before raising an exception

Does this seem like a good plan moving forward?

@zenmonkeykstop
Copy link
Contributor Author

I think we can probably punt on UI changes for now, so that's the only change I'd make to order of operations. Otherwise plan sounds solid to me!

One thing worth considering would be making the retry limit configurable so folks can twiddle with it in prod if necessary. There's already similar code in there for the sync timeout.

@micahflee
Copy link
Contributor

One thing worth considering would be making the retry limit configurable so folks can twiddle with it in prod if necessary. There's already similar code in there for the sync timeout.

Can you point me to that code? I'm wondering how new settings like this are defined.

@zenmonkeykstop
Copy link
Contributor Author

zenmonkeykstop commented May 14, 2024

Yup, it was added in #1552 - used a kindof hack where a service name was defined in dom0 that was actually the name/value pair and then checked by the client. I think if we were doing it now we'd use the qubesdb code already in main of securdrop-workstation, which was added in freedomofpress/securedrop-workstation#1001

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

Successfully merging a pull request may close this issue.

2 participants