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

CORS Middleware docs example doesn't generate correct headers #2361

Closed
CloudNiner opened this issue Jan 17, 2019 · 2 comments

Comments

Projects
None yet
2 participants
@CloudNiner
Copy link
Contributor

commented Jan 17, 2019

The instructions on the documentation site for adding CORS to a server do not work as expected. I would expect to see a longer response headers list that includes the appropriate CORS headers based on the default allow all CORSConfig().

Using:

  • Scala 2.11.12
  • http4s 0.20.0-M5

I typed the entirety of the first set of examples up to the Configuration section into my project REPL and did not see CORS headers included in the response headers (the intermediate instructions that demonstrate a non-CORS request/response are omitted for simplicity):

import cats.effect._
import org.http4s._
import org.http4s.dsl.io._
import org.http4s.implicits._
import org.http4s.server.middleware._
val service = HttpRoutes.of[IO] {
  case _ =>
    Ok()
}
val corsService = CORS(service)
val originHeader = Header("Origin", "somewhere.com")
val request = Request[IO](Method.GET, Uri.uri("/"))
val corsRequest = request.putHeaders(originHeader)
val corsResponse = corsService.orNotFound(corsRequest).unsafeRunSync
// corsResponse: org.http4s.Response[cats.effect.IO] = Response(status=200, headers=Headers(Content-Length: 0))
corsResponse.headers
// res0: org.http4s.Headers = Headers(Content-Length: 0)
corsResponse.headers.size
// res1: Int = 1

I see the same result (headers not included) when I include the CORS middleware in my project:

import cats.effect._
import cats.implicits._
import org.http4s._
import org.http4s.dsl.io._
import org.http4s.implicits._
import org.http4s.server.blaze._
import org.http4s.server.middleware._
import org.http4s.server.Router

object MyServer extends IOApp {
  def run(args: List[String]): IO[ExitCode] =
    ServerStream.stream[IO].compile.drain.as(ExitCode.Success)
}

object ServerStream {
  def service[F[_]: Effect]: HttpRoutes[F] =
    new MyService[F].routes

  def stream[F[_]: ConcurrentEffect: Timer]: fs2.Stream[F, ExitCode] =
    BlazeServerBuilder[F]
      .bindHttp(8080, "0.0.0.0")
      .withHttpApp(Router("/" -> CORS(service)).orNotFound)
      .serve
}

Then making a request to that server with HTTPie:

http --print=Hh :4044/api/data Origin:foo.com
GET /api/data HTTP/1.1
Accept: */*
Accept-Encoding: gzip, deflate
Connection: keep-alive
Host: localhost:4044
Origin: foo.com
User-Agent: HTTPie/0.9.9

HTTP/1.1 200 OK
Content-Length: 1359
Content-Type: application/json
Date: Thu, 17 Jan 2019 14:09:17 GMT

@CloudNiner CloudNiner changed the title CORS Middleware doesn't generate appropriate headers on documentation site CORS Middleware docs example doesn't generate correct headers Jan 17, 2019

@vglushak

This comment has been minimized.

Copy link

commented Jan 25, 2019

I've encountered the same issue.
Something changed in Origin header parser.
If you provide header like "https://somewhere.com" (instead of somewhere.com) it will add necessary CORS headers in response.

CloudNiner added a commit to CloudNiner/http4s that referenced this issue Jan 25, 2019

Include scheme in CORS middleware examples
Clarifies CORS documentation based on a comment in

http4s#2361 (comment)

It remains unclear to me if this is a bug or desired change in behavior
so I made no further attempts to modify source or test code.
@CloudNiner

This comment has been minimized.

Copy link
Contributor Author

commented Feb 1, 2019

Docs PR #2374 was merged to clarify behavior. Given that scheme is a required part of the Origin header this can be closed. I wouldn't expect this middleware to support appending proper CORS headers for an out-of-spec Origin header.

@CloudNiner CloudNiner closed this Feb 1, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.