Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HTTP spec conformance during CI #3169

Open
wants to merge 26 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
da4da1b
add process for conformance suite
Saturn225 Sep 26, 2024
969afa9
workflow for conformance suite added
Saturn225 Sep 26, 2024
698dd7c
fix(conformance): forbidden duplicate headers
Saturn225 Sep 30, 2024
bb47958
fix(conformance): tests and move to other suite for e2e validation
Saturn225 Sep 30, 2024
453c609
chore: cleanup and fix 404 and 405 tests
Saturn225 Sep 30, 2024
73a209b
Update ServerInboundHandler.scala
Saturn225 Sep 30, 2024
1e66f64
Update Routes.scala
Saturn225 Oct 1, 2024
1948dc4
feat(conformance): 404
Saturn225 Oct 1, 2024
6514ff7
chore(conformance): cleanup netty exception tests
Saturn225 Oct 1, 2024
943167c
fix(conformance): fmt
Saturn225 Oct 1, 2024
5257f23
remove netty exceptions added
Saturn225 Oct 1, 2024
b609cd9
fix
Saturn225 Oct 1, 2024
62e78a3
feat(conformance): add review comments
Saturn225 Oct 2, 2024
70d026e
Merge branch 'test-1' into feat/conformance-spec
Saturn225 Oct 10, 2024
f2ccf4a
Handle OPTIONS Method
Saturn225 Oct 19, 2024
66dbe0d
fix in TestServer for validation
Saturn225 Oct 19, 2024
864d676
remove dev logs
Saturn225 Oct 19, 2024
6a29249
Merge branch 'main' into feat/conformance-spec
Saturn225 Oct 19, 2024
957b35f
fix: conformance tests
Saturn225 Oct 26, 2024
265e7e6
Merge branch 'main' into feat/conformance-spec
987Nabil Oct 29, 2024
9b0638e
Merge branch 'main' into feat/conformance-spec
Saturn225 Nov 6, 2024
ab4f1fd
Merge branch 'main' into feat/conformance-spec
Saturn225 Dec 9, 2024
e165ef1
Relax Host header validation logic to allow broader compatibility
Saturn225 Dec 11, 2024
078c76b
Merge branch 'main' into feat/conformance-spec
987Nabil Dec 14, 2024
769e451
update build pipeline for conformance tests
Saturn225 Dec 14, 2024
ecd6797
Merge branch 'main' into feat/conformance-spec
Saturn225 Dec 14, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 73 additions & 0 deletions .github/workflows/http-conformance.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
name: HTTP Conformance

on:
pull_request:
branches: ["**"]
push:
branches: ["**"]
tags: [v*]

env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
JDK_JAVA_OPTIONS: "-Xms4G -Xmx8G -XX:+UseG1GC -Xss10M -XX:ReservedCodeCacheSize=1G -XX:NonProfiledCodeHeapSize=512m -Dfile.encoding=UTF-8"
SBT_OPTS: "-Xms4G -Xmx8G -XX:+UseG1GC -Xss10M -XX:ReservedCodeCacheSize=1G -XX:NonProfiledCodeHeapSize=512m -Dfile.encoding=UTF-8"

jobs:
build:
name: Build and Test
strategy:
matrix:
os: [ubuntu-latest]
scala: [2.12.19, 2.13.14, 3.3.3]
java:
- graal_graalvm@17
- graal_graalvm@21
- temurin@17
- temurin@21
runs-on: ${{ matrix.os }}
timeout-minutes: 60

steps:
- name: Checkout current branch (full)
uses: actions/checkout@v4
with:
fetch-depth: 0

- name: Setup GraalVM (graal_graalvm@17)
if: matrix.java == 'graal_graalvm@17'
uses: graalvm/setup-graalvm@v1
with:
java-version: 17
distribution: graalvm
components: native-image
github-token: ${{ secrets.GITHUB_TOKEN }}
cache: sbt

- name: Setup GraalVM (graal_graalvm@21)
if: matrix.java == 'graal_graalvm@21'
uses: graalvm/setup-graalvm@v1
with:
java-version: 21
distribution: graalvm
components: native-image
github-token: ${{ secrets.GITHUB_TOKEN }}
cache: sbt

