Skip to content

Multipart parser leaks a part's headers into the following part #1278

Description

@pjfanning

Summary

BodyPartParser carries the accumulated headers and headerCount of one body part into the next one when a part ends without a header-separating empty line. The following part is reported with headers it never declared.

This is long-standing and inherited from akka-http; it is not a regression. I noticed it while reviewing #1266, whose fix touches the same branch but does not change this behaviour.

Detail

In parseHeaderLines, the BoundaryHeader branch emits the part it has just finished and then continues into the next part, passing its own headers and headerCount along:

https://github.com/apache/pekko-http/blob/main/http-core/src/main/scala/org/apache/pekko/http/impl/engine/parsing/BodyPartParser.scala#L177-L183

case BoundaryHeader =>
  emit(BodyPartStart(headers.toList, _ => HttpEntity.empty(contentType)))
  val ix = lineStart + eolConfiguration.boundaryLength
  if (eolConfiguration.isEndOfLine(input, ix))
    parseHeaderLines(input, ix + eolConfiguration.eolLength, headers, headerCount, None)
  ...

The boundary starts a new part, so that call should begin with a fresh ListBuffer and a headerCount of 0. cth is already correctly reset to None, which suggests the intent was to reset the header state here and that headers/headerCount were simply missed.

The branch is reached whenever a part has headers but no empty line before the next boundary — the shape the existing test "a part without entity and missing header separation CRLF" covers for a single part.

Two consequences:

  • The next part is reported carrying headers belonging to the previous one.
  • headerCount is never reset either, so a run of such parts accumulates towards max-header-count across parts rather than per part, and can trip "multipart part contains more than the configured limit of N headers" on parts that are individually well under the limit.

Reproducer

Added to MultipartUnmarshallersSpec:

Unmarshal(HttpEntity(
  `multipart/mixed`.withBoundary("XYZABC"),
  ByteString("""--XYZABC
               |Age: 12
               |--XYZABC
               |--XYZABC--""".stripMarginWithNewline(lineFeed)))).to[Multipart.General] should haveParts(
  Multipart.General.BodyPart.Strict(HttpEntity.empty(ContentTypes.`text/plain(UTF-8)`), List(Age(12))),
  Multipart.General.BodyPart.Strict(HttpEntity.empty(ContentTypes.`text/plain(UTF-8)`)))

Expected: the second part has no headers.

Actual — both parts carry Age: 12:

Vector(
  General.BodyPart.Strict(HttpEntity.Strict(text/plain; charset=UTF-8, ByteString()), List(Age: 12)),
  General.BodyPart.Strict(HttpEntity.Strict(text/plain; charset=UTF-8, ByteString()), List(Age: 12)))
did not equal
ArraySeq(
  General.BodyPart.Strict(HttpEntity.Strict(text/plain; charset=UTF-8, ByteString()), List(Age: 12)),
  General.BodyPart.Strict(HttpEntity.Strict(text/plain; charset=UTF-8, ByteString()), List()))

Reproduced on main in both the CRLF and LF variants of the spec.

Notes on fixing

The fix itself is to start the next part with fresh header state. Worth being careful about how, though: the call above is the self-recursive one that makes parseHeaderLines @tailrec, so the reset wants to stay a direct self-call rather than being routed through a helper — a mutual recursion here would be unoptimised and is what the trampoline in parseEntity exists to avoid.

Since the affected shape is a part with headers and no separating empty line, a fix will want a test for the Content-Type interaction too: cth is already reset, so a Content-Type on the first part should not set the content type of the second either.

Happy to put up a PR if this is agreed as a bug.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions