akka / akka-http

The Streaming-first HTTP server/module of Akka
https://doc.akka.io/docs/akka-http
Other
1.34k stars 596 forks source link

Disabling modeled-header-parsing breaks Multipart post requests #2278

Open prateekr opened 5 years ago

prateekr commented 5 years ago

Replicated error on: HEAD of master (25f5eb128e62f303f84f9bcdafd2a7861d604458 currently) My version: v10.0.14

import akka.actor.ActorSystem
import akka.http.scaladsl.client.RequestBuilding
import akka.http.scaladsl.Http
import akka.http.scaladsl.model._
import akka.http.scaladsl.model.headers._
import akka.http.scaladsl.model.MediaTypes
import akka.http.scaladsl.model.StatusCodes
import akka.http.scaladsl.server.Directives._
import akka.http.scaladsl.server.Route
import akka.http.scaladsl.settings._
import akka.http.scaladsl.unmarshalling._
import akka.stream.ActorMaterializer
import akka.testkit._
import org.scalatest.concurrent.{ IntegrationPatience, ScalaFutures }
import org.scalatest.{ BeforeAndAfterAll, Matchers, WordSpecLike }
import com.typesafe.config.ConfigFactory
import com.typesafe.config.ConfigValueFactory

class TestSetup(serverSettings: ServerSettings) {
  val route =
    path("test") {
      entity(as[Multipart.FormData]) { e ⇒ ctx ⇒
        ctx.complete(StatusCodes.OK)
      }
    }

  def run(request: HttpRequest) = {
    val config = ConfigFactory.load()
      .withValue("akka.http.parsing.modeled-header-parsing", ConfigValueFactory.fromAnyRef("false"))

    implicit val system = ActorSystem("TestSetup", config)
    implicit val materializer = ActorMaterializer()
    import system.dispatcher

    val (host, port) = SocketUtil.temporaryServerHostnameAndPort()
    val binding = Http().bindAndHandle(Route.handlerFlow(route), host, port)

    val f = Http().singleRequest(request.withUri(request.uri.withHost(host).withPort(port).withScheme("http")))
    f.onComplete(_ ⇒ binding.flatMap(_.unbind()))
    f.onComplete(_ ⇒ system.terminate())
    f
  }
}

class Test1 extends TestKit(ActorSystem("Test1")) with ScalaFutures with IntegrationPatience with WordSpecLike with RequestBuilding {
  import system.dispatcher

  "A Multipart request" should {
    "be parsed with media parsing modeled parsing header off" in {
      val form = Multipart.FormData(Multipart.FormData.BodyPart.Strict("key1", "value1"), Multipart.FormData.BodyPart.Strict("key2", "value2"))
      val resp = new TestSetup(ServerSettings(system)).run(Post("/test", form)).futureValue
      assert(resp.status == StatusCodes.OK)
    }
  }
}

The above test with akka.http.parsing.modeled-header-parsing disabled will fail with the following:

The request content was malformed: multipart/form-data part must contain `Content-Disposition` header with `name` parameter

Note: Just disabling akka.http.server.parsing.modeled-header-parsing (i.e. disabling only for the server) will result in a success.

It looks like there is some parser engine that MultiPart marshaller uses (i.e. not the server parser engine) that initializes the HttpHeaderParser with header-parsing off https://github.com/akka/akka-http/blob/25f5eb128e62f303f84f9bcdafd2a7861d604458/akka-http-core/src/main/scala/akka/http/impl/engine/parsing/HttpHeaderParser.scala#L472-L475 (i.e. respecting the akka.http.parsing.modeled-header-parsing flag) causing this to fail.

Adding Content-Disposition to

https://github.com/akka/akka-http/blob/25f5eb128e62f303f84f9bcdafd2a7861d604458/akka-http-core/src/main/scala/akka/http/impl/engine/parsing/HttpHeaderParser.scala#L446-L458

fixes this, but I don't have the necessary context to know why it was left off alwaysParsedHeaders list in the first place.

jrudolph commented 5 years ago

Thanks, @prateekr (and sorry for not answering sooner). I agree, adding content-disposition to the alwaysParsedHeaders seems like an easy way to get this fixed. Can you open a PR?

prateekr commented 5 years ago

Sure, I'll take a look.

nooop3 commented 1 year ago

Please note, we need to disable config modeled-header-parsing to let the AKKA use the correct format of the Content-Disposition header. Otherwise, the AKKA will fail to pass examples below:

# All ascii
Content-Disposition: attachment; filename="download.csv"; filename*=UTF-8''another-ascii-name.csv
# UTF-8 name
Content-Disposition: attachment; filename="download.csv"; filename*=UTF-8''%E4%B8%8B%E8%BD%BD.csv