Merge pull request #707 from jotak/cors-requests

[HWKMETRICS-554] Fix several issues with CORS
hawkular · Dec 20, 2016 · a77e80f · a77e80f
2 parents b4fb182 + 8eaeb1a
commit a77e80f
Show file tree

Hide file tree

Showing 13 changed files with 169 additions and 200 deletions.
diff --git a/api/metrics-api-jaxrs/pom.xml b/api/metrics-api-jaxrs/pom.xml
@@ -70,6 +70,12 @@
       <version>${project.version}</version>
     </dependency>
 
+    <dependency>
+      <groupId>org.hawkular.commons</groupId>
+      <artifactId>hawkular-cors-jaxrs-filter</artifactId>
+      <version>${version.org.hawkular.commons}</version>
+    </dependency>
+
     <!-- Wildfly provided -->
     <!-- No need to set the jboss-logging scope here since Wildfly BOM includes jboss-logging -->
     <dependency>

diff --git a/...rics-api-jaxrs/src/main/java/org/hawkular/metrics/api/jaxrs/filter/CorsRequestFilter.java b/...rics-api-jaxrs/src/main/java/org/hawkular/metrics/api/jaxrs/filter/CorsRequestFilter.java
@@ -19,14 +19,14 @@
 import java.io.IOException;
 
 import javax.annotation.Priority;
-import javax.ws.rs.HttpMethod;
+import javax.inject.Inject;
 import javax.ws.rs.container.ContainerRequestContext;
 import javax.ws.rs.container.ContainerRequestFilter;
 import javax.ws.rs.container.PreMatching;
-import javax.ws.rs.core.Response;
 import javax.ws.rs.ext.Provider;
 
