Support for custom TLS port + optional verify peer#37
Open
rohitjoshi wants to merge 25 commits into
Open
Conversation
This allows `WindowSize`s to be compared for equality against `i32`s transparently...
… size methods There is no point in coercing the internal `WindowSize` struct into an `i32` before returning it; in fact, this can easily be done by clients if they have such a requirement.
This commit adds two methods to the `Session` trait that allow it to be notified when a new WINDOW_UPDATE frame signals an increase in the outbound window size for both the connection-level, as well as the stream-level windows.
Two new methods are added to the `Session` trait that allow it to receive notice when either the connection-level or a stream-level inbound window size decreases. These calls will, as a rule, always be emitted in pairs by the `HttpConnection` upon parsing a flow-control subject frame (at the moment this is only the DATA frame, as per the HTTP/2 spec).
The state now has an entry per stream, instead of storing just the raw stream. This is in preparation for extending the state to be able to track more stream-related information, that does not belong on the Stream trait itself. (The Stream trait is supposed to be the bridge between the session and its client that is interested only in stream-level events; requiring those clients to also implement tracking various book-keeping stream state is not only redundant---all streams will need to track the same state in the same way---but also not in line with what the Stream trait is supposed to represent.)
The method is used to signal errors that the local peer detects, in contrast to those that the remote signals by sending an RST_STREAM frame. The Session should send the RST_STREAM after the `on_stream_error` is called.
The mod.rs file was getting quite large...
The WindowUpdateStrategy trait is used to define the algorithm for updating the flow control window of both the connection, as well as individual stream. The object is provided the new size of the corresponding window and is expected to produce an action to be taken immediately: either increase the size of the window or do nothing.
mlalic
reviewed
Apr 12, 2017
| /// Builds up a default `SslContext` instance wth TLS settings that the | ||
| /// HTTP/2 spec mandates. The path to the CA file needs to be provided. | ||
| pub fn build_default_context(ca_file_path: &Path) -> Result<SslContext, TlsConnectError> { | ||
| pub fn build_default_context(ca_file_path: &Path, verify_peer : bool, cert_file: Option<PathBuf>, private_key: Option<PathBuf>) -> Result<SslContext, TlsConnectError> { |
Owner
There was a problem hiding this comment.
This was meant to be a default context, which would be sensible without requiring the user to figure out what various switches do. That said, I'm not opposed to having a helper that offers more customization, without callers having to figure out how to build raw SslContexts.
Would you mind renaming this method to something like build_context and then implementing the original build_default_context in terms of that?
Owner
|
Thanks for the PR! This seems like a few definite quality of life improvements. I just had one minor comment, that'd be great if you could address. |
Author
|
@mlalic thanks. I have addressed the review comments. Let me know if you recommend any other changes. |
Flow ctrl Support
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I have added support for following two features.