Fix `request.body.data` becomes nil instead of empty when the request body is empty

[sidepelican]

When parsing a request, if it is determined that the body is empty, the request.bodyStorage is set to .collected(ByteBuffer()), resulting in request.body.data being empty data. This ensures that even with an empty body, such as with x-www-form-urlencoded, decoding can proceed correctly.

• Fixes: https://github.com/vapor/vapor/issues/3288

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 77.70%. Comparing base (1fdd1c2) to head (52489b2).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3289      +/-   ##
==========================================
- Coverage   77.76%   77.70%   -0.06%     
==========================================
  Files         226      226              
  Lines        9106     9107       +1     
==========================================
- Hits         7081     7077       -4     
- Misses       2025     2030       +5     

Files with missing lines	Coverage Δ
...s/Vapor/HTTP/Server/HTTPServerRequestDecoder.swift	96.59% <100.00%> (+0.01%)	:arrow_up:

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[0xTim]

I don't actually think this is the right approach as it could have unintended consequences for other decoders. Especially JSON - that should fail without a body. I think this should be fixed inside the URL form decoder

[sidepelican]

I think it would be the same if the JSON decoder handled it internally. The crux of the problem is that it's impossible to distinguish whether the request body was empty or cannot read.

[sidepelican]

This issue is not limited to the URL form decoder; it is a broader problem that affects all decoders that can accept an empty body. I think many people are not aware of the problem because they only use JSON.

[gwynne]

I have to agree - JSON failing on an empty body is a behavior specific to JSON; an empty body is a valid input for x-www-form-urlencoded; therefore, this needs to be up to the individual decoders.

[sidepelican]

@0xTim Could you review this?

[0xTim]

@sidepelican I did and my point still stands. This needs to be fixed at the FormDataDeocder level, not in the request body

[sidepelican]

@0xTim Sorry, I might have caused some confusion with my explanation.

No matter how much we tweak FormDataDecoder, it won’t resolve this issue. That’s because if body.data becomes nil, the process doesn’t even reach FormDataDecoder.

The error occurs here:

https://github.com/vapor/vapor/blob/1fdd1c2ff090427476f534fcc9b9eb45a195c390/Sources/Vapor/Request/Request.swift#L141-L144

This handling works fine for JSONDecoder, but it may not be correct for other decoders. That said, I don’t think this handling itself is the problem. The real issue, in my view, is that there are cases where the body is being read correctly, yet request.body.data still ends up as nil. That’s why I went with the approach in this PR.

[gwynne]

This makes sense to me; if the lack of a body never reaches the decoders, the decoders can't make their respective decisions about how to handle it. I'm not sure what else a change at this level might affect, though.

[0xTim]

This would be a major break, removing automatic error handling and unprocessible responses in existing code. What we probably need to do is have a look up one decoders to see what can have an empty body and use the content type header to determine which to use. I'm pretty sure this is only applicable to url encoded forms

[0xTim]

@sidepelican Where did you hit this bug? Because I'm trying to dig through the URI/HTTP spec to ensure that no body is actually valid (as opposed to an empty body)

[sidepelican]

@0xTim
The server-side source code is described in: https://github.com/vapor/vapor/issues/3288

The browser-side HTML is in the following format:

<form action="/list" method="POST">
<input type="hidden" name="filter" value="some filtering text">
</form>

The following line is added only when performing an advanced search: <input type="hidden" name="filter" value="some filtering text">

On a certain list page, the form is used without any parameters under normal circumstances, but when the user performs an advanced search, a form with filtering parameters is used instead. I’d like to handle this on the server side with a single endpoint.

struct PostBody: Decodable {
    var filter: String?
}

I want to receive the data in this format, but currently, it cannot handle in vapor.

[0xTim]

Ok I see what you mean. I think right now, especially Vapor 4, this probably just falls underneath unsupported behaviour. The spec isn't very clear about whether no body is the same as an empty body and it's probably up to implementors to decide.

For your use case you have a couple of options - check the length of the body before decoding, check headers or just wrap the decode in a do/catch