[GitHub] [incubator-pekko-http] lomigmegard commented on a diff in pull request #208: Add pekko-http-cors module

["lomigmegard (via GitHub)" <gi...@apache.org>]

lomigmegard commented on code in PR #208:
URL: https://github.com/apache/incubator-pekko-http/pull/208#discussion_r1246682068


##########
docs/src/main/paradox/migration-guide/index.md:
##########
@@ -11,3 +11,9 @@
 * Where class names have "Akka" in the name, the Pekko ones have "Pekko" - e.g. PekkoException instead of AkkaException
 * Configs in `application.conf` use "pekko" prefix instead of "akka"
 * We will soon provide a more detailed guide for migrating from Akka HTTP v10.2 to Apache Pekko HTTP
+* If you happen to be using [akka-http-cors](https://github.com/lomigmegard/akka-http-cors) this has been directly integrated into
+  Apache Pekko Http which means you should use the [pekko-http-cors artifact](common/cors.md) instead. Note that the root configuration naming 
+  has for cors has also been updated (i.e. changing from `akka-http-cors` to `pekko.http.cors`) to make it consistent with the

Review Comment:
   typo: remove `has for cors`
   
   ```suggestion
     has also been updated (i.e. changing from `akka-http-cors` to `pekko.http.cors`) to make it consistent with the
   ```



##########
http-cors/src/main/scala/org/apache/pekko/http/cors/scaladsl/settings/CorsSettings.scala:
##########
@@ -0,0 +1,236 @@
+package org.apache.pekko.http.cors.scaladsl.settings
+
+import java.util.OptionalLong
+import java.util.concurrent.TimeUnit
+import org.apache.pekko
+import pekko.actor.ActorSystem
+import pekko.annotation.DoNotInherit
+import pekko.http.scaladsl.model.headers.HttpOrigin
+import pekko.http.scaladsl.model.{ HttpHeader, HttpMethod, HttpMethods }
+import pekko.http.cors.javadsl
+import pekko.http.cors.scaladsl.model.{ HttpHeaderRange, HttpOriginMatcher }
+import pekko.util.OptionConverters._
+import com.typesafe.config.ConfigException.{ Missing, WrongType }
+import com.typesafe.config.{ Config, ConfigFactory }
+import org.apache.pekko.http.cors.javadsl.{ model => jmcorsmodel }
+import org.apache.pekko.http.javadsl.{ model => jmmodel }
+
+import scala.collection.JavaConverters._
+import scala.collection.immutable.{ ListMap, Seq }
+import scala.util.Try
+
+/**
+ * Settings used by the CORS directives.
+ *
+ * Public API but not intended for subclassing.
+ */
+@DoNotInherit
+abstract class CorsSettings private[pekko] () extends javadsl.settings.CorsSettings { self: CorsSettingsImpl =>
+
+  /**
+   * If `true`, allow generic requests (that are outside the scope of the specification) to pass through the directive.
+   * Else, strict CORS filtering is applied and any invalid request will be rejected.
+   *
+   * Default: `true`
+   */
+  def allowGenericHttpRequests: Boolean
+
+  /**
+   * Indicates whether the resource supports user credentials. If `true`, the header `Access-Control-Allow-Credentials`
+   * is set in the response, indicating that the actual request can include user credentials. Examples of user
+   * credentials are: cookies, HTTP authentication or client-side certificates.
+   *
+   * Default: `true`
+   *
+   * @see
+   *   [[https://www.w3.org/TR/cors/#access-control-allow-credentials-response-header Access-Control-Allow-Credentials]]
+   */
+  def allowCredentials: Boolean
+
+  /**
+   * List of origins that the CORS filter must allow. Can also be set to `*` to allow access to the resource from any
+   * origin. Controls the content of the `Access-Control-Allow-Origin` response header: if parameter is `*` and
+   * credentials are not allowed, a `*` is set in `Access-Control-Allow-Origin`. Otherwise, the origins given in the
+   * `Origin` request header are echoed.
+   *
+   * Hostname starting with `*.` will match any sub-domain. The scheme and the port are always strictly matched.
+   *
+   * The actual or preflight request is rejected if any of the origins from the request is not allowed.
+   *
+   * Default: `HttpOriginMatcher.*`
+   *
+   * @see
+   *   [[https://www.w3.org/TR/cors/#access-control-allow-origin-response-header Access-Control-Allow-Origin]]
+   */
+  def allowedOrigins: HttpOriginMatcher
+
+  /**
+   * List of request headers that can be used when making an actual request. Controls the content of the
+   * `Access-Control-Allow-Headers` header in a preflight response: if parameter is `*`, the headers from
+   * `Access-Control-Request-Headers` are echoed. Otherwise the parameter list is returned as part of the header.
+   *
+   * Default: `HttpHeaderRange.*`
+   *
+   * @see
+   *   [[https://www.w3.org/TR/cors/#access-control-allow-headers-response-header Access-Control-Allow-Headers]]
+   */
+  def allowedHeaders: HttpHeaderRange
+
+  /**
+   * List of methods that can be used when making an actual request. The list is returned as part of the
+   * `Access-Control-Allow-Methods` preflight response header.
+   *
+   * The preflight request will be rejected if the `Access-Control-Request-Method` header's method is not part of the
+   * list.
+   *
+   * Default: `Seq(GET, POST, HEAD, OPTIONS)`
+   *
+   * @see
+   *   [[https://www.w3.org/TR/cors/#access-control-allow-methods-response-header Access-Control-Allow-Methods]]
+   */
+  def allowedMethods: Seq[HttpMethod]
+
+  /**
+   * List of headers (other than simple response headers) that browsers are allowed to access. If not empty, this list
+   * is returned as part of the `Access-Control-Expose-Headers` header in the actual response.
+   *
+   * Default: `Seq.empty`
+   *
+   * @see
+   *   [[https://www.w3.org/TR/cors/#simple-response-header Simple response headers]]
+   * @see
+   *   [[https://www.w3.org/TR/cors/#access-control-expose-headers-response-header Access-Control-Expose-Headers]]
+   */
+  def exposedHeaders: Seq[String]
+
+  /**
+   * When set, the amount of seconds the browser is allowed to cache the results of a preflight request. This value is
+   * returned as part of the `Access-Control-Max-Age` preflight response header. If `None`, the header is not added to
+   * the preflight response.
+   *
+   * Default: `Some(30 * 60)`
+   *
+   * @see
+   *   [[https://www.w3.org/TR/cors/#access-control-max-age-response-header Access-Control-Max-Age]]
+   */
+  def maxAge: Option[Long]
+
+  /* Java APIs */
+
+  override def getAllowGenericHttpRequests: Boolean = this.allowGenericHttpRequests
+  override def getAllowCredentials: Boolean = this.allowCredentials
+  override def getAllowedOrigins: jmcorsmodel.HttpOriginMatcher = this.allowedOrigins
+  override def getAllowedHeaders: jmcorsmodel.HttpHeaderRange = this.allowedHeaders
+  override def getAllowedMethods: java.lang.Iterable[jmmodel.HttpMethod] =
+    (this.allowedMethods: Seq[org.apache.pekko.http.javadsl.model.HttpMethod]).asJava
+  override def getExposedHeaders: java.lang.Iterable[String] = this.exposedHeaders.asJava
+  override def getMaxAge: OptionalLong = this.maxAge.toJavaPrimitive
+
+  // Currently the easiest way to go from Java models to their Scala equivalent is to cast.
+  // See https://github.com/akka/akka-http/issues/661 for a potential opening of the JavaMapping API.
+  override def withAllowGenericHttpRequests(newValue: Boolean): CorsSettings = {
+    copy(allowGenericHttpRequests = newValue)
+  }
+  override def withAllowCredentials(newValue: Boolean): CorsSettings = {
+    copy(allowCredentials = newValue)
+  }
+  override def withAllowedOrigins(newValue: javadsl.model.HttpOriginMatcher): CorsSettings = {
+    copy(allowedOrigins = newValue.asInstanceOf[HttpOriginMatcher])
+  }
+  override def withAllowedHeaders(newValue: javadsl.model.HttpHeaderRange): CorsSettings = {
+    copy(allowedHeaders = newValue.asInstanceOf[HttpHeaderRange])
+  }
+  override def withAllowedMethods(
+      newValue: java.lang.Iterable[org.apache.pekko.http.javadsl.model.HttpMethod]): CorsSettings = {
+    copy(allowedMethods = newValue.asScala.toList.asInstanceOf[List[HttpMethod]])
+  }
+  override def withExposedHeaders(newValue: java.lang.Iterable[String]): CorsSettings = {
+    copy(exposedHeaders = newValue.asScala.toList)
+  }
+  override def withMaxAge(newValue: OptionalLong): CorsSettings = {
+    copy(maxAge = newValue.toScala)
+  }
+
+  // overloads for Scala idiomatic use
+  def withAllowedOrigins(newValue: HttpOriginMatcher): CorsSettings = copy(allowedOrigins = newValue)
+  def withAllowedHeaders(newValue: HttpHeaderRange): CorsSettings = copy(allowedHeaders = newValue)
+  def withAllowedMethods(newValue: Seq[HttpMethod]): CorsSettings = copy(allowedMethods = newValue)
+  def withExposedHeaders(newValue: Seq[String]): CorsSettings = copy(exposedHeaders = newValue)
+  def withMaxAge(newValue: Option[Long]): CorsSettings = copy(maxAge = newValue)
+
+  private[pekko] def preflightResponseHeaders(origins: Seq[HttpOrigin], requestHeaders: Seq[String]): List[HttpHeader]
+  private[pekko] def actualResponseHeaders(origins: Seq[HttpOrigin]): List[HttpHeader]
+}
+
+object CorsSettings {
+  private val prefix = "pekko.http.cors"
+  final private val MaxCached = 8
+  private[this] var cache = ListMap.empty[ActorSystem, CorsSettings]
+
+  /**
+   * Creates an instance of settings using the given Config.
+   */
+  def apply(config: Config): CorsSettings =
+    fromSubConfig(config.getConfig(prefix))
+
+  /**
+   * Creates an instance of settings using the given String of config overrides to override settings set in the class
+   * loader of this class (i.e. by application.conf or reference.conf files in the class loader of this class).
+   */
+  def apply(configOverrides: String): CorsSettings =
+    apply(ConfigFactory.parseString(configOverrides).withFallback(ConfigFactory.load(getClass.getClassLoader)))
+
+  /**
+   * Creates an instance of CorsSettings using the configuration provided by the given ActorSystem.
+   */
+  def apply(system: ActorSystem): CorsSettings =
+    // From private pekko.http.impl.util.SettingsCompanionImpl implementation

Review Comment:
   The `SettingsCompanionImpl` could now be used, to remove the copy-pasted code



##########
http-cors/src/main/scala/org/apache/pekko/http/cors/javadsl/CorsDirectives.scala:
##########
@@ -0,0 +1,36 @@
+package org.apache.pekko.http.cors.javadsl
+
+import java.util.function.Supplier
+
+import org.apache.pekko
+import pekko.http.javadsl.server.{ RejectionHandler, Route }
+import pekko.http.javadsl.server.directives.RouteAdapter
+import pekko.http.cors.javadsl.settings.CorsSettings
+import pekko.http.cors.scaladsl
+
+object CorsDirectives {
+
+  def cors(inner: Supplier[Route]): Route =
+    RouteAdapter {
+      scaladsl.CorsDirectives.cors() {
+        inner.get() match {
+          case ra: RouteAdapter => ra.delegate
+        }
+      }
+    }
+
+  def cors(settings: CorsSettings, inner: Supplier[Route]): Route =
+    RouteAdapter {
+      // Currently the easiest way to go from Java models to their Scala equivalent is to cast.
+      // See https://github.com/akka/akka-http/issues/661 for a potential opening of the JavaMapping API.
+      val scalaSettings = settings.asInstanceOf[scaladsl.settings.CorsSettings]

Review Comment:
   Now that the code lives inside pekko, we could actually use `JavaMapping`, if this comment is still relevant. At least the comment should be updated to not point to akka? 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org