[I] Multipart parser leaks a part's headers into the following part [pekko-http]

["pjfanning (via GitHub)" <gi...@apache.org>]

pjfanning opened a new issue, #1278:
URL: https://github.com/apache/pekko-http/issues/1278

   ### 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
   
   ```scala
   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`:
   
   ```scala
   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.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org

Re: [I] Multipart parser leaks a part's headers into the following part [pekko-http]

["pjfanning (via GitHub)" <gi...@apache.org>]

pjfanning closed issue #1278: Multipart parser leaks a part's headers into the following part
URL: https://github.com/apache/pekko-http/issues/1278


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org