[Cronet-iOS] Public-Key-Pinning Tests

components/cronet/ios/Cronet.h

Index: components/cronet/ios/Cronet.h
diff --git a/components/cronet/ios/Cronet.h b/components/cronet/ios/Cronet.h
index f7f6cbe13fc94214cf0095bde6f6cedd5d330432..cd2980c8d45a1cfd0c369d750a88ee3dcb88a1cb 100644
--- a/components/cronet/ios/Cronet.h
+++ b/components/cronet/ios/Cronet.h
@@ -73,6 +73,47 @@ GRPC_SUPPORT_EXPORT
 // captures. This method only has any effect before |start| is called.
 + (void)setSslKeyLogFileName:(NSString*)sslKeyLogFileName;
 
+// <p>

[lilyhoughton, 2017/06/09 16:51:57]

Can the html be replaced with something more like markdown?  Whatever the
effects on doc formatting, it makes the source code itself slightly less
readable, I think.

[kapishnikov, 2017/06/09 21:27:01]

Done.

+// Pins a set of public keys for a given host. This method only has any effect
+// before |start| is called. By pinning a set of public keys,
+// |pinsSha256|, communication with |hostName| is required to
+// authenticate with a certificate with a public key from the set of pinned
+// ones. An app can pin the public key of the root certificate, any of the
+// intermediate certificates or the end-entry certificate. Authentication will
+// fail and secure communication will not be established if none of the public
+// keys is present in the host's certificate chain, even if the host attempts to
+// authenticate with a certificate allowed by the device's trusted store of
+// certificates.
+// </p>
+// <p>
+// Calling this method multiple times with the same host name overrides the
+// previously set pins for the host.
+// </p>
+// <p>
+// More information about the public key pinning can be found in
+// <a href="https://tools.ietf.org/html/rfc7469">RFC 7469</a>.
+// </p>
+//
+// @param hostName name of the host to which the public keys should be pinned. A

[lilyhoughton, 2017/06/09 16:51:57]

We also don't have @params for any of the other functions - should we?

[kapishnikov, 2017/06/09 21:27:01]

I think we should.

The correct way of documenting a method is actually looks like this:
/**
 <#Description#>

 @param host <#host description#>
 @param pinHashes <#pinHashes description#>
 @param includeSubdomains <#includeSubdomains description#>
 @param expirationDate <#expirationDate description#>
 */

It is also an acceptable documentation style according to the google style
guide.

Using three slashes is also possible but it usually used for a single line
documentation.
/// <#Description#>
///
/// @param host <#host description#>
/// @param pinHashes <#pinHashes description#>
/// @param includeSubdomains <#includeSubdomains description#>
/// @param expirationDate <#expirationDate description#>

Double slashes that we currently use are not considered documentation comments.

If a method has a return value, it should be documented like this:
/**
 <#Description#>

 @param fileName <#fileName description#>
 @param logBytes <#logBytes description#>
 @return <#return value description#>
 */
+ (BOOL)startNetLogToFile:(NSString*)fileName logBytes:(BOOL)logBytes;

In order to check if a method is documented correctly, you can alt+click on it.

Maybe we should revisit the documentation of other methods in a separate CL.

+// host that
+//                 consists only of digits and the dot character is treated as
+//                 invalid.
+// @param pinsSha256 a set of pins. Each pin is the SHA-256 cryptographic
+//                   hash of the DER-encoded ASN.1 representation of the Subject
+//                   Public Key Info (SPKI) of the host's X.509 certificate.
+//                   Although, the method does not mandate the presence of the
+//                   backup pin that can be used if the control of the primary
+//                   private key has been lost, it is highly recommended to
+//                   supply one.
+// @param includeSubdomains indicates whether the pinning policy should be
+// applied to
+//                          subdomains of {@code hostName}.

[lilyhoughton, 2017/06/09 16:51:57]

is {@code hostName} different from |hostName|?

[kapishnikov, 2017/06/09 21:27:01]

This is the result of copy-paste. Fixed.

+// @param expirationDate specifies the expiration date for the pins.
++ (void)addPublicKeyPinsForHost:(NSString*)host
+                      pinHashes:(NSSet<NSData*>*)pinHashes
+              includeSubdomains:(BOOL)includeSubdomains
+                 expirationDate:(NSDate*)expirationDate;
+
 // Sets the block used to determine whether or not Cronet should handle the
 // request. If the block is not set, Cronet will handle all requests. Cronet
 // retains strong reference to the block, which can be released by calling this