More robust handling of GSSAPI error strings...

More robust handling of GSSAPI error strings

RFC 2744 does not require string types to be NULL terminated, because their length is explicitly passed as part of the gss_buffer_desc (see Section 3.2.). As such, when printing error strings, the length should be explicitly stated. This is shown in the example code of gss_display_status in Section 5.11. While in practice this is the case (at least from checking MIT and Heimdal's error handling code), it doesn't hurt to be defensive.

In addition, there are some conditions where value may be NULL or length may be 0, so make sure to check for these prior to calling StringPrintf, so as not to crash. Finally, for the extreme defensive case, make sure that the length (which is a size_t) is capped at INT_MAX prior to printing. 

Contributed by ryan.sleevi@gmail.com

BUG=33033
TEST=None

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=49299

[rsleevi-old, 2010-06-04 22:39:42]

I wasn't sure whether it was more appropriate to comment on
http://codereview.chromium.org/2268002
given that it appears to be http://codereview.chromium.org/1736009 , which was
closed and committed to trunk as Rev 48945 (
http://src.chromium.org/viewvc/chrome?view=rev&revision=48945 )

If I should have commented there, my apologies. While I cannot find
documentation giving the conditions/library that was in use, this was just
something that I'd experienced in the past when I implemented GSSAPI/Negotiate
support, and it seemed reasonable to expect that Chrom(e/ium) may as well.

[cbentzel, 2010-06-07 17:45:11]

Thanks for providing this fix!

Can you add yourself to the AUTHORS file? See
http://dev.chromium.org/developers/contributing-code for more information.

http://codereview.chromium.org/2646004/diff/1/2
File net/http/http_auth_gssapi_posix.cc (right):

http://codereview.chromium.org/2646004/diff/1/2#newcode216
net/http/http_auth_gssapi_posix.cc:216: size_t max_msg_length =
static_cast<size_t>(std::numeric_limits<int>::max());
To be safe this should probably be even more restrictive (say 4KB) and be a cap
on the length of rv, rather than the string for each call of display_status.

http://codereview.chromium.org/2646004/diff/1/2#newcode223
net/http/http_auth_gssapi_posix.cc:223: gssapi::OM_uint32 maj_stat;
Can you combine the declaration of maj_stat with the initialization on line 225?

[cbentzel, 2010-06-07 17:49:05]

On Mon, Jun 7, 2010 at 1:45 PM, <cbentzel@chromium.org> wrote:
>
>
> Can you add yourself to the AUTHORS file? See
> http://dev.chromium.org/developers/contributing-code for more information.
>
>
Disregard this, it looks like you are already in OWNERS.

[cbentzel, 2010-06-07 17:49:13]

On Mon, Jun 7, 2010 at 1:48 PM, Chris Bentzel <cbentzel@chromium.org> wrote:

>
>
> On Mon, Jun 7, 2010 at 1:45 PM, <cbentzel@chromium.org> wrote:
>>
>>
>> Can you add yourself to the AUTHORS file? See
>> http://dev.chromium.org/developers/contributing-code for more
>> information.
>>
>>
> Disregard this, it looks like you are already in OWNERS.
>

And by OWNERS I mean AUTHORS.

[rsleevi-old, 2010-06-08 01:18:33]

As I don't have committer access, if it looks good with the requested changes,
please apply on my behalf.

[cbentzel, 2010-06-08 13:11:55]

On 2010/06/08 01:18:33, rsleevi wrote:
> As I don't have committer access, if it looks good with the requested changes,
> please apply on my behalf.

LGTM. I'll commit it today.

[ahendrickson, 2010-06-08 16:30:24]

LGTM.

On 2010/06/08 13:11:55, cbentzel wrote:
> On 2010/06/08 01:18:33, rsleevi wrote:
> > As I don't have committer access, if it looks good with the requested
changes,
> > please apply on my behalf.
> 
> LGTM. I'll commit it today.

[cbentzel, 2010-06-08 18:29:20]

> > LGTM. I'll commit it today.

Given the state of the tree today, this will probably have to wait until
tomorrow. I'll still take care of it.

[wtc, 2010-06-12 00:25:33]

http://codereview.chromium.org/2646004/diff/7001/8001
File net/http/http_auth_gssapi_posix.cc (right):

http://codereview.chromium.org/2646004/diff/7001/8001#newcode225
net/http/http_auth_gssapi_posix.cc:225: gssapi::OM_uint32 min_stat;
Andy, could you rename min_stat to minor_status and
maj_stat to major_status?  The Style Guide doesn't allow
this kind of abbreviation.  Thanks.