Skip to content

Commit

Permalink
Issue 3165: Fix Forwarded Header rendering (#3166)
Browse files Browse the repository at this point in the history
This set of change address the issue described in #3165.

For instance the header, 'Header.Forwarded(forValues = List("127.0.0.1"))', previously rendered incorrectly as 'None; for=127.0.0.1; None; None'.

It now properly renders as 'for=127.0.0.1;'.

It includes an additional test spec that uses a property check to verify all permutations optional directives.
  • Loading branch information
scottweaver authored Sep 27, 2024
1 parent c44912b commit 44b2a1c
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 3 deletions.
19 changes: 18 additions & 1 deletion zio-http/jvm/src/test/scala/zio/http/headers/ForwardedSpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ package zio.http.headers
import zio.Scope
import zio.test._

import zio.http.{Header, ZIOHttpSpec}
import zio.http.{Header, Headers, Request, ZIOHttpSpec}

object ForwardedSpec extends ZIOHttpSpec {
override def spec: Spec[TestEnvironment with Scope, Any] = suite("Forwarded suite")(
Expand Down Expand Up @@ -58,5 +58,22 @@ object ForwardedSpec extends ZIOHttpSpec {
)
assertTrue(Header.Forwarded.parse(headerValue) == Right(header))
},
test("render Forwarded produces a valid raw header value") {
val gen = for {
by <- Gen.option(Gen.const("by.host"))
forv <- Gen.listOf(
Gen.elements("127.0.0.1", "localhost", "0.0.0.0"),
)
host <- Gen.option(Gen.const("host.com"))
proto <- Gen.option(Gen.const("http"))
} yield (by, forv, host, proto)

check(gen) { case (by, forv, host, proto) =>
val expected = Header.Forwarded(by = by, forValues = forv, host = host, proto = proto)
val raw = Header.Forwarded.render(expected)
val actual = Header.Forwarded.parse(raw)
assertTrue(actual.is(_.right) == expected)
}
} @@ TestAspect.shrinks(0),
)
}
9 changes: 7 additions & 2 deletions zio-http/shared/src/main/scala/zio/http/Header.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2856,8 +2856,13 @@ object Header {
Right(Forwarded(by, forValue, host, proto))
}

def render(forwarded: Forwarded): String =
s"${forwarded.by}; ${forwarded.forValues.map(v => s"for=$v").mkString(",")}; ${forwarded.host}; ${forwarded.proto}"
def render(forwarded: Forwarded): String = {
def formatDirective(directive: Option[String]) = directive.map(_ + ";").getOrElse("")

val forValues = if (forwarded.forValues.nonEmpty) forwarded.forValues.map(v => s"for=$v").mkString(",") + ";" else ""

s"${formatDirective(forwarded.by.map("by=" + _))}${forValues}${formatDirective(forwarded.host.map("host=" + _))}${formatDirective(forwarded.proto.map("proto=" + _))}"
}
}

/** From header value. */
Expand Down

0 comments on commit 44b2a1c

Please sign in to comment.