tatsuhiro-t / spdylay

The experimental SPDY protocol version 2, 3 and 3.1 implementation in C
http://tatsuhiro-t.github.io/spdylay/
MIT License
603 stars 102 forks source link

Minor .gitignore update for OSX. #6

Closed sorced-jim closed 12 years ago

tatsuhiro-t commented 12 years ago

Thank you. I merged your changes. There were some conflicts, so I made manual merge. Also I made some changes: see 876c33c562

sorced-jim commented 12 years ago

Thanks for pulling in the non changes. Are you sure you only want to set out and outlen if you aren't selecting spdy. This means that the npn callback helper in spdylay isn't enough to negotiate http/1.1 (the most common fallback). Do do that myself, I'll need to first call the npn callback, then check for http.

What do you think of the following ideas to make falling back to http easier: 1) Add a second callback that checks for http/1.X 2) Pass in a bit that says to allow out and outlen to be set to http/1.X. 3) Return 0 for spdy and set out to spdy/2 and return the last protocol if no spdy protocols match. 4) Don't set out and *outlen unless spdy/2 matches.

I'm pretty certain at least 4 is needed. One possible transition path for Google is to change their NPN string to spdy/3,http/1.1 . With spdylay as is, my code will see -1 as the spdy version and try http/1.1. However, the server will see that spdylay helped negotiate spdy/2 and close the connection.

On Fri, Feb 3, 2012 at 7:00 AM, Tatsuhiro Tsujikawa < reply@reply.github.com

wrote:

Thank you. I merged your changes. There were some conflicts, so I made manual merge. Also I made some changes: see 876c33c562


Reply to this email directly or view it on GitHub: https://github.com/tatsuhiro-t/spdylay/pull/6#issuecomment-3797885

Jim https://twitter.com/#!/sorced_eng

tatsuhiro-t commented 12 years ago

Hi,

2012年2月8日1:42 sorced-jim reply@reply.github.com:

Thanks for pulling in the non changes. Are you sure you only want to set out and outlen if you aren't selecting spdy. This means that the npn callback helper in spdylay isn't enough to negotiate http/1.1 (the most common fallback). Do do that myself, I'll need to first call the npn callback, then check for http.

The intention to always set spdy/2 is that it is intended to use for spdylay, and it only speaks spdy/2. Also spdycat is not capable of http/1.1. NPN spec also says that client can choose anything.

But I am OK to extend this function to make it more generic.

What do you think of the following ideas to make falling back to http easier: 1) Add a second callback that checks for http/1.X 2) Pass in a bit that says to allow out and outlen to be set to http/1.X. 3) Return 0 for spdy and set out to spdy/2 and return the last protocol if no spdy protocols match. 4) Don't set out and *outlen unless spdy/2 matches.

I'm pretty certain at least 4 is needed. One possible transition path for Google is to change their NPN string to spdy/3,http/1.1 . With spdylay as is, my code will see -1 as the spdy version and try http/1.1. However, the server will see that spdylay helped negotiate spdy/2 and close the connection.

I am OK with 4). But for more generic solution is 2) + list of preference of protocol. Chromium does this. We can add new argument which is a list of protocol the caller wants to use in preference order. And borrow the algorithm from chromium: If there is overlap of server's protocol and client's protocol, use most preferable one of them and we are done and return 0. If there is no overlap, then select client's most preferable protocol and return -1.

With this algorithm, you can pass "spdy/2" and "http/1.1" in this order and negotiate the protocol. It seems that last protocol is least preferable protocol, so selecting it is not good I think.

What do you think about this?

Best regards,

Tatsuhiro Tsujikawa

On Fri, Feb 3, 2012 at 7:00 AM, Tatsuhiro Tsujikawa < reply@reply.github.com

wrote:

Thank you. I merged your changes. There were some conflicts, so I made manual merge. Also I made some changes: see 876c33c562


Reply to this email directly or view it on GitHub: https://github.com/tatsuhiro-t/spdylay/pull/6#issuecomment-3797885

Jim https://twitter.com/#!/sorced_eng


Reply to this email directly or view it on GitHub: https://github.com/tatsuhiro-t/spdylay/pull/6#issuecomment-3851319

sorced-jim commented 12 years ago

I have two issues with this proposal : 1) If spdylay changes from spdy 2 to 3, I'll need to update my call into the spdy library to spdy/3.

