Skip to content

Commit

Permalink
exceptions: removing DecodeException (envoyproxy#30858)
Browse files Browse the repository at this point in the history
Replacing DecodeException with a normal EnvoyException. coverage lowered simply due to lines of code being lowered.

Risk Level: low
Testing: n/a
Docs Changes: n/a
Release Notes: n/a

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
  • Loading branch information
alyssawilk authored Nov 14, 2023
1 parent 7a2f330 commit 2f35392
Show file tree
Hide file tree
Showing 4 changed files with 12 additions and 28 deletions.
13 changes: 4 additions & 9 deletions source/common/config/xds_resource.cc
Original file line number Diff line number Diff line change
Expand Up @@ -104,10 +104,6 @@ std::string XdsResourceIdentifier::encodeUrl(const xds::core::v3::ResourceLocato

namespace {

void throwDecodeExceptionOrPanic(std::string message) {
throwExceptionOrPanic(XdsResourceIdentifier::DecodeException, message);
}

void decodePath(absl::string_view path, std::string* resource_type, std::string& id) {
// This is guaranteed by Http::Utility::extractHostPathFromUrn.
ASSERT(absl::StartsWith(path, "/"));
Expand All @@ -116,7 +112,7 @@ void decodePath(absl::string_view path, std::string* resource_type, std::string&
if (resource_type != nullptr) {
*resource_type = std::string(path_components[0]);
if (resource_type->empty()) {
throwDecodeExceptionOrPanic(fmt::format("Resource type missing from {}", path));
throwEnvoyExceptionOrPanic(fmt::format("Resource type missing from {}", path));
}
id_it = std::next(id_it);
}
Expand All @@ -143,7 +139,7 @@ void decodeFragment(
} else if (absl::StartsWith(fragment_component, "entry=")) {
directives.Add()->set_entry(PercentEncoding::decode(fragment_component.substr(6)));
} else {
throwDecodeExceptionOrPanic(fmt::format("Unknown fragment component {}", fragment_component));
throwEnvoyExceptionOrPanic(fmt::format("Unknown fragment component {}", fragment_component));
;
}
}
Expand All @@ -153,7 +149,7 @@ void decodeFragment(

xds::core::v3::ResourceName XdsResourceIdentifier::decodeUrn(absl::string_view resource_urn) {
if (!hasXdsTpScheme(resource_urn)) {
throwDecodeExceptionOrPanic(fmt::format("{} does not have an xdstp: scheme", resource_urn));
throwEnvoyExceptionOrPanic(fmt::format("{} does not have an xdstp: scheme", resource_urn));
}
absl::string_view host, path;
Http::Utility::extractHostPathFromUri(resource_urn, host, path);
Expand Down Expand Up @@ -188,8 +184,7 @@ xds::core::v3::ResourceLocator XdsResourceIdentifier::decodeUrl(absl::string_vie
decodePath(path, nullptr, *decoded_resource_locator.mutable_id());
return decoded_resource_locator;
} else {
throwExceptionOrPanic(
XdsResourceIdentifier::DecodeException,
throwEnvoyExceptionOrPanic(
fmt::format("{} does not have a xdstp:, http: or file: scheme", resource_url));
}
decoded_resource_locator.set_authority(PercentEncoding::decode(host));
Expand Down
10 changes: 2 additions & 8 deletions source/common/config/xds_resource.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,18 +44,12 @@ class XdsResourceIdentifier {
return encodeUrl(resource_locator, {});
}

// Thrown when an exception occurs during URI decoding.
class DecodeException : public EnvoyException {
public:
DecodeException(const std::string& what) : EnvoyException(what) {}
};

/**
* Decode a xdstp:// URN string to a xds::core::v3::ResourceName.
*
* @param resource_urn xdstp:// resource URN.
* @return xds::core::v3::ResourceName resource name message for resource_urn.
* @throws DecodeException when parsing fails.
* @throws EnvoyException when parsing fails.
*/
static xds::core::v3::ResourceName decodeUrn(absl::string_view resource_urn);

Expand All @@ -64,7 +58,7 @@ class XdsResourceIdentifier {
*
* @param resource_url xdstp:// resource URL.
* @return xds::core::v3::ResourceLocator resource name message for resource_url.
* @throws DecodeException when parsing fails.
* @throws EnvoyException when parsing fails.
*/
static xds::core::v3::ResourceLocator decodeUrl(absl::string_view resource_url);

Expand Down
15 changes: 5 additions & 10 deletions test/common/config/xds_resource_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -119,33 +119,28 @@ TEST(XdsResourceNameTest, DecodeEmpty) {
// Negative tests for URN decoding.
TEST(XdsResourceNameTest, DecodeFail) {
{
EXPECT_THROW_WITH_MESSAGE(XdsResourceIdentifier::decodeUrn("foo://"),
XdsResourceIdentifier::DecodeException,
EXPECT_THROW_WITH_MESSAGE(XdsResourceIdentifier::decodeUrn("foo://"), EnvoyException,
"foo:// does not have an xdstp: scheme");
}
{
EXPECT_THROW_WITH_MESSAGE(XdsResourceIdentifier::decodeUrn("xdstp://foo"),
XdsResourceIdentifier::DecodeException,
EXPECT_THROW_WITH_MESSAGE(XdsResourceIdentifier::decodeUrn("xdstp://foo"), EnvoyException,
"Resource type missing from /");
}
}

// Negative tests for URL decoding.
TEST(XdsResourceLocatorTest, DecodeFail) {
{
EXPECT_THROW_WITH_MESSAGE(XdsResourceIdentifier::decodeUrl("foo://"),
XdsResourceIdentifier::DecodeException,
EXPECT_THROW_WITH_MESSAGE(XdsResourceIdentifier::decodeUrl("foo://"), EnvoyException,
"foo:// does not have a xdstp:, http: or file: scheme");
}
{
EXPECT_THROW_WITH_MESSAGE(XdsResourceIdentifier::decodeUrl("xdstp://foo"),
XdsResourceIdentifier::DecodeException,
EXPECT_THROW_WITH_MESSAGE(XdsResourceIdentifier::decodeUrl("xdstp://foo"), EnvoyException,
"Resource type missing from /");
}
{
EXPECT_THROW_WITH_MESSAGE(XdsResourceIdentifier::decodeUrl("xdstp://foo/some-type#bar=baz"),
XdsResourceIdentifier::DecodeException,
"Unknown fragment component bar=baz");
EnvoyException, "Unknown fragment component bar=baz");
}
}

Expand Down
2 changes: 1 addition & 1 deletion test/per_file_coverage.sh
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ declare -a KNOWN_LOW_COVERAGE=(
"source/common:96.2"
"source/common/api:84.5" # flaky due to posix: be careful adjusting
"source/common/api/posix:83.8" # flaky (accept failover non-deterministic): be careful adjusting
"source/common/config:95.4"
"source/common/config:95.3"
"source/common/crypto:95.5"
"source/common/event:95.1" # Emulated edge events guards don't report LCOV
"source/common/filesystem/posix:96.2" # FileReadToEndNotReadable fails in some env; createPath can't test all failure branches.
Expand Down

0 comments on commit 2f35392

Please sign in to comment.