Repository: envoyproxy/envoy
File: source/extensions/filters/http/ext_proc/mutation_utils.cc (C++)

Code before:
    rejected_mutations.inc();
    return absl::InvalidArgumentError("header_mutation_remove_and_set_count_exceeded");
  }
  if (remove_size > max_request_headers_count) {
    ENVOY_LOG(debug,
              "Header mutation remove header count {} exceeds the "
              "max header count limit {}. Returning error.",
              remove_size, max_request_headers_count);
    rejected_mutations.inc();
    return absl::InvalidArgumentError("header_mutation_remove_count_exceeded");
  }
  if (set_size > max_request_headers_count) {
    ENVOY_LOG(debug,
              "Header mutation set header count {} exceeds the "
              "max header count limit {}. Returning error.",
              set_size, max_request_headers_count);
    rejected_mutations.inc();
    return absl::InvalidArgumentError("header_mutation_set_count_exceeded");
  }

  return absl::OkStatus();
}

absl::Status MutationUtils::headerMutationResultCheck(const Http::HeaderMap& headers,
                                                      Counter& rejected_mutations) {
  if (headers.byteSize() > headers.maxHeadersKb() * 1024 &&
      headers.size() > headers.maxHeadersCount()) {
    ENVOY_LOG(debug,
              "After mutation, the total header count {} and total header size {} bytes exceed the "
              "count limit {} and the size limit {} kilobytes. Returning error.",
              headers.size(), headers.byteSize(), headers.maxHeadersCount(),
              headers.maxHeadersKb());
    rejected_mutations.inc();
    return absl::InvalidArgumentError("header_mutation_bytes_and_count_exceeded");
  }
  if (headers.byteSize() > headers.maxHeadersKb() * 1024) {
    ENVOY_LOG(debug,
              "After mutation, the total header size {} bytes exceeds the "
              "size limit {} kilobytes. Returning error.",
              headers.byteSize(), headers.maxHeadersKb());
    rejected_mutations.inc();
    return absl::InvalidArgumentError("header_mutation_bytes_exceeded");
  }
  if (headers.size() > headers.maxHeadersCount()) {
    ENVOY_LOG(debug,
              "After mutation, the total header count {} exceeds the "
              "count limit {}. Returning error.",
              headers.size(), headers.maxHeadersCount());
    rejected_mutations.inc();
    return absl::InvalidArgumentError("header_mutation_count_exceeded");
  }

Review comment (suggestion):
it's probably fine to return on the first error, we could log both in logging, but remove the "&&" case for all these checkes.

Code after:
absl::Status MutationUtils::responseHeaderSizeCheck(const Http::HeaderMap& headers,
                                                    const HeaderMutation& mutation,
                                                    Counter& rejected_mutations) {
  const uint32_t remove_size = mutation.remove_headers().size();
  const uint32_t set_size = mutation.set_headers().size();
  const uint32_t max_request_headers_count = headers.maxHeadersCount();
  bool count_exceeded = false;

  if (remove_size > max_request_headers_count) {
    ENVOY_LOG_PERIODIC(error, std::chrono::seconds(10),
                       "Header mutation remove header count {} exceeds the "
                       "max header count limit {}. Returning error.",
                       remove_size, max_request_headers_count);
    count_exceeded = true;
  }
  if (set_size > max_request_headers_count) {
    ENVOY_LOG_PERIODIC(error, std::chrono::seconds(10),
                       "Header mutation set header count {} exceeds the "
                       "max header count limit {}. Returning error.",
                       set_size, max_request_headers_count);
    count_exceeded = true;
  }
  if (count_exceeded) {
    rejected_mutations.inc();
    return absl::InvalidArgumentError(kCountExceededMsg);
  }
  return absl::OkStatus();
}

absl::Status MutationUtils::headerMutationResultCheck(const Http::HeaderMap& headers,
                                                      Counter& rejected_mutations) {
  bool limit_exceeded = false;
  if (headers.byteSize() > headers.maxHeadersKb() * 1024) {
    ENVOY_LOG_PERIODIC(error, std::chrono::seconds(10),
                       "After mutation, the total header size {} bytes exceeds the "
                       "size limit {} kilobytes. Returning error.",
                       headers.byteSize(), headers.maxHeadersKb());
    limit_exceeded = true;
  }
  if (headers.size() > headers.maxHeadersCount()) {
    ENVOY_LOG_PERIODIC(error, std::chrono::seconds(10),
                       "After mutation, the total header count {} exceeds the "
                       "count limit {}. Returning error.",
                       headers.size(), headers.maxHeadersCount());
    limit_exceeded = true;
  }
  if (limit_exceeded) {
    rejected_mutations.inc();
    return absl::InvalidArgumentError(kResultExceedsLimitMsg);
  }
  return absl::OkStatus();