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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to