DRAFT: Add NPN support to stud #10

Merged
merged 3 commits into from Feb 23, 2014

Conversation

Projects
None yet
2 participants
@georgekola
Contributor

georgekola commented Feb 12, 2014

This adds NPN support to SPDY. Android SDK would use OK_HTTP and it requires NPN support.

@georgekola

This comment has been minimized.

Show comment Hide comment
@georgekola

georgekola Feb 12, 2014

Contributor

@fedor, I just based this off your commit 40b884a and chromiums project's NPN support
http://src.chromium.org/chrome/trunk/src/net/tools/flip_server/spdy_ssl.cc.
Can you please check if this is fine. I could not get the direct paste of your code to work and had to refer to chromium and openssl headers.

@ChrisFrancis Can you please confirm if OK HTTP works fine with this

Contributor

georgekola commented Feb 12, 2014

@fedor, I just based this off your commit 40b884a and chromiums project's NPN support
http://src.chromium.org/chrome/trunk/src/net/tools/flip_server/spdy_ssl.cc.
Can you please check if this is fine. I could not get the direct paste of your code to work and had to refer to chromium and openssl headers.

@ChrisFrancis Can you please confirm if OK HTTP works fine with this

@georgekola

This comment has been minimized.

Show comment Hide comment
@georgekola

georgekola Feb 12, 2014

Contributor

@mranney what are your thoughts on this ?
I am just advertising spdy/3 support
We should probably not roll this into production for a month :)
We also need confirmation that android does not break with this.

@ChrisFrancis can you please confirm that our existing android app works fine with this.

Contributor

georgekola commented Feb 12, 2014

@mranney what are your thoughts on this ?
I am just advertising spdy/3 support
We should probably not roll this into production for a month :)
We also need confirmation that android does not break with this.

@ChrisFrancis can you please confirm that our existing android app works fine with this.

stud.cc
@@ -1506,6 +1506,19 @@ static void ssl_write(struct ev_loop *loop, ev_io *w, int revents) {
}
}
+#ifdef OPENSSL_NPN_NEGOTIATED
+static const char ssl_npn_line[] =
+"\x06spdy/3" \

This comment has been minimized.

Show comment Hide comment
@indutny

indutny Feb 13, 2014

Contributor

Please add indent (2 or 4 spaces).

@indutny

indutny Feb 13, 2014

Contributor

Please add indent (2 or 4 spaces).

stud.cc
+
+static int ssl_advertise_spdy(SSL* ssl, const unsigned char** data, unsigned int *len, void* arg) {
+ *data = (const unsigned char*) ssl_npn_line;
+ *len = strlen(ssl_npn_line);

This comment has been minimized.

Show comment Hide comment
@indutny

indutny Feb 13, 2014

Contributor

Could use sizeof() - 1 if you are giving away const char[].

@indutny

indutny Feb 13, 2014

Contributor

Could use sizeof() - 1 if you are giving away const char[].

@indutny

This comment has been minimized.

Show comment Hide comment
@indutny

indutny Feb 13, 2014

Contributor

LGTM, but to merge it in master branch you'll need to introduce new configuration option.

Contributor

indutny commented Feb 13, 2014

LGTM, but to merge it in master branch you'll need to introduce new configuration option.

@indutny

This comment has been minimized.

Show comment Hide comment
@indutny

indutny Feb 13, 2014

Contributor

Almost finished working on NPN config option, will push to this branch

Contributor

indutny commented Feb 13, 2014

Almost finished working on NPN config option, will push to this branch

@indutny

This comment has been minimized.

Show comment Hide comment
@indutny

indutny Feb 13, 2014

Contributor

@georgekola pushed, please review.

Contributor

indutny commented Feb 13, 2014

@georgekola pushed, please review.

@georgekola

This comment has been minimized.

Show comment Hide comment
@georgekola

georgekola Feb 23, 2014

Contributor

Looks Good!

Contributor

georgekola commented Feb 23, 2014

Looks Good!

georgekola added a commit that referenced this pull request Feb 23, 2014

Merge pull request #10 from Voxer/test/npn-support
DRAFT: Add NPN support to stud

@georgekola georgekola merged commit 26776f4 into master Feb 23, 2014

@georgekola georgekola deleted the test/npn-support branch Feb 23, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment