code review 5590045: crypto/x509: use case-insensitive hostname matching.

crypto/x509: use case-insensitive hostname matching.

Fixes issue 2792.

[agl1, 2012-01-27 18:11:43]

Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com),

I'd like you to review this change to
https://go.googlecode.com/hg/

[r, 2012-01-27 18:15:59]

http://codereview.appspot.com/5590045/diff/4/src/pkg/crypto/x509/verify.go
File src/pkg/crypto/x509/verify.go (right):

http://codereview.appspot.com/5590045/diff/4/src/pkg/crypto/x509/verify.go#ne...
src/pkg/crypto/x509/verify.go:234: if (c >= 'A' && c <= 'Z') || c > 127 {
this is hard to read. the tests change direction and i think the last > should
be >=.
moreover, what the test does is not what the name says: it asks if there are
upper case letters or high bits set.
what are you trying to do?

[agl1, 2012-01-31 00:05:18]

http://codereview.appspot.com/5590045/diff/4/src/pkg/crypto/x509/verify.go
File src/pkg/crypto/x509/verify.go (right):

http://codereview.appspot.com/5590045/diff/4/src/pkg/crypto/x509/verify.go#ne...
src/pkg/crypto/x509/verify.go:234: if (c >= 'A' && c <= 'Z') || c > 127 {
On 2012/01/27 18:15:59, r wrote:
> what are you trying to do?

I've renamed things a bit and I've also switched to testing just for RuneError
rather than any non-ASCII value.

[r, 2012-01-31 00:23:09]

LGTM

http://codereview.appspot.com/5590045/diff/5001/src/pkg/crypto/x509/verify.go
File src/pkg/crypto/x509/verify.go (right):

http://codereview.appspot.com/5590045/diff/5001/src/pkg/crypto/x509/verify.go...
src/pkg/crypto/x509/verify.go:229: // toLowerCaseASCII returns a lowercase
version of in. See RFC 6125 6.4.1. We use
s/lowercase/lower-case/

http://codereview.appspot.com/5590045/diff/5001/src/pkg/crypto/x509/verify.go...
src/pkg/crypto/x509/verify.go:242: if c >= 'A' && c <= 'Z' {
if 'A' <= c && c <= 'Z' {

http://codereview.appspot.com/5590045/diff/5001/src/pkg/crypto/x509/verify.go...
src/pkg/crypto/x509/verify.go:255: out[i] += 32
s/32/'a' -'A'/

[agl1, 2012-01-31 16:00:23]

*** Submitted as http://code.google.com/p/go/source/detail?r=c66dbb0415cc ***

crypto/x509: use case-insensitive hostname matching.

Fixes issue 2792.

R=golang-dev, r
CC=golang-dev
http://codereview.appspot.com/5590045

http://codereview.appspot.com/5590045/diff/5001/src/pkg/crypto/x509/verify.go
File src/pkg/crypto/x509/verify.go (right):

http://codereview.appspot.com/5590045/diff/5001/src/pkg/crypto/x509/verify.go...
src/pkg/crypto/x509/verify.go:229: // toLowerCaseASCII returns a lowercase
version of in. See RFC 6125 6.4.1. We use
On 2012/01/31 00:23:09, r wrote:
> s/lowercase/lower-case/

Done.

http://codereview.appspot.com/5590045/diff/5001/src/pkg/crypto/x509/verify.go...
src/pkg/crypto/x509/verify.go:242: if c >= 'A' && c <= 'Z' {
On 2012/01/31 00:23:09, r wrote:
> if 'A' <= c && c <= 'Z' {

Done.

http://codereview.appspot.com/5590045/diff/5001/src/pkg/crypto/x509/verify.go...
src/pkg/crypto/x509/verify.go:255: out[i] += 32
On 2012/01/31 00:23:09, r wrote:
> s/32/'a' -'A'/

Done.

[agl1, 2012-03-03 01:03:39]

*** Abandoned ***