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

Back to downloading of jars #42

Merged
merged 6 commits into from
Mar 31, 2021
Merged

Back to downloading of jars #42

merged 6 commits into from
Mar 31, 2021

Conversation

vitorenesduarte
Copy link
Contributor

@vitorenesduarte vitorenesduarte commented Mar 29, 2021

Closes: #41

This PR essentially reverts #25. Differences:

  • use ureq instead reqwest as ureq is more lightweight
  • download jars from our repository instead of downloading them from the releases page of their repositories; this prevents situations where a GitHub release is deleted, which would result in modelator not working
  • ensure that jars were not corrupted by embedding their SHA256 hash in the Rust code and checking that hashes match after the jars are written to disk

@andrey-kuprianov
Copy link
Contributor

Nice Vitor! I think it is really good that way -- I like the minimal dependency introduced, as well that it's now downloaded from our repo.

Feel free to merge:)

@andrey-kuprianov
Copy link
Contributor

Just an afterthought: I think it would be nice when the JARs are downloaded to display to the user the information like

Downloading model-checkers:
  downloading Apalache ... done
  downloading TLC ... done

This way the user will understand where the initial delay comes from.

@romac
Copy link
Member

romac commented Mar 29, 2021

May I suggest embedding the SHA-256 hashes of each JAR file in the Rust code, and checking them after the download to ensure they were not corrupted or tempered with during transfer.

@andrey-kuprianov
Copy link
Contributor

May I suggest embedding the SHA-256 hashes of each JAR file in the Rust code, and checking them after the download to ensure they were not corrupted or tempered with during transfer.

that is a very good idea!

@vitorenesduarte
Copy link
Contributor Author

May I suggest embedding the SHA-256 hashes of each JAR file in the Rust code, and checking them after the download to ensure they were not corrupted or tempered with during transfer.

ureq uses TLS by default: https://github.com/algesten/ureq/blob/main/Cargo.toml#L18
Doesn't this prevent that situation?

@romac
Copy link
Member

romac commented Mar 29, 2021

Doesn't this prevent that situation?

It does prevent against tampering during transfer but not corruption when writing to disk for example. I would therefore still advise checking the hash to ensure the file that's been downloaded and written to disk is intact.

@vitorenesduarte
Copy link
Contributor Author

Doesn't this prevent that situation?

It does prevent against tampering during transfer but not corruption when writing to disk for example. I would therefore still advise checking the hash to ensure the file that's been downloaded and written to disk is intact.

That makes sense, thanks! Done in 4b27208. I'll update the top-level comment accordingly.

@vitorenesduarte
Copy link
Contributor Author

Just an afterthought: I think it would be nice when the JARs are downloaded to display to the user the information like

Downloading model-checkers:
  downloading Apalache ... done
  downloading TLC ... done

This way the user will understand where the initial delay comes from.

Done in 7562461.

@andrey-kuprianov
Copy link
Contributor

Looks good to me:)

@vitorenesduarte vitorenesduarte merged commit 01c50f6 into main Mar 31, 2021
@vitorenesduarte vitorenesduarte deleted the vitor/download_jars branch March 31, 2021 07:33
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.

Make modelator crate publishable
3 participants