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

Code before:
                              std::string);
    if (!inline_jwks.empty()) {
      auto jwks =
          ::google::jwt_verify::Jwks::createFrom(inline_jwks, ::google::jwt_verify::Jwks::JWKS);
      if (jwks->getStatus() != Status::Ok) {
        ENVOY_LOG(warn, "Invalid inline jwks for issuer: {}, jwks: {}", jwt_provider_.issuer(),
                  inline_jwks);
      } else {
        setJwksToAllThreads(std::move(jwks));
      }
    } else {
      if (!jwt_provider_.has_remote_jwks()) {
        return;
      }

      // remote_jwks.retry_policy has an invalid case that could not be validated by the
      // proto validation annotation. It has to be validated by the code.
      if (jwt_provider_.remote_jwks().has_retry_policy()) {
        THROW_IF_NOT_OK(
            Http::Utility::validateCoreRetryPolicy(jwt_provider_.remote_jwks().retry_policy()));
        envoy::config::route::v3::RetryPolicy route_retry_policy =
            Http::Utility::convertCoreToRouteRetryPolicy(jwt_provider_.remote_jwks().retry_policy(),
                                                         "5xx,gateway-error,connect-failure,reset");
        // Use the null validation visitor because it used by the async client in the previous
        // implementation.
        auto policy_or_error = Router::RetryPolicyImpl::create(
            route_retry_policy, ProtobufMessage::getNullValidationVisitor(),
            context.serverFactoryContext());
        THROW_IF_NOT_OK_REF(policy_or_error.status());
        retry_policy_ = std::move(policy_or_error.value());
      }

      if (jwt_provider_.remote_jwks().has_cache_duration()) {
        // Use `durationToMilliseconds` as it has stricter max boundary to the `seconds` value to
        // avoid overflow.
        Protobuf::Duration duration_copy(jwt_provider_.remote_jwks().cache_duration());
        (void)DurationUtil::durationToMilliseconds(duration_copy);

        // remote_jwks.duration is used as: now + remote_jwks.duration.
        // need to verify twice of its `seconds` value.
        duration_copy.set_seconds(2 * duration_copy.seconds());
        (void)DurationUtil::durationToMilliseconds(duration_copy);
      }

      // create async_fetch for remote_jwks, if is no-op if async_fetch is not enabled.
      async_fetcher_ = std::make_unique<JwksAsyncFetcher>(
          jwt_provider_.remote_jwks(), retry_policy_, context, fetcher_cb, stats,
          [this](google::jwt_verify::JwksPtr&& jwks) { setJwksToAllThreads(std::move(jwks)); });
    }
  }


Review comment (suggestion):
`auto` vs. `explicit types` - I have seen both patterns so not really sure which one we prefer.

Code after:
                              std::string);
    if (!inline_jwks.empty()) {
      auto jwks =
          ::google::jwt_verify::Jwks::createFrom(inline_jwks, ::google::jwt_verify::Jwks::JWKS);
      if (jwks->getStatus() != Status::Ok) {
        ENVOY_LOG(warn, "Invalid inline jwks for issuer: {}, jwks: {}", jwt_provider_.issuer(),
                  inline_jwks);
      } else {
        setJwksToAllThreads(std::move(jwks));
      }
    } else {
      if (!jwt_provider_.has_remote_jwks()) {
        return;
      }

      // remote_jwks.retry_policy has an invalid case that could not be validated by the
      // proto validation annotation. It has to be validated by the code.
      if (jwt_provider_.remote_jwks().has_retry_policy()) {
        THROW_IF_NOT_OK(
            Http::Utility::validateCoreRetryPolicy(jwt_provider_.remote_jwks().retry_policy()));
        envoy::config::route::v3::RetryPolicy route_retry_policy =
            Http::Utility::convertCoreToRouteRetryPolicy(jwt_provider_.remote_jwks().retry_policy(),
                                                         "5xx,gateway-error,connect-failure,reset");
        // Use the null validation visitor because it was used by the async client in the previous
        // implementation.
        auto policy_or_error = Router::RetryPolicyImpl::create(
            route_retry_policy, ProtobufMessage::getNullValidationVisitor(),
            context.serverFactoryContext());
        THROW_IF_NOT_OK_REF(policy_or_error.status());
        retry_policy_ = std::move(policy_or_error.value());
      }

      if (jwt_provider_.remote_jwks().has_cache_duration()) {
        // Use `durationToMilliseconds` as it has stricter max boundary to the `seconds` value to
        // avoid overflow.
        Protobuf::Duration duration_copy(jwt_provider_.remote_jwks().cache_duration());
        (void)DurationUtil::durationToMilliseconds(duration_copy);

        // remote_jwks.duration is used as: now + remote_jwks.duration.
        // need to verify twice of its `seconds` value.
        duration_copy.set_seconds(2 * duration_copy.seconds());
        (void)DurationUtil::durationToMilliseconds(duration_copy);
      }

      // create async_fetch for remote_jwks, if is no-op if async_fetch is not enabled.
      async_fetcher_ = std::make_unique<JwksAsyncFetcher>(
          jwt_provider_.remote_jwks(), retry_policy_, context, fetcher_cb, stats,
          [this](google::jwt_verify::JwksPtr&& jwks) { setJwksToAllThreads(std::move(jwks)); });
    }
  }