Methods needed to get owned headers out of requests and responses

[acfoltzer]

The main case where this is currently painful is when trying to perform modifications while forwarding along an incoming-request as an outgoing-request, or incoming-response as an outgoing-response. The desired workflow is something like:

headers <- incoming.headers();
headers.delete("header-I-want-to-filter");
outgoing <- outgoing-request.new(headers);

Currently, this fails because headers is a child resource of incoming_request where the underlying headers are immutable, so methods like append and delete are not permitted. Invoking outgoing-request.new with this child resource is fine, with embedders able to clone the headers under the hood. But once it's in the outgoing resource it's once again only accessible via an immutable child resource.

As far as I can tell, the only way to modify headers in this proxying scenario is to explicitly clone the headers:

headers <- incoming.headers();
new_headers <- headers.clone();
new_headers.delete("header-I-want-to-filter");
outgoing <- outgoing-request.new(new_headers);

This isn't the end of the world, but it would be nice to avoid this clone and the implicit clone performed by constructing outgoing-request with a child resource by offering a means to get the headers back from these types as an owned resource. Perhaps we could, similar to Rust's http::Request::into_parts() method, augment the consume method to also return the headers (and other parts like method, authority, path, status code [for responses]) along with the body. Strawman signatures:

resource incoming-request {
  consume: func() -> result<(
    method,
    option<scheme>,
    option<string>, // authority
    option<string>, // path-with-query
    headers,
    incoming-body
  )>;
}

resource incoming-response {
  consume: func() -> result<(status-code, headers, incoming-body)>;
}

This will remain relevant even once 0.3.0 arrives with the unification of the incoming- and outgoing- types. If the headers can still only be accessed immutably from an existing value, proxy applications wishing to perform modifications will still have to get a mutable headers resource via some means, and absent other changes that'll mean an unnecessary clone.

[lukewagner]

I think this is a very good idea. IIRC, @elliottt independently made the same suggestion too. Given how immediate the use case is and how small of an addition, this seems like a good candidate for inclusion in our next minor (0.2.1) release.

[acfoltzer]

@lukewagner great! @elliottt do you already have a branch with this or should I prepare a PR?

[elliottt]

I don't have a branch, and my suggestion was a little less general: I was proposing we add a take-headers method that would transfer ownership of the headers belonging to a request/response. I like your consume idea more, as it covers all fields instead of just the headers.

What do you think about making it a static method that takes an owning reference to the request/response? That way we could be sure that there's no leftover request/response resource that has weird internal state. I suppose that since consume is already part of the api, we would need a new name, but perhaps we could mirror the rust api you mentioned and call it into-parts?

[acfoltzer]

I suppose that since consume is already part of the api, we would need a new name, but perhaps we could mirror the rust api you mentioned and call it into-parts?

That would work for me, but maybe you could remind me why consume is not already a static method?

[elliottt]

The name evolved over time, and the current behavior of returning an option<incoming-body> that only returns some once was due to wanting to persist access to the other fields of incoming-request while also transferring ownership of the body. Something like into-parts would be a good way to avoid that problem, as we wouldn't be losing access to anything by consuming the original incoming-request.

[acfoltzer]

In that case, maybe we should add into-parts as a static method and consider deprecating consume? I'm not sure what standards of compatibility we want to keep between 0.2.0 and 0.3.0 but we could probably remove consume at that point

[elliottt]

Adding a new static method would be the best path forward for an 0.2.x change, and we can remove or change consume in 0.3+.

[acfoltzer]

@elliottt I can't tag you as reviewer, but I just opened #103 with these new static methods.