Closed Nakort closed 10 years ago
Nice find, I agree: Invalid dates shouldn't raise an error.
Right now this pull is failing in CI. You can make ruby 1.9 tests pass by adding # encoding: UTF-8
to the top of api_auth_spec.rb
The ruby 1.8 tests are failing because apparently Time.parse
was way less strict in ruby 1.8. I think if we change Time.parse
to Time.httpdate
this should make everything pass, and it will be even more correct than before since that header should always be in RFC 2616 format. What do you think?
:+1:
What version do we release this change in. If we follow SemVer this is technically a backwards incompatible API change, but yet this doesn't feel like a 2.0.0. I don't know.
I think it's ok to do a minor release for this.
It doesn't really change the functionality because it never allowed a request with an invalid date to pass authentication. It just responded differently.
Only apps that are specifically testing for an exception to be raised would be affected. In those cases, if they are using a gem version like '~> 1.2.4' they wouldn't even pull down the new version anyway.
Yeah, the odds must be very, very slim of someone specifically testing for the ArgumentError to be raised, and then considering the request to be valid.
The more I think about this, the more I'm okay with calling this a bugfix and releasing as 1.2.5. Any objections to that @mgomes?
Nope. Fire away.
Released in v1.2.5
Ran into this issue on our API today, an invalid Date was being to sent to our servers, think it is better to return a fail authentication than raise an ArgumentError exception.