Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Invalid behaviour according to RFC. #1844

Closed
ioquatix opened this issue Dec 11, 2022 · 8 comments
Closed

Invalid behaviour according to RFC. #1844

ioquatix opened this issue Dec 11, 2022 · 8 comments

Comments

@ioquatix
Copy link
Contributor

it 'raises Sinatra::BadRequest when multipart/form-data request has no content' do
request = Sinatra::Request.new(
'REQUEST_METHOD' => 'POST',
'CONTENT_TYPE' => 'multipart/form-data; boundary=dummy',
'rack.input' => StringIO.new('')
)
assert_raises(Sinatra::BadRequest) { request.params }
end

This test is invalid according to https://www.rfc-editor.org/rfc/rfc2046#section-5.1.

I'm assuming this is because of golang/go#52519?

@dentarg
Copy link
Member

dentarg commented Dec 14, 2022

It was added in #1743 to address #1739

As stated in that issue it was really easy to make any rack app blow up

I'm running my apps with the middleware at #1739 (comment) to avoid that happening

@dentarg
Copy link
Member

dentarg commented Dec 14, 2022

@ioquatix
Copy link
Contributor Author

@dentarg maybe you can give your opinion on rack/rack#1994 (comment) and the related issues.

@dentarg
Copy link
Member

dentarg commented Dec 14, 2022

Sure

@dentarg
Copy link
Member

dentarg commented Dec 14, 2022

Or well, my opinion is that we can not expect everyone on the internet to be following RFCs :-) We need to guard our apps against non-standard behaviour. I think that's what Sinatra is doing here? Sure, it would be better to address it directly in Rack.

@dentarg
Copy link
Member

dentarg commented Dec 28, 2022

I think we have an understanding why this test case exist in Sinatra, can we close this issue @ioquatix?

It was needed after rack/rack#1572 that closed rack/rack#761.

@ioquatix
Copy link
Contributor Author

You can close it, but if we adopt some mechanism to indicate "bad request" you may want to change the behaviour.

@dentarg dentarg closed this as completed Feb 9, 2023
@ioquatix
Copy link
Contributor Author

In Rack 3.1, this will exist: rack/rack#2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants