Skip to content

Commit

Permalink
xds: returning absl::Status (envoyproxy#29497)
Browse files Browse the repository at this point in the history
follow up to envoyproxy#29439 using the new status return code

Risk Level: medium
Testing: updated tests
Docs Changes: n/a
Release Notes: n/a
envoyproxy/envoy-mobile#176

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
  • Loading branch information
alyssawilk committed Nov 28, 2023
1 parent acc54c5 commit 89a4848
Show file tree
Hide file tree
Showing 4 changed files with 13 additions and 11 deletions.
1 change: 0 additions & 1 deletion source/common/config/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,6 @@ envoy_cc_library(
":custom_config_validators_lib",
":type_to_endpoint_lib",
":utility_lib",
":xds_resource_lib",
"//envoy/config:subscription_factory_interface",
"//envoy/config:subscription_interface",
"//envoy/config:xds_config_tracker_interface",
Expand Down
1 change: 0 additions & 1 deletion source/common/config/subscription_factory_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
#include "source/common/config/custom_config_validators_impl.h"
#include "source/common/config/type_to_endpoint.h"
#include "source/common/config/utility.h"
#include "source/common/config/xds_resource.h"
#include "source/common/http/utility.h"
#include "source/common/protobuf/protobuf.h"
#include "source/common/protobuf/utility.h"
Expand Down
20 changes: 12 additions & 8 deletions source/common/config/xds_resource.cc
Original file line number Diff line number Diff line change
Expand Up @@ -104,19 +104,20 @@ std::string XdsResourceIdentifier::encodeUrl(const xds::core::v3::ResourceLocato

namespace {

void decodePath(absl::string_view path, std::string* resource_type, std::string& id) {
absl::Status decodePath(absl::string_view path, std::string* resource_type, std::string& id) {
// This is guaranteed by Http::Utility::extractHostPathFromUrn.
ASSERT(absl::StartsWith(path, "/"));
const std::vector<absl::string_view> path_components = absl::StrSplit(path.substr(1), '/');
auto id_it = path_components.cbegin();
if (resource_type != nullptr) {
*resource_type = std::string(path_components[0]);
if (resource_type->empty()) {
throwEnvoyExceptionOrPanic(fmt::format("Resource type missing from {}", path));
return absl::InvalidArgumentError(fmt::format("Resource type missing from {}", path));
}
id_it = std::next(id_it);
}
id = PercentEncoding::decode(absl::StrJoin(id_it, path_components.cend(), "/"));
return absl::OkStatus();
}

void decodeQueryParams(absl::string_view query_params,
Expand Down Expand Up @@ -147,9 +148,9 @@ void decodeFragment(

} // namespace

xds::core::v3::ResourceName XdsResourceIdentifier::decodeUrn(absl::string_view resource_urn) {
absl::StatusOr<xds::core::v3::ResourceName> XdsResourceIdentifier::decodeUrn(absl::string_view resource_urn) {
if (!hasXdsTpScheme(resource_urn)) {
throwEnvoyExceptionOrPanic(fmt::format("{} does not have an xdstp: scheme", resource_urn));
absl::InvalidArgumentError(fmt::format("{} does not have an xdstp: scheme", resource_urn));
}
absl::string_view host, path;
Http::Utility::extractHostPathFromUri(resource_urn, host, path);
Expand All @@ -160,8 +161,11 @@ xds::core::v3::ResourceName XdsResourceIdentifier::decodeUrn(absl::string_view r
decodeQueryParams(path.substr(query_params_start), *decoded_resource_name.mutable_context());
path = path.substr(0, query_params_start);
}
decodePath(path, decoded_resource_name.mutable_resource_type(),
status = decodePath(path, decoded_resource_name.mutable_resource_type(),
*decoded_resource_name.mutable_id());
if (!status.ok()) {
return status;
}
return decoded_resource_name;
}

Expand All @@ -181,7 +185,7 @@ xds::core::v3::ResourceLocator XdsResourceIdentifier::decodeUrl(absl::string_vie
} else if (absl::StartsWith(resource_url, "file:")) {
decoded_resource_locator.set_scheme(xds::core::v3::ResourceLocator::FILE);
// File URLs only have a path and fragment.
decodePath(path, nullptr, *decoded_resource_locator.mutable_id());
THROW_IF_NOT_OK(decodePath(path, nullptr, *decoded_resource_locator.mutable_id()));
return decoded_resource_locator;
} else {
throwEnvoyExceptionOrPanic(
Expand All @@ -194,8 +198,8 @@ xds::core::v3::ResourceLocator XdsResourceIdentifier::decodeUrl(absl::string_vie
*decoded_resource_locator.mutable_exact_context());
path = path.substr(0, query_params_start);
}
decodePath(path, decoded_resource_locator.mutable_resource_type(),
*decoded_resource_locator.mutable_id());
THROW_IF_NOT_OK(decodePath(path, decoded_resource_locator.mutable_resource_type(),
*decoded_resource_locator.mutable_id()));
return decoded_resource_locator;
}

Expand Down
2 changes: 1 addition & 1 deletion source/common/config/xds_resource.h
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ class XdsResourceIdentifier {
* @return xds::core::v3::ResourceName resource name message for resource_urn.
* @throws EnvoyException when parsing fails.
*/
static xds::core::v3::ResourceName decodeUrn(absl::string_view resource_urn);
static absl::StatusOr<xds::core::v3::ResourceName> decodeUrn(absl::string_view resource_urn);

/**
* Decode a xdstp:// URL string to a xds::core::v3::ResourceLocator.
Expand Down

0 comments on commit 89a4848

Please sign in to comment.