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

Return nil (and don't raise) when "Content-Type" header is an empty string. #8464

Merged
merged 2 commits into from
Nov 12, 2019

Conversation

Sija
Copy link
Contributor

@Sija Sija commented Nov 12, 2019

This PR changes the behavior of HTTP::Client::Response#mime_type to return nil (and not raise) when Content-Type header is an empty string.

As per #8398 (comment)
Fixes #8398

@bcardiff bcardiff added kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:stdlib:networking labels Nov 12, 2019
@bcardiff
Copy link
Member

Can we get a spec (and a PR description that describes what and not how?)

@Sija
Copy link
Contributor Author

Sija commented Nov 12, 2019

@bcardiff done!

Copy link
Member

@bcardiff bcardiff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Please rephrase the PR title :-)

@Sija Sija changed the title Use String#presence in HTTP::Client::Response#mime_type Return nil (and don't raise) when "Content-Type" header is an empty string. Nov 12, 2019
@bararchy
Copy link
Contributor

@bcardiff did he get the bonus points for using string presence? ;)

@bcardiff
Copy link
Member

Of course!

image

@bcardiff bcardiff added this to the 0.32.0 milestone Nov 12, 2019
@bcardiff bcardiff merged commit 7334bec into crystal-lang:master Nov 12, 2019
@jkthorne
Copy link
Contributor

should the be the default behavior or Headers#[]?. I am not aware of a header where an empty string is a valid value.

@straight-shoota
Copy link
Member

@wontruefree According to RFC 7230 empty header values are legal, so we should honor that and don't confuse it with an absent header.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:stdlib:networking
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unhandled exception on HTTP::Handler
5 participants