OnRequestBody not called if onRequestHeaders returns stopIteration

[bladedancer]

I'll preface this with it could be user error but my scenario is I'm making a grpc call out to some backend service. The backend server will mutate both the headers and the body. So I need to execute both onRequestHeaders and onRequestBody before getting the result.

The problem is it hangs indefinitely, but I'm told it should work. Assuming it did work once then I believe it's related to this change:

#95

Sampe code is here: https://github.com/bladedancer/envoy-wasm-stopitertest/blob/main/wasm-filter/context.cc#L32.

Is this scenario achievable? Is it a bug or the expected behaviour?

[PiotrSikora]

It's the expected behavior. It did work in the past, but such behavior is error-prone and resulted in a lot of weird edge cases.

Why exactly do you need to block headers?

[bladedancer]

I have a legacy decision point that potentially operates on the headers and the body. My problem is that I cannot mutate the headers after sending the body to the service... Because the only way i can send the body is to continue the headers.

[PiotrSikora]

The question is why do you need to modify headers after mutating the body?

[bladedancer]

I haven't mutated the body at this point - I'm forwarding the entire request to the decision point. It will look at the entire message - body and headers and apply policy to the request - it may redact headers, transcode content, enrich content, add headers, etc. I was using the wasm instead of lua (which I believe can do the header modification) here because I wanted to use grpc for the communication.

I suppose the question I'd have is - is this a fundamental design decision for the wasm filters? I know you say it's "error prone", and I'm sure I could misuse it but it also opens up scope for legacy integrations like this.

[wbpcode]

😢 I encountered the same problem. I wanted to implement a custom ext auth filter using wasm, but found that I couldn't stop processing headers and continue processing bodies, so I couldn't send my entire request to an external authentication service and decide my next action based on the results.

[PiotrSikora]

It's a bit of a fundamental issue. Buffering complete requests/responses doesn't work at scale.

If you offload buffering to the proxy, then you're going to hit proxy buffer limits pretty fast (those are usually 16/32/64 KiB).

If you start buffering inside WasmVM, then you're going to hit Wasm memory limits pretty fast... unless there is no upper limit, in which case you're risking OOM or perceived memory leaks (currently Wasm memory can grow, but it cannot shrink back when unused).

Basically, buffering works when there is little traffic, but it either breaks under load or requires extreme care and awarness of the memory limits when developing a plugin, so not the most friendly feature.

Having said that, it seems like there are legitimate use cases for it, and we might need to support them, but it's unlikely to happen in the next few weeks.

@bladedancer if I'm understanding your use case correctly, then you could stream the original request to the authorization server, and stream the response from the authorization server to the backend, without any buffering. There is no need to buffer the original request, correct?

@wbpcode on the other hand, you need to always buffer complete request, and send it to the authorization server and the backend, correct?

[wbpcode]

Yep, but it will do some limit to buffer size. For example, 4 Kb.

[bladedancer]

@PiotrSikora yes, in the policy decision point the goal will be to minimize the scenarios that require buffering but if buffering is required it will be done there, not in envoy.

As an aside - I tried with the ext_proc also, that resulted in this one envoyproxy/envoy#15798

[nullpo-head]

@PiotrSikora We've encountered the same problem. In our case, we don't modify the request header, but our backend server decides whether to deny or allow it after the whole request is processed. So, we want to stop the request headers until the request body is processed.

There seems to be a fair amount of requests for the ability to reject/modify headers after inspecting the request/response body. So do you have any idea when to proceed with this feature? Also, are there any other obstacles besides the buffer limitation issue?

• add new HTTP header based on data from body proxy-wasm-rust-sdk#114
• modify response headers values based on response body contents proxy-wasm-cpp-sdk#62
• https://envoyproxy.slack.com/archives/CM2CQ4XJ5/p1623261646039100

[pdeligia]

+1 to several valid use cases for this.

We have also encountered the same problem. Besides what @bladedancer and @nullpo-head mentioned, which is really useful in various scenarios where you need both headers+body to enforce some policy, we also have a scenario where there might be some processing of the body (or parts of the body) -- e.g. decryption or some aggregation -- and then a header has to be updated accordingly (e.g. the body might have different length, so the content-length header would be different). And all these have to be done inside the proxy before they reach the destination. All these scenarios are sadly not possible today.

This is somewhat related to the other issue we discussed before @PiotrSikora envoyproxy/envoy#17208, in general being able to set headers/bodies outside the scope of the respective callbacks would be extremely useful feature.

[jcchavezs]

We have same issue as described below. We need both headers and body to decide some blocking. See https://github.com/corazawaf/coraza-proxy-wasm/pull/128/files#diff-a755f6a359f42d229feba810f3cd6f9c128aa43ffb716b33b186ec5fe8aa0a49R173-R180.

Any chance we can change this behaviour? cc @mathetake

