Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

printing non-utf8 string from metadata crashes Envoy (when trace enabled) #22609

Closed
stevenzzzz opened this issue Aug 8, 2022 · 5 comments
Closed
Assignees
Labels
bug stale stalebot believes this issue/PR has not been touched recently

Comments

@stevenzzzz
Copy link
Contributor

Title: printing non-utf8 string from metadata crashes Envoy (when trace enabled)

Description:
envoy/http/metadata_interface.h seems assume that metadata K:V are in utf8 strings:

class MetadataMap : public UnorderedStringMap {
public:
  using UnorderedStringMap::UnorderedStringMap;

  friend std::ostream& operator<<(std::ostream& out, const MetadataMap& metadata_map) {
    out << "metadata map:";
    for (const auto& metadata : metadata_map) {
      out << "\nkey: " << metadata.first << ", value: " << metadata.second << std::endl;
    }
    return out;
  }
};

But seems no such constraint in spec for metadata.

We should at least eacape the K:V before passing them into spdglog which expects utf8 string.
https://github.com/abseil/abseil-cpp/blob/master/absl/strings/escaping.h

Repro steps:

  1. Enable trace logging in Envoy,
  2. put some protobuf::serializeAsString() as stream metadata value.

Envoy will crash when the serializeAsString() contains invalid utf8 string, while spdlog tries to interpret the value as an utf8 string.

@stevenzzzz stevenzzzz added bug triage Issue requires triage labels Aug 8, 2022
@stevenzzzz
Copy link
Contributor Author

assign to @yanavlasov for triaging.
/assign @yanavlasov

@stevenzzzz
Copy link
Contributor Author

cc @adisuissa

@stevenzzzz
Copy link
Contributor Author

/assign stevenzzzz

@github-actions
Copy link

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or "no stalebot" or other activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale stalebot believes this issue/PR has not been touched recently label Sep 15, 2022
@stevenzzzz
Copy link
Contributor Author

the fix is in.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug stale stalebot believes this issue/PR has not been touched recently
Projects
None yet
Development

No branches or pull requests

3 participants