Properly parse UTF8Strings in certificates on Windows.

Properly parse UTF8Strings in certificates on Windows.

BUG=114168
TEST=https://www.verisign.co.jp appears correctly regardless of system
locale. Additionally, net_unittests:X509TypesTest* should cover this.

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=122053

[Ryan Sleevi, 2012-02-15 00:39:22]

wtc: For your review. It's been requested to be M17 mergable.

kenji: FYI

[wtc, 2012-02-15 01:34:41]

Patch Set 1 LGTM.  Just some nits and questions.  Thanks.

http://codereview.chromium.org/9358080/diff/1/crypto/capi_util.h
File crypto/capi_util.h (right):

http://codereview.chromium.org/9358080/diff/1/crypto/capi_util.h#newcode36
crypto/capi_util.h:36: CRYPTO_EXPORT void WINAPI CryptFreeFunction(void* p);

Nit: omit "Function" in the function names.

http://codereview.chromium.org/9358080/diff/1/net/base/x509_cert_types_win.cc
File net/base/x509_cert_types_win.cc (right):

http://codereview.chromium.org/9358080/diff/1/net/base/x509_cert_types_win.cc...
net/base/x509_cert_types_win.cc:7: #include <string>

Nit: <string> is already included by "net/base/x509_cert_types.h".

http://codereview.chromium.org/9358080/diff/1/net/base/x509_cert_types_win.cc...
net/base/x509_cert_types_win.cc:16: #include <wincrypt.h>

Nit: the C system headers should be listed after
"net/base/x509_cert_types.h".

http://codereview.chromium.org/9358080/diff/1/net/base/x509_cert_types_win.cc...
net/base/x509_cert_types_win.cc:36: // Converts the value for |attribute| to an
ASCII string, storing the result

Typo: ASCII => UTF-8

http://codereview.chromium.org/9358080/diff/1/net/base/x509_cert_types_win.cc...
net/base/x509_cert_types_win.cc:57: wide_name.resize(chars_written - 1);

Why isn't the equivalent of this line in the original code?
I guess the null character in the std::string doesn't really
hurt when we display it?

http://codereview.chromium.org/9358080/diff/1/net/base/x509_cert_types_win.cc...
net/base/x509_cert_types_win.cc:87: }

Nit: add a
    // namespace
comment

http://codereview.chromium.org/9358080/diff/1/net/base/x509_cert_types_win.cc...
net/base/x509_cert_types_win.cc:120: &(this->domain_components)

I assume you use this-> for clarity.

Nit: you can omit the parentheses after '&'.  We omit those
parentheses in this kind of expression elsewhere, for example,
on lines 133-136 below.

[Ryan Sleevi, 2012-02-15 02:36:55]

http://codereview.chromium.org/9358080/diff/1/net/base/x509_cert_types_win.cc
File net/base/x509_cert_types_win.cc (right):

http://codereview.chromium.org/9358080/diff/1/net/base/x509_cert_types_win.cc...
net/base/x509_cert_types_win.cc:57: wide_name.resize(chars_written - 1);
On 2012/02/15 01:34:41, wtc wrote:
> 
> Why isn't the equivalent of this line in the original code?
> I guess the null character in the std::string doesn't really
> hurt when we display it?

Correct. The unit tests correctly pointed out that without this line, the full
string was effectively double-null terminated.

[commit-bot: I haz the power, 2012-02-15 02:37:12]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/[email protected]/9358080/5001

[commit-bot: I haz the power, 2012-02-15 02:37:17]

Presubmit check for 9358080-5001 failed and returned exit status 1.

Running presubmit commit checks ...

** Presubmit Warnings **
Found lines longer than 80 characters (first 5 shown).
  net/base/x509_cert_types_unittest.cc, line 21, 97 chars \
  net/base/x509_cert_types_unittest.cc, line 45, 85 chars \
  net/base/x509_cert_types_unittest.cc, line 49, 83 chars \
  net/base/x509_cert_types_unittest.cc, line 87, 96 chars \
  net/base/x509_cert_types_unittest.cc, line 109, 105 chars

New code should not use wstrings.  If you are calling an API that accepts a
wstring, fix the API.
    net/base/x509_cert_types_win.cc:50

Presubmit checks took 1.1s to calculate.

Was the presubmit check useful? Please send feedback & hate mail to
[email protected]!