On Thu, Oct 29, 2015 at 4:53 PM, Bert Huijben <b...@qqmail.nl> wrote:
>> 1. In response_buckets.c, when Content-Length and Transfer-Encoding
>> are both present serf goes with Content-Length. The HTTP spec says
>> Transfer-Encoding takes precedence here, and our experience with
>> servers has been that if both are present then Transfer-Encoding is
>> what we should actually pay attention to. We patched this to change:
>>
>> ...
>
> I think we should apply this change. I recently noticed this in the updated
> HTTP/1.1 specs too.
>
Great!
>> 2. In response_buckets.c serf automatically uncompresses responses,
>> but PageSpeed would rather receive them compressed if they're served
>> that way. Currently we just remove the block:
>>
>> v = serf_bucket_headers_get(ctx->headers, "Content-Encoding");
>> if (v) {
>> /* Need to handle multiple content-encoding. */
>> if (v && strcasecmp("gzip", v) == 0) {
>> ctx->body =
>> serf_bucket_deflate_create(ctx->body, bkt->allocator,
>> SERF_DEFLATE_GZIP);
>> }
>> else if (v && strcasecmp("deflate", v) == 0) {
>> ctx->body =
>> serf_bucket_deflate_create(ctx->body, bkt->allocator,
>> SERF_DEFLATE_DEFLATE);
>> }
>> }
>>
>> Ideally, though, whether to uncompress would be a serf setting.
>
> A new setting can't be backported to 1.3.x, but I don't see why this can't be
> configurable in 1.4+
That would be great!
>> 3. We want to be able to check if a connection had error events
>> reported to it during the last call to serf_context_run so that we can
>> manually close it and open a new connection. We added a new API call
>> to outgoing.c to give this information:
>>
>> int serf_connection_is_in_error_state(serf_connection_t* conn)
>> {
>> return ((conn->seen_in_pollset & (APR_POLLERR | APR_POLLHUP)) !=
>> 0);
>> }
>
> I don't think this is really going to work. There are more cases where a
> connection is in an error state than just these events. Some platforms report
> connection errors in a different way...
>
> Why can't you use serf's own connection resetting? In Subversion we just rely
> on Serf resetting connections as needed.
We added this back in May 2011 to fix a problem where serf would spew
dozens of errors each time it got a connection refused. We may not
need it anymore. I'll check.
>
>
>> 4. If you're a CDN, one setup is for someone to CNAME www.example.com
>> to you and point ref.example.com at their origin. Then you want to
>> fetch from ref.example.com but you need to send "www.example.com" as
>> the HOST header. So we'd like to set the HOST header separately on
>> requests. We added serf_request_bucket_request_create_for_host for
>> this:
>
> I'm not entirely sure what you try to accomplish here.
Let's say you run something CDN like. People CNAME their pages to
you, and tell you where you can find their origin. In this case
there's www.example.com (cnamed to you) and ref.example.com (the
origin). A request for www.example.com comes in, you don't have it in
cache, so you need to fetch it from origin. You need to send a
request to the server at ref.example.com but that server believes it's
name is www.example.com and might not serve from the right vhost if it
gets a request for ref.example.com. So you want to connect to:
IP: look up "ref.example.com" in DNS
Host: "www.example.com"
>
> Do you want to send requests to a different host than that you connected
> to... or explicitly send a 'wrong' host?
>
Explicitly send the wrong host. That is, use one host string to look
up the IP and another host strong to send in the Host header.
> I would guess that you can just change 'Host' in the setup or request handler
> of the request?
> Have you tried that approach?
I don't think so? I'll need to check.
>
>>
>> 5. We want to be able to set a custom certificates directory or file,
>> so we added an API to ssl_buckets.c for that:
>>
>> +apr_status_t serf_ssl_set_certificates_directory(serf_ssl_context_t
>> *ssl_ctx,
>> + const char* path)
>> +{
>> + X509_STORE *store = SSL_CTX_get_cert_store(ssl_ctx->ctx);
>> + int result = X509_STORE_load_locations(store, NULL, path);
>> + return result ? APR_SUCCESS : APR_EGENERAL;
>> +}
>> +
>> +apr_status_t serf_ssl_set_certificates_file(serf_ssl_context_t
>> *ssl_ctx,
>> + const char* file)
>> +{
>> + X509_STORE *store = SSL_CTX_get_cert_store(ssl_ctx->ctx);
>> + int result = X509_STORE_load_locations(store, file, NULL);
>> + return result ? APR_SUCCESS : APR_EGENERAL;
>> +}
>
> I'm not sure if this passes the file and path in the right encoding. In
> general serf follows apr, which on Windows has a UTF-8 pathstyle, while
> openssl probably just passes the path to the ANSI apis on Windows.
>
> We have a serf_ssl_load_cert_file() on trunk that works around these
> limitations (and certain openssl linkage problems). We should probably use
> similar code for these functions.
It sounds like on trunk we would have serf_ssl_load_cert_file() which
we could use here, and then we could manually call it on all the files
in the specified directory.
>
>> 6. We null out ctx->ssl_ctx after it's free'd to help make sure it
>> doesn't get reused:
>>
>> if (!--ctx->ssl_ctx->refcount) {
>> ssl_free_context(ctx->ssl_ctx);
>> + ctx->ssl_ctx = NULL;
>> }
>