2) After having to pass spdy/2,http/1.1, I need to check *out to see what matched. I think if you end up passing in a list of protocols, I'm still going to write a wrapper around that, that returns an int. I'll probably do -1 for no match, 0 for http, and X for some version of spdy that spdylay understands.

As we've said, at least (2) should be done for now.

On Feb 7, 2012 10:25 AM, "Tatsuhiro Tsujikawa" < reply@reply.github.com> wrote:

Hi,

2012年2月8日1:42 sorced-jim reply@reply.github.com:

Thanks for pulling in the non changes. Are you sure you only want to set out and outlen if you aren't selecting spdy. This means that the npn callback helper in spdylay isn't enough to negotiate http/1.1 (the most common fallback). Do do that myself, I'll need to first call the npn callback, then check for http.

The intention to always set spdy/2 is that it is intended to use for spdylay, and it only speaks spdy/2. Also spdycat is not capable of http/1.1. NPN spec also says that client can choose anything.

But I am OK to extend this function to make it more generic.

What do you think of the following ideas to make falling back to http easier: 1) Add a second callback that checks for http/1.X 2) Pass in a bit that says to allow out and outlen to be set to http/1.X. 3) Return 0 for spdy and set out to spdy/2 and return the last protocol if no spdy protocols match. 4) Don't set out and *outlen unless spdy/2 matches.

I'm pretty certain at least 4 is needed. One possible transition path for Google is to change their NPN string to spdy/3,http/1.1 . With spdylay as is, my code will see -1 as the spdy version and try http/1.1. However, the server will see that spdylay helped negotiate spdy/2 and close the connection.

I am OK with 4). But for more generic solution is 2) + list of preference of protocol. Chromium does this. We can add new argument which is a list of protocol the caller wants to use in preference order. And borrow the algorithm from chromium: If there is overlap of server's protocol and client's protocol, use most preferable one of them and we are done and return 0. If there is no overlap, then select client's most preferable protocol and return -1.

With this algorithm, you can pass "spdy/2" and "http/1.1" in this order and negotiate the protocol. It seems that last protocol is least preferable protocol, so selecting it is not good I think.

What do you think about this?

Best regards,

Tatsuhiro Tsujikawa

On Fri, Feb 3, 2012 at 7:00 AM, Tatsuhiro Tsujikawa < reply@reply.github.com

wrote:

Thank you. I merged your changes. There were some conflicts, so I made manual merge. Also I made some changes: see 876c33c562


Reply to this email directly or view it on GitHub: https://github.com/tatsuhiro-t/spdylay/pull/6#issuecomment-3797885

Jim https://twitter.com/#!/sorced_eng


Reply to this email directly or view it on GitHub: https://github.com/tatsuhiro-t/spdylay/pull/6#issuecomment-3851319


Reply to this email directly or view it on GitHub: https://github.com/tatsuhiro-t/spdylay/pull/6#issuecomment-3853236

tatsuhiro-t commented 12 years ago

2012/02/08 7:22 "sorced-jim" < reply@reply.github.com

:

I have two issues with this proposal : 1) If spdylay changes from spdy 2 to 3, I'll need to update my call into the spdy library to spdy/3.

Right.

2) After having to pass spdy/2,http/1.1, I need to check *out to see what matched. I think if you end up passing in a list of protocols, I'm still going to write a wrapper around that, that returns an int. I'll probably do -1 for no match, 0 for http, and X for some version of spdy that spdylay understands.

As we've said, at least (2) should be done for now.

I agree that the function selects the spdy protocol spdylay supports and select http/1.1 as a fallback if server advertise it. I am also ok to return -1 for no match and 0 for http/1.1. I am concerend about returning spdy version number as int. While it seems ok at the moment (only possible versions we can see is spdy/2 or 3), but future is not certain. This is because npn is really just a string. They can add sny optional extensions as string. I think if we make spdylay do all spdy stuff, we can just return 1 for spdy. This makes application code less dependent on spdy version.

Best regards,

Tatsuhiro Tsujikawa

On Feb 7, 2012 10:25 AM, "Tatsuhiro Tsujikawa" < reply@reply.github.com> wrote:

Hi,

