From: Eric Wong <normalperson@...>
Date: 2015-07-15T03:03:41+00:00
Subject: [ruby-core:69975] Re: [Ruby trunk - Feature #9390] Support for the ALPN TLS extension

tenderlove@ruby-lang.org wrote:
> +ssl_alpn_select_cb(SSL *ssl, const unsigned char **out, unsigned char *outlen, const unsigned char *in, unsigned int inlen, void *arg)
> +{
> +    int i = 0;
> +    VALUE sslctx_obj, cb, protocols, selected;
> +
> +    sslctx_obj = (VALUE) arg;
> +    cb = rb_iv_get(sslctx_obj, "@alpn_select_cb");
> +    protocols = rb_ary_new();
> +
> +    /* The format is len_1|proto_1|...|len_n|proto_n\0 */
> +    while (in[i]) {
> +	VALUE protocol = rb_str_new((const char *) &in[i + 1], in[i]);
> +	rb_ary_push(protocols, protocol);
> +	i += in[i] + 1;
> +    }
> +
> +    selected = rb_funcall(cb, rb_intern("call"), 1, protocols);
> +    StringValue(selected);
> +    *out = (unsigned char *) StringValuePtr(selected);
> +    *outlen = RSTRING_LENINT(selected);

I think we need to keep `selected' markable by GC as long as anything
may use `out'.  Otherwise `out' can refer to a freed region.

Perhaps add the following here:

   rb_iv_set(sslctx_obj, "@_alpn_selected", selected);

Side note: StringValue is redundant if using StringValuePtr

Haven't looked at the rest closely, but that jumped out at me.