- name: Setup Java (temurin@17)
if: matrix.java == 'temurin@17'
uses: actions/setup-java@v4
with:
distribution: temurin
java-version: 17
cache: sbt

- name: Setup Java (temurin@21)
if: matrix.java == 'temurin@21'
uses: actions/setup-java@v4
with:
distribution: temurin
java-version: 21
cache: sbt

- name: Run HTTP Conformance Tests
run: sbt "project zioHttpJVM" "testOnly zio.http.ConformanceSpec zio.http.ConformanceE2ESpec"
Original file line number Diff line number Diff line change
Expand Up @@ -87,12 +87,19 @@ private[zio] final case class ServerInboundHandler(
)
releaseRequest()
} else {
val req = makeZioRequest(ctx, jReq)
val exit = handler(req)
if (attemptImmediateWrite(ctx, req.method, exit)) {
val req = makeZioRequest(ctx, jReq)
if (!validateHostHeader(req)) {
attemptFastWrite(ctx, req.method, Response.status(Status.BadRequest))
releaseRequest()
} else {
writeResponse(ctx, runtime, exit, req)(releaseRequest)

val exit = handler(req)
if (attemptImmediateWrite(ctx, req.method, exit)) {
releaseRequest()
} else {
writeResponse(ctx, runtime, exit, req)(releaseRequest)

}
}
}
} finally {
Expand All @@ -108,6 +115,32 @@ private[zio] final case class ServerInboundHandler(

}

private def validateHostHeader(req: Request): Boolean = {
req.headers.get("Host") match {
Saturn225 marked this conversation as resolved.
Show resolved Hide resolved
case Some(host) =>
val parts = host.split(":")
val hostname = parts(0)
val isValidHost = validateHostname(hostname)
val isValidPort = parts.length == 1 || (parts.length == 2 && parts(1).forall(_.isDigit))
val isValid = isValidHost && isValidPort
isValid
case None =>
false
}
}

// Validate a regular hostname (based on RFC 1035)
private def validateHostname(hostname: String): Boolean = {
if (hostname.isEmpty || hostname.contains("_")) {
return false
}
val labels = hostname.split("\\.")
if (labels.exists(label => label.isEmpty || label.length > 63 || label.startsWith("-") || label.endsWith("-"))) {
return false
}
hostname.forall(c => c.isLetterOrDigit || c == '.' || c == '-') && hostname.length <= 253
}

override def exceptionCaught(ctx: ChannelHandlerContext, cause: Throwable): Unit =
cause match {
case ioe: IOException if {
Expand Down Expand Up @@ -262,7 +295,6 @@ private[zio] final case class ServerInboundHandler(
remoteCertificate = clientCert,
)
}

}

/*
Expand Down
51 changes: 51 additions & 0 deletions zio-http/jvm/src/test/scala/zio/http/ConformanceE2ESpec.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
package zio.http

import zio._
import zio.test.Assertion._
import zio.test.TestAspect._
import zio.test._

import zio.http._
import zio.http.internal.{DynamicServer, RoutesRunnableSpec}
import zio.http.netty.NettyConfig

object ConformanceE2ESpec extends RoutesRunnableSpec {

private val port = 8080
private val MaxSize = 1024 * 10
val configApp = Server.Config.default
.requestDecompression(true)
.disableRequestStreaming(MaxSize)
.port(port)
.responseCompression()

private val app = serve

def conformanceSpec = suite("ConformanceE2ESpec")(
test("should return 400 Bad Request if Host header is missing") {
val routes = Handler.ok.toRoutes

val res = routes.deploy.status.run(path = Path.root, headers = Headers(Header.Host("%%%%invalid%%%%")))
assertZIO(res)(equalTo(Status.BadRequest))
},
test("should return 200 OK if Host header is present") {
val routes = Handler.ok.toRoutes

val res = routes.deploy.status.run(path = Path.root, headers = Headers(Header.Host("localhost")))
assertZIO(res)(equalTo(Status.Ok))
},
)

override def spec =
suite("ConformanceE2ESpec") {
val spec = conformanceSpec
suite("app without request streaming") { app.as(List(spec)) }
}.provideShared(
DynamicServer.live,
ZLayer.succeed(configApp),
Server.customized,
Client.default,
ZLayer.succeed(NettyConfig.default),
) @@ sequential @@ withLiveClock

}
Loading
Loading