Skip to content

Conversation

@lame-nickname
Copy link
Contributor

I've started working on Content-Range header and realized that my previous implementation of Range header was wrong. RangeSpecs make sense only for byte ranges and all custom range units can have arbitrary strings as their values.

I've modified the code to reflect that, but there is one thing for which I need your feedback:
I've used Range::Other for custom ranges, as it IMO matches more closely naming in the RFC (other-ranges-specifier). The thing is that Accept-Ranges header uses RangeUnit::Unregistered for the same purpose, so it would make sense to decide on one them and use it consistently.

This is kinda a breaking change. However since the Range header was introduced only a week ago, I'm not sure, if I should mark it as such.

@seanmonstar
Copy link
Member

I haven't published a version with Range yet, as I'm trying to wrap up the current set of breaking changes before publishing 0.6.

@lame-nickname
Copy link
Contributor Author

Cool. Do you have an opinion on Other vs Unregistered? Let's also cc @pyfisch, as he's the one who created Accept-Ranges header.

@seanmonstar
Copy link
Member

Unregistered is used frequently in other headers and for StatusCodes, as many use a "registry", though sometimes that registry is just the original spec and who cares....

@lame-nickname
Copy link
Contributor Author

I've given it some more thought and I'm no longer sure that Range and RangeUnit names have to be consistent. In case of Range, Unregistered makes little sense, as range as a whole isn't registered nor unregistered. So I'd leave it as it is.

I know this isn't a very important problem, but I just like to name stuff the way it makes the most sense ;)

@pyfisch
Copy link
Contributor

pyfisch commented Jun 23, 2015

Looks good. I am in favor of Unregistered since ranges are registered, the name of the range unit is registered in a IANA registry and it must supply a description of the range. Ranges without public description are unregistered from my understanding.

@lame-nickname
Copy link
Contributor Author

I'd argue that RangeUnits have to be registered, not Ranges.
But I'm ok with Unregistered too, so no more bikeshedding ;)

seanmonstar added a commit that referenced this pull request Jun 24, 2015
refactor(headers):  improve `Range` header adherence to HTTP spec
@seanmonstar seanmonstar merged commit 1b6c692 into hyperium:master Jun 24, 2015
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

Successfully merging this pull request may close these issues.

3 participants