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

Allow more generic SSL verification (fixes #244) #249

Merged
merged 2 commits into from
Jan 15, 2015

Conversation

Manishearth
Copy link
Contributor

No description provided.

@GitCop
Copy link

GitCop commented Jan 14, 2015

There were the following issues with your Pull Request

  • Commit: 8e44720
    • Commits must be in the following format: %{type}(%{scope}): %{description}
    • Invalid type. Valid types are feat, fix, docs, style, refactor, perf, test, chore, revert

Guidelines are available at https://github.com/hyperium/hyper/blob/master/CONTRIBUTING.md


This message was auto-generated by https://gitcop.com

@GitCop
Copy link

GitCop commented Jan 14, 2015

There were the following issues with your Pull Request

  • Commit: 69d3aeb
    • Commits must be in the following format: %{type}(%{scope}): %{description}

Guidelines are available at https://github.com/hyperium/hyper/blob/master/CONTRIBUTING.md


This message was auto-generated by https://gitcop.com

@Manishearth
Copy link
Contributor Author

This doesn't use unboxed closures yet, because I don't like having to specify a dummy unboxed closure every time. I tried using default type params to get around it, but it didn't work too well and I got an ICE.

Any ideas as to how this can be done?

@seanmonstar
Copy link
Member

I don't understand what you mean. I'd likely let set_ssl_verifier be generic over Fn, and then store it as Box.

@@ -325,7 +328,9 @@ impl NetworkConnector for HttpConnector {
debug!("https scheme");
let stream = try!(TcpStream::connect(addr));
let mut context = try!(SslContext::new(Sslv23).map_err(lift_ssl_error));
self.0.as_ref().map(|cb| context.set_verify(SslVerifyPeer, Some(*cb)));
if let Some(ref v) = self.0 {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we use a more descriptive name like verifier? It's a small change but it brings clarity.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

(fixed)

@Manishearth
Copy link
Contributor Author

@seanmonstar Oh, I thought "boxed unboxed closures" didn't work yet

Added a commit which fixes this

seanmonstar added a commit that referenced this pull request Jan 15, 2015
Allow more generic SSL verification (fixes #244)
@seanmonstar seanmonstar merged commit a0bf2a4 into hyperium:master Jan 15, 2015
@Manishearth Manishearth deleted the sslcontext branch January 28, 2015 06:34
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.

4 participants