http client: Fallback to default mime type 'application/octet-stream'

[bew]

Fixes #7370

Uses a default mime type as a fallback when the Content-Type header cannot be parsed.

I'm not sure about the DEFAULT_MIME_TYPE as a constant, should the user be able to customize the default mime type?

[WJWH]

Whoa, I had just cloned the repo and opened up my editor but you already made a fix! That is less than 45 minutes from issue creation to pr submission ❤️

[straight-shoota]

mime_type is not used anywhere except for querying the charset parameter, which isn't set in application/octet-stream anyway. So the following condition should simply be skipped when parse? returns nil.

[bew]

ok, so something like that?

    if (content_type = headers["Content-Type"]?) &&
       (mime_type = MIME::MediaType.parse?(content_type)) &&
       (charset = mime_type["charset"]?)
      body.set_encoding(charset, invalid: :skip)
    end

or

    if content_type = headers["Content-Type"]?
      if MIME::MediaType.parse?(content_type).try &.["charset"]
        body.set_encoding(charset, invalid: :skip)
      end
    end

[Sija]

@bew why not just return unless mime_type?

[asterite]

Yes. In fact we should return unless a lot more here, to avoid nested code.

[bew]

Squashed!

[bcardiff]

Thanks @bew

[straight-shoota]

Honestly, I really don't like the change to unnest all conditions from if to return unless. IMHO that makes it harder to follow control flow. I'm definitely in favour of using return unless more often, but having it in every second line is simply confusing.

Obviously, this is a matter of stylistic preference. But the agreement is not to change from one style to another for no semantic reason.