2012$BG/(B2$B7n(B8$BF|(B1:42 sorced-jim reply@reply.github.com:

Thanks for pulling in the non changes. Are you sure you only want to set out and outlen if you aren't selecting spdy. This means that the npn callback helper in spdylay isn't enough to negotiate http/1.1 (the most common fallback). Do do that myself, I'll need to first call the npn callback, then check for http.

The intention to always set spdy/2 is that it is intended to use for spdylay, and it only speaks spdy/2. Also spdycat is not capable of http/1.1. NPN spec also says that client can choose anything.

But I am OK to extend this function to make it more generic.

What do you think of the following ideas to make falling back to http easier: 1) Add a second callback that checks for http/1.X 2) Pass in a bit that says to allow out and outlen to be set to http/1.X. 3) Return 0 for spdy and set out to spdy/2 and return the last protocol if no spdy protocols match. 4) Don't set out and *outlen unless spdy/2 matches.

I'm pretty certain at least 4 is needed. One possible transition path for Google is to change their NPN string to spdy/3,http/1.1 . With spdylay as is, my code will see -1 as the spdy version and try http/1.1. However, the server will see that spdylay helped negotiate spdy/2 and close the connection.

I am OK with 4). But for more generic solution is 2) + list of preference of protocol. Chromium does this. We can add new argument which is a list of protocol the caller wants to use in preference order. And borrow the algorithm from chromium: If there is overlap of server's protocol and client's protocol, use most preferable one of them and we are done and return 0. If there is no overlap, then select client's most preferable protocol and return -1.

With this algorithm, you can pass "spdy/2" and "http/1.1" in this order and negotiate the protocol. It seems that last protocol is least preferable protocol, so selecting it is not good I think.

What do you think about this?

Best regards,

Tatsuhiro Tsujikawa

On Fri, Feb 3, 2012 at 7:00 AM, Tatsuhiro Tsujikawa < reply@reply.github.com

wrote:

Thank you. I merged your changes. There were some conflicts, so I made manual merge. Also I made some changes: see 876c33c562


Reply to this email directly or view it on GitHub: https://github.com/tatsuhiro-t/spdylay/pull/6#issuecomment-3797885

Jim https://twitter.com/#!/sorced_eng


Reply to this email directly or view it on GitHub: https://github.com/tatsuhiro-t/spdylay/pull/6#issuecomment-3851319


Reply to this email directly or view it on GitHub: https://github.com/tatsuhiro-t/spdylay/pull/6#issuecomment-3853236


Reply to this email directly or view it on GitHub: https://github.com/tatsuhiro-t/spdylay/pull/6#issuecomment-3858133

sorced-jim commented 12 years ago

1 for spdy, 0 for http, and -1 for anything else works for me.

On Tue, Feb 7, 2012 at 6:18 PM, Tatsuhiro Tsujikawa < reply@reply.github.com

wrote:

2012/02/08 7:22 "sorced-jim" < reply@reply.github.com

:

I have two issues with this proposal : 1) If spdylay changes from spdy 2 to 3, I'll need to update my call into the spdy library to spdy/3.

Right.

2) After having to pass spdy/2,http/1.1, I need to check *out to see what matched. I think if you end up passing in a list of protocols, I'm still going to write a wrapper around that, that returns an int. I'll probably do -1 for no match, 0 for http, and X for some version of spdy that spdylay understands.

As we've said, at least (2) should be done for now.

I agree that the function selects the spdy protocol spdylay supports and select http/1.1 as a fallback if server advertise it. I am also ok to return -1 for no match and 0 for http/1.1. I am concerend about returning spdy version number as int. While it seems ok at the moment (only possible versions we can see is spdy/2 or 3), but future is not certain. This is because npn is really just a string. They can add sny optional extensions as string. I think if we make spdylay do all spdy stuff, we can just return 1 for spdy. This makes application code less dependent on spdy version.

Best regards,

Tatsuhiro Tsujikawa

On Feb 7, 2012 10:25 AM, "Tatsuhiro Tsujikawa" < reply@reply.github.com> wrote:

Hi,

2012 $BG/ (B2 $B7n (B8 $BF| (B1:42 sorced-jim reply@reply.github.com:

Thanks for pulling in the non changes. Are you sure you only want to set out and outlen if you aren't selecting spdy. This means that the npn callback helper in spdylay isn't enough to negotiate http/1.1 (the most common fallback). Do do that myself, I'll need to first call the npn callback, then check for http.

The intention to always set spdy/2 is that it is intended to use for spdylay, and it only speaks spdy/2. Also spdycat is not capable of http/1.1. NPN spec also says that client can choose anything.

But I am OK to extend this function to make it more generic.

What do you think of the following ideas to make falling back to http easier: 1) Add a second callback that checks for http/1.X 2) Pass in a bit that says to allow out and outlen to be set to http/1.X. 3) Return 0 for spdy and set out to spdy/2 and return the last protocol if no spdy protocols match. 4) Don't set out and *outlen unless spdy/2 matches.