-import org.hawkular.metrics.api.jaxrs.util.Headers;
+import org.hawkular.jaxrs.filter.cors.CorsFilters;
+import org.hawkular.metrics.api.jaxrs.util.OriginValidation;
 
 /**
  * @author Stefan Negrea
@@ -37,16 +37,11 @@
 @Priority(0)
 public class CorsRequestFilter implements ContainerRequestFilter {
 
+    @Inject
+    OriginValidation validator;
+
     @Override
     public void filter(ContainerRequestContext requestContext) throws IOException {
-        //NOT a CORS request
-        if (requestContext.getHeaderString(Headers.ORIGIN) == null) {
-            return;
-        }
-
-        //It is a CORS pre-flight request, there is no route for it, just return 200
-        if (requestContext.getMethod().equalsIgnoreCase(HttpMethod.OPTIONS)) {
-            requestContext.abortWith(Response.status(Response.Status.OK).build());
-        }
+        CorsFilters.filterRequest(requestContext, validator.getPredicate());
     }
 }
diff --git a/...ics-api-jaxrs/src/main/java/org/hawkular/metrics/api/jaxrs/filter/CorsResponseFilter.java b/...ics-api-jaxrs/src/main/java/org/hawkular/metrics/api/jaxrs/filter/CorsResponseFilter.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2014-2015 Red Hat, Inc. and/or its affiliates
+ * Copyright 2014-2016 Red Hat, Inc. and/or its affiliates
  * and other contributors as indicated by the @author tags.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
@@ -17,38 +17,25 @@
 package org.hawkular.metrics.api.jaxrs.filter;
 
 import static org.hawkular.metrics.api.jaxrs.config.ConfigurationKey.ALLOWED_CORS_ACCESS_CONTROL_ALLOW_HEADERS;
-import static org.hawkular.metrics.api.jaxrs.util.Headers.ACCESS_CONTROL_ALLOW_CREDENTIALS;
-import static org.hawkular.metrics.api.jaxrs.util.Headers.ACCESS_CONTROL_ALLOW_HEADERS;
-import static org.hawkular.metrics.api.jaxrs.util.Headers.ACCESS_CONTROL_ALLOW_METHODS;
-import static org.hawkular.metrics.api.jaxrs.util.Headers.ACCESS_CONTROL_ALLOW_ORIGIN;
-import static org.hawkular.metrics.api.jaxrs.util.Headers.ACCESS_CONTROL_MAX_AGE;
-import static org.hawkular.metrics.api.jaxrs.util.Headers.DEFAULT_CORS_ACCESS_CONTROL_ALLOW_HEADERS;
-import static org.hawkular.metrics.api.jaxrs.util.Headers.DEFAULT_CORS_ACCESS_CONTROL_ALLOW_METHODS;
-import static org.hawkular.metrics.api.jaxrs.util.Headers.ORIGIN;
 
 import java.io.IOException;
 
 import javax.inject.Inject;
 import javax.ws.rs.container.ContainerRequestContext;
 import javax.ws.rs.container.ContainerResponseContext;
 import javax.ws.rs.container.ContainerResponseFilter;
-import javax.ws.rs.core.MultivaluedMap;
-import javax.ws.rs.core.Response;
 import javax.ws.rs.ext.Provider;
 
+import org.hawkular.jaxrs.filter.cors.CorsFilters;
 import org.hawkular.metrics.api.jaxrs.config.Configurable;
 import org.hawkular.metrics.api.jaxrs.config.ConfigurationProperty;
-import org.hawkular.metrics.api.jaxrs.util.OriginValidation;
 
 /**
  * @author Stefan Negrea
  */
 @Provider
 public class CorsResponseFilter implements ContainerResponseFilter {
 
-    @Inject
-    OriginValidation validator;
-
     @Inject
     @Configurable
     @ConfigurationProperty(ALLOWED_CORS_ACCESS_CONTROL_ALLOW_HEADERS)
@@ -57,27 +44,9 @@ public class CorsResponseFilter implements ContainerResponseFilter {
     @Override
     public void filter(ContainerRequestContext requestContext, ContainerResponseContext responseContext)
             throws IOException {
-
-        String requestOrigin = requestContext.getHeaderString(ORIGIN);
-        if (requestOrigin == null) {
-            return;
-        }
-
-        if (validator.isAllowedOrigin(requestOrigin)) {
-            MultivaluedMap<String, Object> responseHeaders = responseContext.getHeaders();
-            responseHeaders.add(ACCESS_CONTROL_ALLOW_ORIGIN, requestOrigin);
-            responseHeaders.add(ACCESS_CONTROL_ALLOW_CREDENTIALS, "true");
-            responseHeaders.add(ACCESS_CONTROL_ALLOW_METHODS, DEFAULT_CORS_ACCESS_CONTROL_ALLOW_METHODS);
-            responseHeaders.add(ACCESS_CONTROL_MAX_AGE, 72 * 60 * 60);
-
-            if (extraAccesControlAllowHeaders != null) {
-                responseHeaders.add(ACCESS_CONTROL_ALLOW_HEADERS,
-                        DEFAULT_CORS_ACCESS_CONTROL_ALLOW_HEADERS + "," + extraAccesControlAllowHeaders.trim());
-            } else {
-                responseHeaders.add(ACCESS_CONTROL_ALLOW_HEADERS, DEFAULT_CORS_ACCESS_CONTROL_ALLOW_HEADERS);
-            }
-        } else {
-            responseContext.setStatus(Response.Status.BAD_REQUEST.getStatusCode());
-        }
+        CorsFilters.filterResponse(
+                requestContext,
+                responseContext,
+                extraAccesControlAllowHeaders);
     }
 }
diff --git a/...s-api-jaxrs/src/main/java/org/hawkular/metrics/api/jaxrs/handler/AvailabilityHandler.java b/...s-api-jaxrs/src/main/java/org/hawkular/metrics/api/jaxrs/handler/AvailabilityHandler.java
@@ -317,7 +317,7 @@ public void getData(
                         .flatMap(p -> metricsService.findDataPoints(metricIds, p.getTimeRange().getStart(),
                                 p.getTimeRange().getEnd(), p.getLimit(), p.getOrder())
                                 .observeOn(Schedulers.io())))
-                .subscribe(createNamedDataPointObserver(AVAILABILITY));
+                .subscribe(createNamedDataPointObserver(asyncResponse, AVAILABILITY));
     }
 
     @Deprecated

diff --git a/...etrics-api-jaxrs/src/main/java/org/hawkular/metrics/api/jaxrs/handler/CounterHandler.java b/...etrics-api-jaxrs/src/main/java/org/hawkular/metrics/api/jaxrs/handler/CounterHandler.java
@@ -288,7 +288,7 @@ public void getData(
                             .flatMap(p -> metricsService.findDataPoints(metricIds, p.getTimeRange().getStart(),
                                     p.getTimeRange().getEnd(), p.getLimit(), p.getOrder())
                                 .observeOn(Schedulers.io())))
-                .subscribe(createNamedDataPointObserver(COUNTER));
+                .subscribe(createNamedDataPointObserver(asyncResponse, COUNTER));
     }
 
     @POST
