Fix error with body.rewind with rack 3.0

[aglushkov]

In rack 3.0 there are no method #rewind in Rack::Lint::InputWrapper that existed in rack 2.2

Type of change

• Bug fix (non-breaking change that fixes an issue)

Related issues

• Fix #1106

[brianr]

Thanks @aglushkov , we're taking a look at this.

[waltjones]

@aglushkov Thanks for bringing up this issue, and thank you for the PR.

I looked up the rack issue (rack/rack#1148) and PR (rack/rack#1804) for more insight.

My concern about this PR is that the input is still IO-like, and reading from it will still update the IO cursor. The difference is that since it is non-rewindable, it can only be read once, so rollbar-gem probably shouldn't be reading it.

Am I missing something that would make this safe as is?

[aglushkov]

@waltjones Your concerns are correct.

• If input was already read, you can not read it again (so you can receive issues, that input data was not tracked by Rollbar)
• If input was not read, you can read it, but no one can do it after.

There are Rack::RewindableInput class in Rack-2
and additional Rewindable::Middleware in Rack-3 that can be used to get around this issues. But using them can be also an issue, as they will copy input to Tempfile, that can be no desirable behavior for application.

I would suggest using not-public body.instance_variable_get(:@input) interface for Rack 3.0 to achieve previous behavior for most applications without Rack::RewindableInput.

def rewind_body(rack_req)
     body = rack_req.body
     if body.respond_to?(:rewind)
       body.rewind
     else
       # workaround for Rack 3.0
       # for my application (Rack, Roda, Puma) `body_input` is StringIO, Puma::NullIO, Tempfile, everything is rewindable
       body_input = body.instance_variable_get(:@input) 
       body_input.rewind if body_input.respond_to?(:rewind) # additional check if its not rewindable
   end
end

Such should be used before and after reading input.

[waltjones]

@aglushkov Nice workaround!

I have two suggestions.

• We need to check for nil body_input, since @input could be removed/renamed in the future.
• We need to skip rack_req.body.read if the input is not rewindable. For example, something like:

return {} unless rewindable?(rack_req)

[aglushkov]

It turns out this @input is only present when Rack::Lint middleware is used. It is enabled by default by Rackup, but can be disabled with --env none option. In most production apps, where Rollbar is used it should be disabled, so having
req.body.rewind if req.body.respond_to?(:rewind) should be enough to satisfy new Rack3 specification

[waltjones]

@aglushkov That sounds fine. We still need to skip rack_req.body.read if the input is not rewindable.

[aglushkov]

@waltjones Sorry for delay, I've update PR, add test

[waltjones]

@aglushkov Nice work. Thank you for the test coverage, and thank you again for the PR.