net: rework the NPN patch.

net/third_party/nss/ssl/ssl.h

Index: net/third_party/nss/ssl/ssl.h
diff --git a/net/third_party/nss/ssl/ssl.h b/net/third_party/nss/ssl/ssl.h
index 03535f3c6a940af276bd75b18a6841955c60c47a..27b7e2ccc4b7ebc93f32ba96e312e2a784d8fc9d 100644
--- a/net/third_party/nss/ssl/ssl.h
+++ b/net/third_party/nss/ssl/ssl.h
@@ -157,14 +157,51 @@ SSL_IMPORT SECStatus SSL_OptionSetDefault(PRInt32 option, PRBool on);
 SSL_IMPORT SECStatus SSL_OptionGetDefault(PRInt32 option, PRBool *on);
 SSL_IMPORT SECStatus SSL_CertDBHandleSet(PRFileDesc *fd, CERTCertDBHandle *dbHandle);
 
+/* SSLNextProtoCallback is called, during the handshake, when the server has
+ * sent a Next Protocol Negotiation extension. |protos| and |protosLen| define
+ * a buffer which contains the server's advertisement. This data is guaranteed
+ * to be well formed per the NPN spec. |protoOut| is a buffer of length 255
+ * (the maximum allowed by the protocol) which, on successful return, must

[wtc, 2011/10/18 00:58:08]

Nit: I think "will" or "shall" sounds better than "must".
"must" seems to imply something the caller needs to do, but
here we're describing what the function does.

We should clarify that |protoOut| is a buffer provided by
the caller, and that on successful return, |*protoOutLen|
contains the number of bytes the function copied to the
|protoOut| buffer.

[agl, 2011/10/18 16:44:43]

Done.

+ * contain the protocol to be announced to the server. */
+typedef SECStatus (PR_CALLBACK *SSLNextProtoCallback)(
+    void *arg,
+    PRFileDesc *fd,
+    const unsigned char* protos,
+    unsigned int protosLen,
+    unsigned char* protoOut,
+    unsigned int* protoOutLen);
+
+/* SSL_SetNextProtoCallback sets a callback function to handle Next Protocol
+ * Negotiation. It causes a client to advertise NPN. */
+SSL_IMPORT SECStatus SSL_SetNextProtoCallback(PRFileDesc *fd,
+                                              SSLNextProtoCallback callback,
+                                              void *arg);
+
+/* SSL_SetNextProtoNego can be used as an alternative to
+ * SSL_SetNextProtoCallback. It also causes a client to advertise NPN and
+ * installs a default callback function which selects the first supported
+ * protocol in server-preference order. Otherwise it selects the first

[wtc, 2011/10/18 00:58:08]

Nit: it's not clear what "Otherwise" means here.  I guess you
meant "If the server's Next Protocol Negotiation extension is
empty".

[agl, 2011/10/18 16:44:43]

Done.

+ * supported protocol.
+ *
+ * The supported protocols are specified in |data| in wire-format (8-bit
+ * length-prefixed). For example: "\010http/1.1\006spdy/2". */
 SSL_IMPORT SECStatus SSL_SetNextProtoNego(PRFileDesc *fd,
 					  const unsigned char *data,
-					  unsigned short length);
+					  unsigned int length);
+/* SSL_GetNextProto can be used after a handshake on a socket where
+ * SSL_SetNextProtoNego was called to retrieve the result of the Next Protocol
+ * negotiation.
+ *
+ * state is set to one of the SSL_NEXT_PROTO_* constants. The negotiated
+ * protocol, if any, is written into buf, which must be at least buf_len bytes
+ * long. If the negotiated protocol is longer than this, it is truncated.  The
+ * number of bytes copied is written into length. */

[wtc, 2011/10/18 00:58:08]

Nit: length => *length

[agl, 2011/10/18 16:44:43]

Done.

 SSL_IMPORT SECStatus SSL_GetNextProto(PRFileDesc *fd,
 				      int *state,
 				      unsigned char *buf,
 				      unsigned *length,
 				      unsigned buf_len);

[wtc, 2011/10/18 00:58:08]

Nit: use "unsigned int" instead of just "unsigned" for
|length| and |buf_len|.

[agl, 2011/10/18 16:44:43]

Done.

+
 #define SSL_NEXT_PROTO_NO_SUPPORT	0 /* No peer support                */
 #define SSL_NEXT_PROTO_NEGOTIATED	1 /* Mutual agreement               */
 #define SSL_NEXT_PROTO_NO_OVERLAP	2 /* No protocol overlap found      */

[wtc, 2011/10/18 00:58:08]

It may be a good idea to define these as an enum type.  You
can just add a TODO comment about this.

[agl, 2011/10/18 16:44:43]

I've added a TODO because, at the moment, we use one of these defines as the
#ifdef to figure out if the NSS that we're compiling against supports NPN.