Can we support FilterHeadersStatus::StopIteration ?

[gengleilei]

We have a usage scenario where we need to extract data from the request body, then store it in the request header, and perform routing matching based on the request header during routing, sample code:

FilterHeadersStatus PluginContext::onRequestHeaders(uint32_t, bool) {
  auto content_type_ptr = getRequestHeader("content-type");
  auto transfer_encoding_ptr = getRequestHeader("transfer-encoding");
  if (!content_type_ptr && !transfer_encoding_ptr) {
    return FilterHeadersStatus::Continue;
  }

  stopIteration_ = true;
  return FilterHeadersStatus::StopIteration;
}

FilterDataStatus PluginContext::onRequestBody(size_t body_size, bool) {
  auto body =
      getBufferBytes(WasmBufferType::HttpRequestBody, 0, body_size);
  LOG_INFO("body: " + body->toString());

  if (stopIteration_ && (0 != body->size())) {
    addRequestHeader("body-test", body->toString());
  }

  return FilterDataStatus::Continue;
}

however, StopIteration is forced to be converted to StopAllIterationAndWatermark in proxy-wasm-cpp-host, can we remove this forced conversion?

FilterHeadersStatus ContextBase::convertVmCallResultToFilterHeadersStatus(uint64_t result) {
  if (wasm()->isNextIterationStopped() ||
      result > static_cast<uint64_t>(FilterHeadersStatus::StopAllIterationAndWatermark)) {
    return FilterHeadersStatus::StopAllIterationAndWatermark;
  }
  if ((wasm_->on_request_headers_abi_01_ || wasm_->on_request_headers_abi_02_) &&
      result == static_cast<uint64_t>(FilterHeadersStatus::StopIteration)) {
    // Always convert StopIteration (pause processing headers, but continue processing body)
    // to StopAllIterationAndWatermark (pause all processing), since the former breaks all
    // assumptions about HTTP processing.
    return FilterHeadersStatus::StopAllIterationAndWatermark;
  }
  return static_cast<FilterHeadersStatus>(result);
}

I think it would be better to let developers choose which state to use.

[gengleilei]

@PiotrSikora any suggestions?

[martijneken]

AIUI #397 was sent for this. Do you want to discuss?

This comment implies StopIteration is not good:

proxy-wasm-cpp-host/src/context.cc

Lines 497 to 499 in 04dfb94

	// Always convert StopIteration (pause processing headers, but continue processing body)
	// to StopAllIterationAndWatermark (pause all processing), since the former breaks all
	// assumptions about HTTP processing.

Source enum:
https://github.com/envoyproxy/envoy/blob/92118595813ac208b0072ab280d7d1b199ab83dd/envoy/http/filter.h#L109

And I think these issues are likely related:

• OnRequestBody not called if onRequestHeaders returns stopIteration #143
• ext_proc: Returning StopAllIterationAndWatermark prevents whole request processing envoyproxy/envoy#15793

Want to close this issue as a dupe of #143?

[johnlanni]

@martijneken Actually, StopIteration is very useful, although it might be too complex for hosts other than envoy. But for envoy, only by supporting StopIteration can these cases be implemented:

1. In the body stage, read the body content and modify the header based on this, thereby re-routing based on the parameters in the body.
2. Block the header stage, do not send to subsequent router filters (so the upstream will not receive the request), until the body content is verified in the body stage, then continue.