Skip to content

Commit

Permalink
fix: Ignore everything after closing boundary when parsing multi form…
Browse files Browse the repository at this point in the history
… data (#2862)

* add working test case

* break test

* wip

* fix bug

* make boundaryMatches val

* revert buffer to ChunkBuilder

* buffer is val

* fmt

* fmt

* Update zio-http/shared/src/main/scala/zio/http/internal/FormState.scala

Co-authored-by: kyri-petrou <[email protected]>

* Extract closingBoundaryBytesSize and reset boundaryMatches with a new Array[Boolean](closingBoundaryBytesSize)

* Update zio-http/shared/src/main/scala/zio/http/internal/FormState.scala

Co-authored-by: kyri-petrou <[email protected]>

* Replace Array[Boolean] with single boolean

---------

Co-authored-by: kyri-petrou <[email protected]>
  • Loading branch information
seakayone and kyri-petrou authored Jun 12, 2024
1 parent abdaf0b commit 68a7abe
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 3 deletions.
26 changes: 25 additions & 1 deletion zio-http/jvm/src/test/scala/zio/http/FormSpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ object FormSpec extends ZIOHttpSpec {
form2 == form,
)
},
test("encoding with custom paramaters [charset]") {
test("encoding with custom parameters [charset]") {

val form = Form(
FormField.textField(
Expand Down Expand Up @@ -144,6 +144,30 @@ object FormSpec extends ZIOHttpSpec {
}

},
test("decoding no lf at the end") {
val body = Chunk.fromArray(
s"""|--(((AaB03x)))${CR}
|Content-Disposition: form-data; name="hocon-data"${CR}
|Content-Type: text/plain${CR}
|${CR}
|foos: []${CR}
|--(((AaB03x)))${CR}
|Content-Disposition: form-data; name="json-data"${CR}
|Content-Type: text/plain${CR}
|${CR}
|{ "bars": [] }${CR}
|--(((AaB03x)))--""".stripMargin.getBytes(),
)

val form = Form(
FormField.textField("hocon-data", "foos: []", MediaType.text.`plain`),
FormField.textField("json-data", """{ "bars": [] }""", MediaType.text.`plain`),
)

Form
.fromMultipartBytes(body)
.map(form2 => assertTrue(form2 == form))
},
)

val multiFormStreamingSuite: Spec[Any, Throwable] =
Expand Down
20 changes: 18 additions & 2 deletions zio-http/shared/src/main/scala/zio/http/internal/FormState.scala
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,10 @@ private[http] object FormState {
final class FormStateBuffer(boundary: Boundary) extends FormState { self =>

private val tree0: ChunkBuilder[FormAST] = ChunkBuilder.make[FormAST]()
private val buffer: ChunkBuilder.Byte = new ChunkBuilder.Byte
private val buffer: ChunkBuilder[Byte] = new ChunkBuilder.Byte
private var bufferSize: Int = 0
private val closingBoundaryBytesSize = boundary.closingBoundaryBytes.size
private var boundaryMatches: Boolean = true

private var lastByte: OptionalByte = OptionalByte.None
private var isBufferEmpty = true
Expand All @@ -54,8 +57,14 @@ private[http] object FormState {

val crlf = byte == '\n' && !lastByte.isEmpty && lastByte.get == '\r'

val posInLine = bufferSize + (if (this.lastByte.isEmpty) 0 else 1)
boundaryMatches &&= posInLine < closingBoundaryBytesSize && boundary.closingBoundaryBytes(posInLine) == byte
val boundaryDetected = boundaryMatches && posInLine == closingBoundaryBytesSize - 1

def flush(ast: FormAST): Unit = {
buffer.clear()
bufferSize = 0
boundaryMatches = true
lastByte = OptionalByte.None
if (crlf && isBufferEmpty && (phase eq Phase.Part1)) phase0 = Phase.Part2
if (ast.isContent && dropContents) () else addToTree(ast)
Expand All @@ -72,7 +81,11 @@ private[http] object FormState {
case ClosingBoundary(_) => BoundaryClosed(tree)
}

} else if (crlf && (phase eq Phase.Part2)) {
} else if ((crlf || boundaryDetected) && (phase eq Phase.Part2)) {
if (boundaryDetected) {
buffer += '-'
buffer += '-'
}
val ast = FormAST.makePart2(buffer.result(), boundary)

ast match {
Expand All @@ -87,6 +100,7 @@ private[http] object FormState {
if (!lastByte.isEmpty) {
if (isBufferEmpty) isBufferEmpty = false
buffer += lastByte.get
bufferSize += 1
}
lastByte = OptionalByte.Some(byte)
self
Expand All @@ -102,6 +116,8 @@ private[http] object FormState {
def reset(): Unit = {
tree0.clear()
buffer.clear()
bufferSize = 0
boundaryMatches = true
isBufferEmpty = true
dropContents = false
phase0 = Phase.Part1
Expand Down

0 comments on commit 68a7abe

Please sign in to comment.