proxy-wasm-cpp-host icon indicating copy to clipboard operation
proxy-wasm-cpp-host copied to clipboard

Can we support FilterHeadersStatus::StopIteration ?

Open gengleilei opened this issue 1 year ago • 3 comments

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 avatar Sep 21 '23 04:09 gengleilei

@PiotrSikora any suggestions?

gengleilei avatar Sep 21 '23 04:09 gengleilei