I'm pretty certain at least 4 is needed. One possible transition path for Google is to change their NPN string to spdy/3,http/1.1 . With spdylay as is, my code will see -1 as the spdy version and try http/1.1. However, the server will see that spdylay helped negotiate spdy/2 and close the connection.

I am OK with 4). But for more generic solution is 2) + list of preference of protocol. Chromium does this. We can add new argument which is a list of protocol the caller wants to use in preference order. And borrow the algorithm from chromium: If there is overlap of server's protocol and client's protocol, use most preferable one of them and we are done and return 0. If there is no overlap, then select client's most preferable protocol and return -1.

With this algorithm, you can pass "spdy/2" and "http/1.1" in this order and negotiate the protocol. It seems that last protocol is least preferable protocol, so selecting it is not good I think.

What do you think about this?

Best regards,

Tatsuhiro Tsujikawa

On Fri, Feb 3, 2012 at 7:00 AM, Tatsuhiro Tsujikawa < reply@reply.github.com

wrote:

Thank you. I merged your changes. There were some conflicts, so I made manual merge. Also I made some changes: see 876c33c562


Reply to this email directly or view it on GitHub: https://github.com/tatsuhiro-t/spdylay/pull/6#issuecomment-3797885

Jim https://twitter.com/#!/sorced_eng


Reply to this email directly or view it on GitHub: https://github.com/tatsuhiro-t/spdylay/pull/6#issuecomment-3851319


Reply to this email directly or view it on GitHub: https://github.com/tatsuhiro-t/spdylay/pull/6#issuecomment-3853236


Reply to this email directly or view it on GitHub: https://github.com/tatsuhiro-t/spdylay/pull/6#issuecomment-3858133


Reply to this email directly or view it on GitHub: https://github.com/tatsuhiro-t/spdylay/pull/6#issuecomment-3861277

Jim https://twitter.com/#!/sorced_eng

tatsuhiro-t commented 12 years ago

OK, I pushed changes b870025 Please comment here if the commit does not look good.

sorced-jim commented 12 years ago

I'm pretty certain you shouldn't write "spdy/2" when you return -1. Otherwise, the change looks good.

On Wed, Feb 8, 2012 at 4:25 AM, Tatsuhiro Tsujikawa < reply@reply.github.com

wrote:

OK, I pushed changes b870025 Please comment here if the commit does not look good.


Reply to this email directly or view it on GitHub: https://github.com/tatsuhiro-t/spdylay/pull/6#issuecomment-3866748

Jim https://twitter.com/#!/sorced_eng

tatsuhiro-t commented 12 years ago

2012/02/09 3:53 "sorced-jim" < reply@reply.github.com

:

I'm pretty certain you shouldn't write "spdy/2" when you return -1. Otherwise, the change looks good.

Ok, I'll fix it.

Best regards,

Tatsuhiro Tsujikawa

On Wed, Feb 8, 2012 at 4:25 AM, Tatsuhiro Tsujikawa < reply@reply.github.com

wrote:

OK, I pushed changes b870025 Please comment here if the commit does not look good.


Reply to this email directly or view it on GitHub: https://github.com/tatsuhiro-t/spdylay/pull/6#issuecomment-3866748

Jim https://twitter.com/#!/sorced_eng


Reply to this email directly or view it on GitHub: https://github.com/tatsuhiro-t/spdylay/pull/6#issuecomment-3873475

tatsuhiro-t commented 12 years ago

Fixed. 8fac259

sorced-jim commented 12 years ago

I have to bring this up again. I think the way of getting the version is backwards. We have to parse the NPN string twice to get the spdy version. Why don't we simply return the spdy version from the ssl npn handler instead of having the client save the npn string, then parse it again with spdylay_npn_get_version.

On Thu, Feb 9, 2012 at 5:48 AM, Tatsuhiro Tsujikawa < reply@reply.github.com

wrote:

Fixed. 8fac259


Reply to this email directly or view it on GitHub: https://github.com/tatsuhiro-t/spdylay/pull/6#issuecomment-3887522

Jim https://twitter.com/#!/sorced_eng

tatsuhiro-t commented 12 years ago

I agree with you. Fixed in 8fd2fab