On 11/30/2012 04:49 AM, Amos Jeffries wrote: > On 25/11/2012 4:47 a.m., Tsantilas Christos wrote: >>>> acl aclname server_ssl_cert_fingerprint [-sha1] fingerprint1 ...
>>> * ACL name seems to be a bit on the long side. How about dropping the >>> "ssl_" sub-section out of it and changing "_fingerprint" to "_sha1"? >>> ==> server_cert_sha1 >> This is not correct. >> The -sha1 is an argument to specify the type of fingerprint. In the >> future we may add the "-md5" argument to specify an server_cert_md5 acl. >> >> I renamed the acl name to >> server_cert_fingerprint [-sha1] > Hmm. Okay. I was thinking we should have a different ACL for each > fingerprint type, that would help prevent wrong fingerprints being > entered. I'm not strongly for this though, so the above change is > accptible. I do not think we should have different ACLs for different fingerprint types. The fingerprinting algorithm is just a parameter -- all other aspects of the ACL remain the same. This is similar to having one ACL for all HTTP response headers. As for preventing typos, I do not see why accidentally typing "-sha1" instead of "-md5" is somehow more likely than typing "_sha1" instead of "_md5". >>> - In ACLCertificateData::parse() the message "required attribute >>> argument missing" does not say anything useful for fixing the problem >>> when it hits the logs. The other messages after are a bit more >>> expressive, since they at least mention 'Acl' or what the attribute >>> should be, but they all need a bit friendliness polish. >>> + something along the lines of "FATAL: Acl '"<< name? <<"' ..." >> In logs someone will see something like: >> 2012/11/24 17:38:10| FATAL: required attribute argument missing >> FATAL: Bungled squid-cert-validation.conf line 151: acl USERCERT user_cert >> >> Will understand whe wrong syntax. >> We do not have the ability inside ACLCertificateData to print acl name >> or other informations. > Administrators are commonly using automated tools now to grep for message > lines. And additionally I am hoping to remove the "bungled" completely from > Squid. > I'm okay with defering this fix for later since it affects all ACLs. Yes, we should use the existing interface until a new interface is available. It is better to be consistent, especially if we care about automated tools (that will grep for FATAL and see both messages BTW). Thank you, Alex.