Fix missing header rejection for custom headers #2487 (#2504)

When the HeaderMagnet is formed for a user-defined custom header, it
used the class' name to populate the `headerName` field. However, the
user may have named their class differently than its name, by excluding
dashes, for example.

This could produce an incosistency in behavior when routes would
generate a `MissingHeaderRejection` for a custom header because the
`headerValueByType` directive constructs a `MissingHeaderRejection` with
a `headerName` populated from the magnet, which in the case of a custom
header, comes from the header's class name rather than its name field.

Since the magnet implicits have access to the user-defined header name
in the `ModeledCustomHeaderCompanion` object, we should take advantage
of it: This change updates the implicit to override the `headerName`
behavior by setting it to the companion's `name` field.

I considered using an alternate approach of analyzing the class passed
into the `ModeledCompanion.nameFromClass` method. However, that required
using reflection and potentially had broader side-effects since it would
change the underlying behavior of a public trait.

Fixes #2487.

akka-http-tests/src/test/scala/akka/http/scaladsl/server/ModeledCustomHeaderSpec.scala

@@ -120,7 +120,7 @@ class ModeledCustomHeaderSpec extends RoutingSpec {
       }
 
       Get().withHeaders(RawHeader("somethingElse", "TheKey")) ~> routes ~> check {
-        rejection should ===(MissingHeaderRejection("ApiTokenHeader"))
+        rejection should ===(MissingHeaderRejection("apiKey"))
       }
 
       Get().withHeaders(ApiTokenHeader("TheKey")) ~> routes ~> check {

akka-http-tests/src/test/scala/akka/http/scaladsl/server/directives/HeaderDirectivesSpec.scala

@@ -7,7 +7,11 @@ package akka.http.scaladsl.server.directives
 import akka.http.scaladsl.model._
 import headers._
 import akka.http.scaladsl.server._
+import directives.HeaderDirectivesSpec.XCustomHeader
+
 import org.scalatest.Inside
+import scala.reflect.ClassTag
+import scala.util.Try
 
 class HeaderDirectivesSpec extends RoutingSpec with Inside {
 
@@ -61,6 +65,16 @@ class HeaderDirectivesSpec extends RoutingSpec with Inside {
         }
       }
     }
+    "reject a request if no header matches a custom one, and use the custom header's name for the rejection " in {
+      val route = headerValueByType[XCustomHeader]() { customValue ⇒
+        complete(s"Custom-Value: $customValue")
+      }
+      Get("abc") ~> route ~> check {
+        inside(rejection) {
+          case MissingHeaderRejection("X-Custom-Header") ⇒
+        }
+      }
+    }
   }
 
   "The headerValueByName directive" should {
@@ -202,4 +216,30 @@ class HeaderDirectivesSpec extends RoutingSpec with Inside {
       }
     }
   }
+
+  "The HeaderMagnet" should {
+    "get the header name from the ModeledCustomHeaderCompanion" in {
+      val unitMagnet: HeaderMagnet[XCustomHeader] = {}
+      unitMagnet.headerName shouldEqual "X-Custom-Header"
+
+      val classMagnet = HeaderMagnet.fromClassForModeledCustomHeader(classOf[XCustomHeader], XCustomHeader)
+      classMagnet.headerName shouldEqual "X-Custom-Header"
+
+      val classTagMagnet = HeaderMagnet.fromClassTagForModeledCustomHeader(ClassTag(classOf[XCustomHeader]), XCustomHeader)
+      classTagMagnet.headerName shouldEqual "X-Custom-Header"
+    }
+  }
+}
+
+object HeaderDirectivesSpec {
+  final object XCustomHeader extends ModeledCustomHeaderCompanion[XCustomHeader] {
+    override def name: String = "X-Custom-Header"
+    override def parse(value: String): Try[XCustomHeader] = Try(new XCustomHeader)
+  }
+  final class XCustomHeader extends ModeledCustomHeader[XCustomHeader] {
+    override def companion: ModeledCustomHeaderCompanion[XCustomHeader] = XCustomHeader
+    override def value(): String = "custom-value"
+    override def renderInRequests(): Boolean = true
+    override def renderInResponses(): Boolean = false
+  }
 }

akka-http/src/main/scala/akka/http/scaladsl/server/directives/HeaderDirectives.scala

@@ -159,7 +159,7 @@ object HeaderDirectives extends HeaderDirectives
 trait HeaderMagnet[T] {
   def classTag: ClassTag[T]
   def runtimeClass: Class[T]
-  def headerName = ModeledCompanion.nameFromClass(runtimeClass)
+  def headerName: String = ModeledCompanion.nameFromClass(runtimeClass)
 
   /**
    * Returns a partial function that checks if the input value is of runtime type
@@ -181,11 +181,12 @@ object HeaderMagnet extends LowPriorityHeaderMagnetImplicits {
 
   implicit def fromClassTagForModeledCustomHeader[T <: ModeledCustomHeader[T], H <: ModeledCustomHeaderCompanion[T]](tag: ClassTag[T], companion: ModeledCustomHeaderCompanion[T]): HeaderMagnet[T] =
     new HeaderMagnet[T] {
-      override def runtimeClass = tag.runtimeClass.asInstanceOf[Class[T]]
-      override def classTag = tag
-      override def extractPF = {
+      override def classTag: ClassTag[T] = tag
+      override def runtimeClass: Class[T] = tag.runtimeClass.asInstanceOf[Class[T]]
+      override def extractPF: PartialFunction[HttpHeader, T] = {
         case h if h.is(companion.lowercaseName) => companion.apply(h.value)
       }
+      override def headerName: String = companion.name
     }
 
 }
@@ -199,7 +200,9 @@ trait LowPriorityHeaderMagnetImplicits {
     new HeaderMagnet[T] {
       override def classTag: ClassTag[T] = ClassTag(clazz)
       override def runtimeClass: Class[T] = clazz
-      override def extractPF: PartialFunction[HttpHeader, T] = { case x if runtimeClass.isAssignableFrom(x.getClass) => x.asInstanceOf[T] }
+      override def extractPF: PartialFunction[HttpHeader, T] = {
+        case x if runtimeClass.isAssignableFrom(x.getClass) => x.asInstanceOf[T]
+      }
     }
 
   implicit def fromUnitNormalHeader[T <: HttpHeader](u: Unit)(implicit tag: ClassTag[T]): HeaderMagnet[T] =
@@ -209,6 +212,8 @@ trait LowPriorityHeaderMagnetImplicits {
     new HeaderMagnet[T] {
       val classTag: ClassTag[T] = tag
       val runtimeClass: Class[T] = tag.runtimeClass.asInstanceOf[Class[T]]
-      val extractPF: PartialFunction[Any, T] = { case x if runtimeClass.isAssignableFrom(x.getClass) => x.asInstanceOf[T] }
+      val extractPF: PartialFunction[Any, T] = {
+        case x if runtimeClass.isAssignableFrom(x.getClass) => x.asInstanceOf[T]
+      }
     }
 }