FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Add a downloader + verifier for static.rust-lang.org by emberian · Pull Request #179 · PistonDevelopers/VisualRust · GitHub

/ VisualRust Public

Add a downloader + verifier for static.rust-lang.org - #179

Open
emberian wants to merge 1 commit into
PistonDevelopers:masterfrom
emberian:master
ConversationCommits (1)ChecksFiles changed
Open

Add a downloader + verifier for static.rust-lang.org#179
emberian wants to merge 1 commit into
PistonDevelopers:masterfrom
emberian:master

Conversation

emberian commented Aug 21, 2015

Copy link
Copy Markdown
Contributor

This is pretty primitive and completely synchronous with no progress notifications.

First step towards #172

Add a downloader + verifier for static.rust-lang.org
e59a1e3 
This is pretty primitive and completely synchronous with no progress notifications.
vosen reviewed Aug 23, 2015
View reviewed changes
Comment thread VisualRust/Downloader.cs

vosen Aug 23, 2015

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I'm not a security person by any stretch of the imagination, so this might be dumb, but what does checking for exact cert fingerprint gives us?

emberian Aug 24, 2015

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

It gives us the knowledge that the connection isn't being MITM'd by any other certificates that may be trusted by the root CA store - either added by malware, compromised certificate, or just plain malice (which has been documented)

emberian Aug 24, 2015

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

It's called certificate pinning, and browsers use it in the form of HSTS.

vosen commented Aug 23, 2015

Copy link
Copy Markdown
Member

I don't enforce any particular style of coding in this project, but please use camelCase for variables and function arguments.
i don't know which gpg4win build did you download, but in "vanilla" package, gpg2.exe and gpgconf.exe don't depend on libassuan-0.dll, giving a modest of saving of 69KB.
And a heads up, I'll merge this after releasing 0.1.1 into the wild, which will happen once I get my identity verified for code signing certificate, so this PR might linger here few days longer than seemingly necessary.

emberian commented Aug 24, 2015

Copy link
Copy Markdown
Contributor Author

@vosen No problem! I'm glad we'll be getting a signing cert :) I'll clean up the style - C# isn't what I've used in any projects except minor "learn the language" programs. I'll poke around for something that doesn't depend on libassuan-0.dll

emberian commented Aug 24, 2015

Copy link
Copy Markdown
Contributor Author

I'm back to uni tomorrow so I might be a bit slower than usual in responding, as well.

emberian commented Aug 24, 2015

Copy link
Copy Markdown
Contributor Author

Also, as detailed a review as you can give in regards to structure etc is useful for me - I don't do much programming in the OO style, and C# is a language I'm very weak in. Any avoidance of useful features or handy library things is not a guided choice, but just ignorance!

vosen commented Aug 27, 2015

Copy link
Copy Markdown
Member

Your style is OK, other than using a Gpg singleton. I know, we have some examples of this style in the codebase already, but it should be avoided. And I've noticed just now that the VS settings in ReinitializeGpgPaths are only read once, at the first use of Gpg. This is wrong, we won't apply any settings updates.

Boddlnagg commented Oct 10, 2016

Copy link
Copy Markdown
Contributor

I guess this is outdated and we should just wrap rustup instead, right? (Sorry for the wasted effort ...)

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

Back | FazBrowse Home | New Git URL