@@ -316,7 +316,7 @@ public void getRateData(
                         .flatMap(p -> metricsService.findRateData(metricIds, p.getTimeRange().getStart(),
                                 p.getTimeRange().getEnd(), p.getLimit(), p.getOrder())
                             .observeOn(Schedulers.io())))
-                .subscribe(createNamedDataPointObserver(COUNTER_RATE));
+                .subscribe(createNamedDataPointObserver(asyncResponse, COUNTER_RATE));
     }
 
     @Deprecated

diff --git a/api/metrics-api-jaxrs/src/main/java/org/hawkular/metrics/api/jaxrs/handler/GaugeHandler.java b/api/metrics-api-jaxrs/src/main/java/org/hawkular/metrics/api/jaxrs/handler/GaugeHandler.java
@@ -321,7 +321,7 @@ public void getData(
                         .flatMap(p -> metricsService.findDataPoints(metricIds, p.getTimeRange().getStart(),
                                 p.getTimeRange().getEnd(), p.getLimit(), p.getOrder())
                                 .observeOn(Schedulers.io())))
-                .subscribe(createNamedDataPointObserver(GAUGE));
+                .subscribe(createNamedDataPointObserver(asyncResponse, GAUGE));
     }
 
     @POST
@@ -349,7 +349,7 @@ public void getRateData(
                         .flatMap(p -> metricsService.findRateData(metricIds, p.getTimeRange().getStart(),
                                 p.getTimeRange().getEnd(), p.getLimit(), p.getOrder())
                                 .observeOn(Schedulers.io())))
-                .subscribe(createNamedDataPointObserver(GAUGE_RATE));
+                .subscribe(createNamedDataPointObserver(asyncResponse, GAUGE_RATE));
     }
 
     @Deprecated

diff --git a/...api-jaxrs/src/main/java/org/hawkular/metrics/api/jaxrs/handler/MetricsServiceHandler.java b/...api-jaxrs/src/main/java/org/hawkular/metrics/api/jaxrs/handler/MetricsServiceHandler.java
@@ -25,9 +25,8 @@
 import java.util.Optional;
 
 import javax.inject.Inject;
-import javax.servlet.http.HttpServletRequest;
-import javax.servlet.http.HttpServletResponse;
 import javax.validation.constraints.NotNull;
+import javax.ws.rs.container.AsyncResponse;
 import javax.ws.rs.core.Context;
 import javax.ws.rs.core.HttpHeaders;
 
@@ -40,7 +39,6 @@
 import org.hawkular.metrics.model.exception.RuntimeApiError;
 import org.hawkular.metrics.model.param.Tags;
 import org.hawkular.metrics.model.param.TimeRange;
-import org.jboss.resteasy.spi.ResteasyProviderFactory;
 
 import com.fasterxml.jackson.databind.ObjectMapper;
 
@@ -64,10 +62,8 @@ protected String getTenant() {
         return httpHeaders.getRequestHeaders().getFirst(TENANT_HEADER_NAME);
     }
 
