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.

Sign up for GitHub

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

http: remove getAll() header map API and switch all usages to get() #13363

Merged
merged 12 commits into from
Oct 12, 2020
Merged

http: remove getAll() header map API and switch all usages to get() #13363

Changes from 1 commit
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
2f20030
http: remove getAll() header map API and switch all usages to get()
mattklein123 Jul 16, 2020
ca47109
Merge branch 'master' into headers_get_all
mattklein123 Oct 7, 2020
0a111ae
comments
mattklein123 Oct 7, 2020
d83bd5f
Merge remote-tracking branch 'origin/master' into headers_get_all
mattklein123 Oct 7, 2020
2c353b3
Merge branch 'master' into headers_get_all
mattklein123 Oct 8, 2020
c3c0f94
comments
mattklein123 Oct 8, 2020
32eb7b8
Merge remote-tracking branch 'origin/master' into headers_get_all
mattklein123 Oct 8, 2020
5405ce7
Merge remote-tracking branch 'origin/master' into headers_get_all
mattklein123 Oct 9, 2020
d0ba0c2
fix
mattklein123 Oct 9, 2020
8f4219e
Merge branch 'master' into headers_get_all
mattklein123 Oct 12, 2020
930359c
update filter example SHA
mattklein123 Oct 12, 2020
ce7ec36
WASM fix
mattklein123 Oct 12, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions source/extensions/common/wasm/context.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -696,16 +696,18 @@ WasmResult Context::getHeaderMapValue(WasmHeaderMapType type, absl::string_view
return WasmResult::BadArgument;
}
const Http::LowerCaseString lower_key{std::string(key)};
auto entry = map->get(lower_key);
if (!entry) {
const auto entry = map->get(lower_key);
if (entry.empty()) {
if (wasm()->abiVersion() == proxy_wasm::AbiVersion::ProxyWasm_0_1_0) {
*value = "";
return WasmResult::Ok;
} else {
return WasmResult::NotFound;
}
}
*value = entry->value().getStringView();
// TODO(kyessenov, PiotrSikora): This needs to either return a concatenated list of values, or
// the ABI needs to be changed to return multiple values. This is a potential security issue.
Comment on lines +708 to +709
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kyessenov @PiotrSikora this needs to be fixed ASAP

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Talked to @kyessenov offline about this and it will be fixed soon)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW, the ABI in proxy-wasm/spec#1 already returns multiple values.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfect. Let's do that then.

*value = entry[0]->value().getStringView();
return WasmResult::Ok;
}

Expand Down