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

test -gt, -lt integer overflow #5414

Closed
floam opened this issue Dec 14, 2018 · 5 comments
Closed

test -gt, -lt integer overflow #5414

floam opened this issue Dec 14, 2018 · 5 comments
Labels
bug Something that's not working as intended regression Something that used to work, but was broken, especially between releases
Milestone

Comments

@floam
Copy link
Member

floam commented Dec 14, 2018

In fish 3:

$ test 9223372036854775808 -lt 42 && echo true || echo false
true

Previously:

test returned eval errors:
	invalid integer '9223372036854775808'

This happened when test was extended to take floats. (Technically, long long, double pairs because of precision requirements).

@floam floam added bug Something that's not working as intended regression Something that used to work, but was broken, especially between releases labels Dec 14, 2018
@floam floam added this to the fish-future milestone Dec 14, 2018
@floam floam changed the title test integer overflow test -gt, -lt integer overflow Dec 14, 2018
@floam
Copy link
Member Author

floam commented Dec 14, 2018

We need to explicitly check errno for ERANGE. (And a different topic, but shouldn't this ought to be intmax_t rather than long long, using wcstoimax()?) edit: Well, it looks like whatever was done was pretty deliberate, pretty recently, so I'll just assume it's doing exactly what is intended.

@floam floam closed this as completed in 4aa069a Dec 14, 2018
@floam
Copy link
Member Author

floam commented Dec 14, 2018

We now catch the error, and have different output for different kinds of errors:

$ test 9324234234234234234234234.56 -lt 5
test returned eval errors:
	Result too large: '9324234234234234234234234.56'
$ test 2345a.44 -gt 1
test returned eval errors:
	Integer 2345 in '2345a.44' followed by non-digit
$ test '' -gt 1
test returned eval errors:
	Invalid argument: ''

@zanchey
Copy link
Member

zanchey commented Dec 16, 2018

This seems like something suitable for 3.0?

@floam floam modified the milestones: fish-future, fish-3.0 Dec 16, 2018
floam added a commit that referenced this issue Dec 16, 2018
We were not parsing an in-range number when we claimed we were,
and were thus failing to error with invalid numbers and returned
a wrong test result. Fixed #5414

Also, provide the detail we can for the other error cases.
@ridiculousfish
Copy link
Member

Yeah let's pick it into 3.0

@floam
Copy link
Member Author

floam commented Dec 17, 2018

I've had it in Integration for a couple hours

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something that's not working as intended regression Something that used to work, but was broken, especially between releases
Projects
None yet
Development

No branches or pull requests

3 participants