[martijneken]

I am trying to understand #95 better.

In that PR people asked why FilterHeadersStatus::StopIteration exists. I understand why it's a bad idea paired with sendLocalReply(), which should terminate the filter chain, but that's not what we're talking about here.

Here's how I understand it (please correct any errors):

• Assume you have a proxy with filters [A, B, C] and filter B leverages ProxyWasm (inline or ext_proc, doesn't matter).
• The point of the StopIteration / StopAllIterationAndBuffer / StopAllIterationAndWatermark return codes is to pause the filter chain until the local filter (in this case filter B) can make a decision.
• The intent described here is to pause the header filter chain after filter B (using StopIteration), but to continue sending body chunks to ProxyWasm.
• AIUI, this can challenge the proxy (let's assume Envoy) for two reasons:
  1. The proxy has to implement sequencing / head-of-line blocking. As soon as ProxyWasm makes the decision to continue, it has to ensure that headers are sent to filter C ahead of any body chunks. Does this work properly in Envoy?
  2. The proxy has to buffer the all data it sends to ProxyWasm. ProxyWasm can modify the headers/body, but it isn't required to. So although the headers and body need not be buffered in ProxyWasm itself (or an external decision point it uses), they must be buffered in the proxy until ProxyWasm makes a decision (so that the proxy can send the data onward to filter C). There is no way for ProxyWasm to 'consume and replace' the header/body data.

Did I represent this accurately @PiotrSikora ?

I can see why we'd want to avoid buffering in the proxy in general, but it also sounds like there are legitimate uses for it. It seems to me that the decision should be made in the proxy/filter rather than in ProxyWasm itself. For example, the ext_proc filter has BUFFERED and BUFFERED_PARTIAL modes to limit buffering on external requests.

[PiotrSikora]

There is no way for ProxyWasm to 'consume and replace' the header/body data.

The whole ABI is about modifying streamed headers, body data, and trailers.

Did I represent this accurately @PiotrSikora ?

I think so, but honestly I don't recall all the details about the issue we had with FilterHeadersStatus::StopIteration. Other than the crashes, I think it was possible to receive body before headers, but I might be misremembering it now.

I can see why we'd want to avoid buffering in the proxy in general, but it also sounds like there are legitimate uses for it.

Right. There are legitimate use cases that we should support.

It seems to me that the decision should be made in the proxy/filter rather than in ProxyWasm itself.

Proxy-Wasm plugins need a way to control this behavior. If we delegate this to proxy, then it's not going to be portable across implementations.

[martijneken]

There is no way for ProxyWasm to 'consume and replace' the header/body data.

The whole ABI is about modifying streamed headers, body data, and trailers.

Right, I worded that poorly. I meant: the ProxyWasm API is geared toward modification, not replacement. There is no way to configure ProxyWasm to always consume the data it receives, which would mean that the proxy can discard the data and avoid buffering it. Such a feature might solve the buffering problem.

It seems to me that the decision should be made in the proxy/filter rather than in ProxyWasm itself.

Proxy-Wasm plugins need a way to control this behavior.

See above. In my mind, ProxyWasm should control pause/continue, but the proxy can decide whether to buffer, how much to buffer, etc. Do you see it differently?

[jchadwick-buf]

I believe that the ability to do this is actually pretty important, despite that it may seem like a bad idea. I do agree that buffering full requests and/or responses does not scale well, however, not all solutions necessarily need to scale well to be immensely useful (as evidenced by the testimony here and the existence of filters that do this in Envoy upstream), and it'd be nice if Proxy-Wasm supported this functionality: without it, some use cases are simply not possible to do.

As far as correctness goes, when I personally was working on Envoy filters directly in C++, the semantics seemed fairly straight-forward to me, though maybe I missed something important. It seemed like no matter what I did, the request and response callbacks ultimately came in the expected order: headers, data frames, trailers. I may just be wrong, but I spent a lot of time trying to make sure that assumption was correct and never found any condition that seemed to violate it. As an example of what can be done with this, one thing you can do in Envoy is take trailers and move them into the headers: so as long as the filter chain remained stopped at headers, it's still kosher to modify the header map that was passed in the requestHeaders/responseHeaders callback, even in the trailer frame. I think this is not possible with Proxy-Wasm today, for possibly more than one reason, but hopefully it is somewhat apparent how this could be useful.

100% agree and understand that this is complicated. However, I personally do not think it inherently violates the expectations of HTTP processing; All of the filters should be processing callbacks in order of the h2 frames as expected, but this would just stall the filter chain until we're ready to send the headers frame, at which point filters following the Wasm filter would see the rewritten frames to that point as if nothing had happened.

At this point I do not really expect this feature to return to Proxy-Wasm, but I figured it couldn't hurt to leave my personal thoughts, as I genuinely feel this feature would be very useful.