From 1311c0eadf0a6777bda1fe77e412b8cd41cd7d9b Mon Sep 17 00:00:00 2001 From: "tim.quinn@oracle.com" Date: Wed, 29 Apr 2020 11:50:33 -0500 Subject: [PATCH 1/2] Make clear which methods expect mapped CORS config nodes vs. lower-level CORS-info-only config nodes Signed-off-by: tim.quinn@oracle.com --- .../microprofile/cors/CrossOriginFilter.java | 2 +- .../io/helidon/webserver/cors/Aggregator.java | 15 +++++++++++-- .../helidon/webserver/cors/CorsSupport.java | 19 +++++++++++++---- .../webserver/cors/CorsSupportBase.java | 12 +++++++++++ .../webserver/cors/CorsSupportHelper.java | 21 ++++++++++--------- .../io/helidon/webserver/cors/TestUtil.java | 18 +++++++++++++--- 6 files changed, 67 insertions(+), 20 deletions(-) diff --git a/microprofile/cors/src/main/java/io/helidon/microprofile/cors/CrossOriginFilter.java b/microprofile/cors/src/main/java/io/helidon/microprofile/cors/CrossOriginFilter.java index 57545ec7786..4accfac103c 100644 --- a/microprofile/cors/src/main/java/io/helidon/microprofile/cors/CrossOriginFilter.java +++ b/microprofile/cors/src/main/java/io/helidon/microprofile/cors/CrossOriginFilter.java @@ -64,7 +64,7 @@ class CrossOriginFilter implements ContainerRequestFilter, ContainerResponseFilt CrossOriginFilter() { Config config = (Config) ConfigProvider.getConfig(); - cors = CorsSupportMp.builder().config(config.get(CORS_CONFIG_KEY)) + cors = CorsSupportMp.builder().mappedConfig(config.get(CORS_CONFIG_KEY)) .secondaryLookupSupplier(this::crossOriginFromAnnotationSupplier) .build(); } diff --git a/webserver/cors/src/main/java/io/helidon/webserver/cors/Aggregator.java b/webserver/cors/src/main/java/io/helidon/webserver/cors/Aggregator.java index dc2f5f2877f..340982ec94a 100644 --- a/webserver/cors/src/main/java/io/helidon/webserver/cors/Aggregator.java +++ b/webserver/cors/src/main/java/io/helidon/webserver/cors/Aggregator.java @@ -92,10 +92,21 @@ public boolean isActive() { return isEnabled && !crossOriginConfigMatchables.isEmpty(); } + Aggregator config(Config config) { + if (config.exists()) { + ConfigValue configValue = config.as(CrossOriginConfig::builder); + if (configValue.isPresent()) { + CrossOriginConfig crossOriginConfig = configValue.get().build(); + addPathlessCrossOrigin(crossOriginConfig); + } + } + return this; + } + /** - * Add cross-origin information from a {@link Config} node. + * Add mapped cross-origin information from a {@link Config} node. * - * @param config {@code Config} node containing + * @param config {@code Config} node containing mapped {@code CrossOriginConfig} data * @return updated builder */ Aggregator mappedConfig(Config config) { diff --git a/webserver/cors/src/main/java/io/helidon/webserver/cors/CorsSupport.java b/webserver/cors/src/main/java/io/helidon/webserver/cors/CorsSupport.java index cc830ff9bb7..7d9bacb2d65 100644 --- a/webserver/cors/src/main/java/io/helidon/webserver/cors/CorsSupport.java +++ b/webserver/cors/src/main/java/io/helidon/webserver/cors/CorsSupport.java @@ -26,8 +26,6 @@ import io.helidon.webserver.ServerResponse; import io.helidon.webserver.Service; -import static io.helidon.webserver.cors.Aggregator.PATHLESS_KEY; - /** * SE implementation of {@link CorsSupportBase}. */ @@ -93,8 +91,21 @@ public static CorsSupport create(Config config) { if (!config.exists()) { throw MissingValueException.create(config.key()); } - Builder builder = builder().addCrossOrigin(PATHLESS_KEY, CrossOriginConfig.builder(config).build()); - return builder.build(); + return builder().config(config).build(); + } + + /** + * Creates a new {@code CorsSupport} instance based on the provided configuration expected to contain mapped cross-origin + * config information. + * + * @param config node containing the mapped cross-origin information + * @return initialized {@code CorsSupport} instance + */ + public static CorsSupport createMapped(Config config) { + if (!config.exists()) { + throw MissingValueException.create(config.key()); + } + return builder().mappedConfig(config).build(); } @Override diff --git a/webserver/cors/src/main/java/io/helidon/webserver/cors/CorsSupportBase.java b/webserver/cors/src/main/java/io/helidon/webserver/cors/CorsSupportBase.java index c7b7732b7fc..7f2e9e57030 100644 --- a/webserver/cors/src/main/java/io/helidon/webserver/cors/CorsSupportBase.java +++ b/webserver/cors/src/main/java/io/helidon/webserver/cors/CorsSupportBase.java @@ -131,6 +131,18 @@ public B config(Config config) { return me(); } + /** + * Merges mapped CORS config information. Typically, the app or component will retrieve the provided {@code Config} + * instance from its own config. + * + * @param config the mapped CORS config information + * @return the updated builder + */ + public B mappedConfig(Config config) { + helperBuilder.mappedConfig(config); + return me(); + } + /** * Sets whether CORS support should be enabled or not. * diff --git a/webserver/cors/src/main/java/io/helidon/webserver/cors/CorsSupportHelper.java b/webserver/cors/src/main/java/io/helidon/webserver/cors/CorsSupportHelper.java index aa267ef3b4d..cdb525c12e1 100644 --- a/webserver/cors/src/main/java/io/helidon/webserver/cors/CorsSupportHelper.java +++ b/webserver/cors/src/main/java/io/helidon/webserver/cors/CorsSupportHelper.java @@ -150,16 +150,6 @@ public enum RequestType { HelidonFeatures.register(HelidonFlavor.SE, "CORS"); } - /** - * Creates a new instance using CORS config in the provided {@link Config}. - * - * @param config Config node containing CORS set-up - * @return new instance based on the config - */ - public static CorsSupportHelper create(Config config) { - return CorsSupportHelper.builder().config(config).build(); - } - /** * Creates a new instance that is enabled but with no path mappings. * @@ -219,6 +209,17 @@ public Builder secondaryLookupSupplier(Supplier config(Config config) { + aggregator.config(config); + return this; + } + + /** + * Adds mapped cross-origin information via config. + * + * @param config config node containing mapped CORS set-up information + * @return updated builder + */ + public Builder mappedConfig(Config config) { aggregator.mappedConfig(config); return this; } diff --git a/webserver/cors/src/test/java/io/helidon/webserver/cors/TestUtil.java b/webserver/cors/src/test/java/io/helidon/webserver/cors/TestUtil.java index 2fba3592e6f..5a2521ca6bf 100644 --- a/webserver/cors/src/test/java/io/helidon/webserver/cors/TestUtil.java +++ b/webserver/cors/src/test/java/io/helidon/webserver/cors/TestUtil.java @@ -71,16 +71,28 @@ static Routing.Builder prepRouting() { * Load a specific config for "/othergreet." */ Config twoCORSConfig = minimalConfig(ConfigSources.classpath("twoCORS.yaml")); + Config twoCORSMappedConfig = twoCORSConfig.get("cors-2-setup"); + if (!twoCORSMappedConfig.exists()) { + throw new IllegalArgumentException("Expected config 'cors-2-setup' from twoCORS.yaml not found"); + } + Config somewhatRestrictedConfig = twoCORSConfig.get("somewhat-restrictive"); + if (!somewhatRestrictedConfig.exists()) { + throw new IllegalArgumentException("Expected config 'somewhat-restrictive' from twoCORS.yaml not found"); + } + Config corsMappedSetupConfig = Config.create().get("cors-setup"); + if (!corsMappedSetupConfig.exists()) { + throw new IllegalArgumentException("Expected config 'cors-setup' from default app config not found"); + } Routing.Builder builder = Routing.builder() .register(GREETING_PATH, - CorsSupport.builder().config(Config.create().get("cors-setup")).build(), + CorsSupport.createMapped(corsMappedSetupConfig), new GreetService()) .register(OTHER_GREETING_PATH, - CorsSupport.builder().config(twoCORSConfig.get("cors-2-setup")).build(), + CorsSupport.createMapped(twoCORSMappedConfig), new GreetService("Other Hello")) .any(TestHandlerRegistration.CORS4_CONTEXT_ROOT, - CorsSupport.create(twoCORSConfig.get("somewhat-restrictive")), // handler settings from config subnode + CorsSupport.create(somewhatRestrictedConfig), // handler settings from config subnode (req, resp) -> resp.status(Http.Status.OK_200).send()) .get(TestHandlerRegistration.CORS4_CONTEXT_ROOT, // handler settings in-line CorsSupport.builder() From 2ea0504d3e3ca34a698d3655b98d1c8899656803 Mon Sep 17 00:00:00 2001 From: "tim.quinn@oracle.com" Date: Wed, 29 Apr 2020 11:57:23 -0500 Subject: [PATCH 2/2] MP CORS filter should make sure the 'cors' config node exists before using it to build CorsSupportMp --- .../java/io/helidon/microprofile/cors/CrossOriginFilter.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/microprofile/cors/src/main/java/io/helidon/microprofile/cors/CrossOriginFilter.java b/microprofile/cors/src/main/java/io/helidon/microprofile/cors/CrossOriginFilter.java index 4accfac103c..c6901551630 100644 --- a/microprofile/cors/src/main/java/io/helidon/microprofile/cors/CrossOriginFilter.java +++ b/microprofile/cors/src/main/java/io/helidon/microprofile/cors/CrossOriginFilter.java @@ -64,7 +64,9 @@ class CrossOriginFilter implements ContainerRequestFilter, ContainerResponseFilt CrossOriginFilter() { Config config = (Config) ConfigProvider.getConfig(); - cors = CorsSupportMp.builder().mappedConfig(config.get(CORS_CONFIG_KEY)) + CorsSupportMp.Builder corsBuilder = CorsSupportMp.builder(); + config.get(CORS_CONFIG_KEY).ifExists(corsBuilder::mappedConfig); + cors = corsBuilder .secondaryLookupSupplier(this::crossOriginFromAnnotationSupplier) .build(); }