-    <T> NamedDataPointObserver<T>  createNamedDataPointObserver(MetricType<T> type) {
-        HttpServletRequest request = ResteasyProviderFactory.getContextData(HttpServletRequest.class);
-        HttpServletResponse response = ResteasyProviderFactory.getContextData(HttpServletResponse.class);
-        return new NamedDataPointObserver<>(request, response, mapper, type);
+    <T> NamedDataPointObserver<T> createNamedDataPointObserver(AsyncResponse response, MetricType<T> type) {
+        return new NamedDataPointObserver<>(response, mapper, type);
     }
 
     <T> Observable<MetricId<T>> findMetricsByNameOrTag(List<String> metricNames, String tags, MetricType<T> type) {

diff --git a/...metrics-api-jaxrs/src/main/java/org/hawkular/metrics/api/jaxrs/handler/StringHandler.java b/...metrics-api-jaxrs/src/main/java/org/hawkular/metrics/api/jaxrs/handler/StringHandler.java
@@ -285,7 +285,7 @@ public void getData(
                         .flatMap(p -> metricsService.findDataPoints(metricIds, p.getTimeRange().getStart(),
                                 p.getTimeRange().getEnd(), p.getLimit(), p.getOrder())
                                 .observeOn(Schedulers.io())))
-                .subscribe(createNamedDataPointObserver(STRING));
+                .subscribe(createNamedDataPointObserver(asyncResponse, STRING));
     }
 
     @GET

diff --git a/...src/main/java/org/hawkular/metrics/api/jaxrs/handler/observer/NamedDataPointObserver.java b/...src/main/java/org/hawkular/metrics/api/jaxrs/handler/observer/NamedDataPointObserver.java
@@ -16,12 +16,15 @@
  */
 package org.hawkular.metrics.api.jaxrs.handler.observer;
 
+import java.io.ByteArrayOutputStream;
 import java.io.IOException;
 
-import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
+import javax.ws.rs.container.AsyncResponse;
+import javax.ws.rs.core.MediaType;
+import javax.ws.rs.core.Response;
 
-import org.hawkular.metrics.model.ApiError;
+import org.hawkular.metrics.api.jaxrs.util.ApiUtils;
 import org.hawkular.metrics.model.AvailabilityType;
 import org.hawkular.metrics.model.MetricType;
 import org.hawkular.metrics.model.NamedDataPoint;
@@ -30,7 +33,6 @@
 import com.fasterxml.jackson.core.JsonEncoding;
 import com.fasterxml.jackson.core.JsonGenerator;
 import com.fasterxml.jackson.databind.ObjectMapper;
-import com.google.common.base.Throwables;
 
 import rx.Subscriber;
 
@@ -45,22 +47,18 @@ private interface WriteValue<T> {
         void call(NamedDataPoint<T> dataPoint) throws IOException;
     }
 
-    private final HttpServletRequest request;
-    private final HttpServletResponse response;
-    private final ObjectMapper mapper;
+    private final AsyncResponse response;
     private final JsonGenerator generator;
     private final WriteValue<T> writeValue;
+    private final ByteArrayOutputStream jsonOutputStream;
 
     private volatile String currentMetric;
 
-
-    public NamedDataPointObserver(HttpServletRequest request, HttpServletResponse response, ObjectMapper mapper,
-                                  MetricType<T> type) {
-        this.request = request;
+    public NamedDataPointObserver(AsyncResponse response, ObjectMapper mapper, MetricType<T> type) {
         this.response = response;
-        this.mapper = mapper;
+        jsonOutputStream = new ByteArrayOutputStream();
         try {
-            this.generator = mapper.getFactory().createGenerator(response.getOutputStream(), JsonEncoding.UTF8);
+            this.generator = mapper.getFactory().createGenerator(jsonOutputStream, JsonEncoding.UTF8);
         } catch (IOException e) {
             throw new RuntimeException(e);
         }
@@ -85,9 +83,6 @@ public NamedDataPointObserver(HttpServletRequest request, HttpServletResponse re
     public void onNext(NamedDataPoint<T> dataPoint) {
         try {
             if (currentMetric == null) {
-                response.setStatus(HttpServletResponse.SC_OK);
-                response.setHeader("Content-Type", "application/json");
-
                 generator.writeStartArray();
                 generator.writeStartObject();
                 generator.writeStringField("id", dataPoint.getName());
@@ -125,34 +120,37 @@ public void onError(Throwable e) {
         log.trace("Fetching data failed", e);
         try {
             if (currentMetric == null) {
-                response.setStatus(HttpServletResponse.SC_INTERNAL_SERVER_ERROR);
-                response.setHeader("Content-Type", "application/json");
-                ApiError apiError = new ApiError(Throwables.getRootCause(e).getMessage());
-                mapper.writeValue(response.getOutputStream(), apiError);
+                response.resume(ApiUtils.serverError(e));
             } else {
                 generator.close();
             }
         } catch (IOException ignored) {
-        } finally {
-            request.getAsyncContext().complete();
         }
     }
 
     @Override
     public void onCompleted() {
+        Response result = null;
         try {
             if (currentMetric == null) {
-                response.setStatus(HttpServletResponse.SC_NO_CONTENT);
+                generator.close();
+                result = Response.ok().status(HttpServletResponse.SC_NO_CONTENT).build();
             } else {
                 generator.writeEndArray();
                 generator.writeEndObject();
                 generator.writeEndArray();
+                generator.close();
+                result = Response.ok(new String(jsonOutputStream.toByteArray()), MediaType.APPLICATION_JSON_TYPE)
+                        .build();
             }
-            generator.close();
         } catch (IOException e) {
+            result = ApiUtils.serverError(e);
             log.trace("Error while finishing streaming data", e);
         } finally {
-            request.getAsyncContext().complete();
+            if (result == null) {
+                result = Response.serverError().build();
+            }
+            response.resume(result);
         }
     }
 }
diff --git a/api/metrics-api-jaxrs/src/main/java/org/hawkular/metrics/api/jaxrs/util/Headers.java b/api/metrics-api-jaxrs/src/main/java/org/hawkular/metrics/api/jaxrs/util/Headers.java