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

Bad date calculations #53

Closed
grosch opened this issue Apr 25, 2018 · 5 comments
Closed

Bad date calculations #53

grosch opened this issue Apr 25, 2018 · 5 comments
Labels
bug Something isn't working
Milestone

Comments

@grosch
Copy link
Contributor

grosch commented Apr 25, 2018

https://github.com/vapor/postgresql/blob/388346471c6a89c2695235d8ffff43c5941283af/Sources/PostgreSQL/Data/PostgreSQLData%2BDate.swift#L32

You're doing date calculations by using seconds, which is always a bad thing to begin with.

However, I'm hitting a case where the app crashes with

NIO-ELT-#1 (4): EXC_BAD_INSTRUCTION (code=EXC_I386_INVOP, subcode=0x0)

I believe it's because the days is coming through as a negative number.

@grosch
Copy link
Contributor Author

grosch commented Apr 25, 2018

I just found this date value in my database

0214-02-05

I'm sure that's what's making it die. Obviously that's corrupt data in my database that I need to fix.

@MrMage
Copy link
Contributor

MrMage commented Apr 25, 2018

If it crashes Vapor, we should probably look into fixing it anyway. Do you have a reproducible case?

Also, why is using seconds for date calculations bad?

@grosch
Copy link
Contributor Author

grosch commented Apr 25, 2018

https://developer.apple.com/videos/play/wwdc2011/117/ talks all about why it's bad. But at the end of the day, no pun intended, there aren't always 24 * 60 * 60 seconds in a day.

I think it's reproducible by just setting the date to what I showed above.

@MrMage
Copy link
Contributor

MrMage commented Apr 25, 2018

That's a different use case. For timestamps, "number of seconds since some reference date" is the standard way of doing things as far as I know. IIRC this is actually how Postgres stores timestamps internally.

@tanner0101 tanner0101 added the bug Something isn't working label Jun 15, 2018
@tanner0101 tanner0101 added this to the 1.0.0 milestone Jun 15, 2018
@tanner0101
Copy link
Member

The issue was integer overflow. Fixed by using Int64.

tanner0101 added a commit that referenced this issue Jun 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants