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

GpsPoint validator needs a rewrite because it returns true for the input "foo, bar" #135

Closed
froschdesign opened this issue Jun 23, 2022 · 5 comments · Fixed by #169
Closed
Labels
Bug Something isn't working
Milestone

Comments

@froschdesign
Copy link
Member

froschdesign commented Jun 23, 2022

I believe this validator should either be rewritten or removed entirely as it will return true for the input "foo, bar"

It expects a string as input in the form "latitude, longitude" :

    public function isValid($value)
    {
        if (strpos($value, ',') === false) {
            $this->error(self::INCOMPLETE_COORDINATE, $value);
            return false;
        }

        [$lat, $long] = explode(',', $value);

but the isValidCoordinate() method blindly casts the supposed lat or long to a double:

        $doubleLatitude = (double) $value;

        if ($doubleLatitude <= $maxBoundary && $doubleLatitude >= $maxBoundary * -1) {
            return true;
        }

which results in a 0 value in the case of non-numerical strings, which then passes validation as a valid lat/long.

At the very least it should have explicit comments stating that the expected input is in a very particular format already.

Originally posted by @ajgale in #69 (comment)

@froschdesign froschdesign added the Bug Something isn't working label Jun 23, 2022
@Ocramius
Copy link
Member

Deprecation would be best then.

@froschdesign
Copy link
Member Author

I have checked it in the unit tests and confirm the problem.

@froschdesign
Copy link
Member Author

Another option would be to check if a number is included before conversion to double is done:

$doubleLatitude = (double) $value;
if ($doubleLatitude <= $maxBoundary && $doubleLatitude >= $maxBoundary * -1) {
return true;
}
$this->error(self::OUT_OF_BOUNDS);
return false;

@boesing
Copy link
Member

boesing commented Jun 23, 2022

Yah, using is_numeric should be suitable here

@froschdesign
Copy link
Member Author

@boesing

Yah, using is_numeric should be suitable here

This will fail with the current test cases because coordinates like 63 47 24.691 N are converted to 634724.691N. The error for "out of bounds" is not set.

public function errorMessageTestValues(): array
{
return [
['63 47 24.691 N, 18 2 54.363 W', GpsPoint::OUT_OF_BOUNDS, '63 47 24.691 N'],
['° \' " N,° \' " E', GpsPoint::CONVERT_ERROR, '° \' " N'],
['° \' " N', GpsPoint::INCOMPLETE_COORDINATE, '° \' " N'],
];
}

codisart added a commit to codisart/laminas-validator that referenced this issue Jan 12, 2023
codisart added a commit to codisart/laminas-validator that referenced this issue Jan 12, 2023
codisart added a commit to codisart/laminas-validator that referenced this issue Jan 12, 2023
codisart added a commit to codisart/laminas-validator that referenced this issue Jan 12, 2023
codisart added a commit to codisart/laminas-validator that referenced this issue Jan 12, 2023
codisart added a commit to codisart/laminas-validator that referenced this issue Jan 12, 2023
codisart added a commit to codisart/laminas-validator that referenced this issue Jan 12, 2023
codisart added a commit to codisart/laminas-validator that referenced this issue Jan 12, 2023
codisart added a commit to codisart/laminas-validator that referenced this issue Jan 12, 2023
codisart added a commit to codisart/laminas-validator that referenced this issue Jan 12, 2023
codisart added a commit to codisart/laminas-validator that referenced this issue Jan 12, 2023
codisart added a commit to codisart/laminas-validator that referenced this issue Jan 12, 2023
codisart added a commit to codisart/laminas-validator that referenced this issue Jan 12, 2023
codisart added a commit to codisart/laminas-validator that referenced this issue Jan 12, 2023
codisart added a commit to codisart/laminas-validator that referenced this issue Jan 12, 2023
codisart added a commit to codisart/laminas-validator that referenced this issue Jan 12, 2023
codisart added a commit to codisart/laminas-validator that referenced this issue Jan 31, 2023
codisart added a commit to codisart/laminas-validator that referenced this issue Jan 31, 2023
codisart added a commit to codisart/laminas-validator that referenced this issue Jan 31, 2023
codisart added a commit to codisart/laminas-validator that referenced this issue Jan 31, 2023
codisart added a commit to codisart/laminas-validator that referenced this issue Feb 21, 2024
codisart added a commit to codisart/laminas-validator that referenced this issue Feb 21, 2024
codisart added a commit to codisart/laminas-validator that referenced this issue Feb 21, 2024
codisart added a commit to codisart/laminas-validator that referenced this issue Feb 21, 2024
Ocramius added a commit that referenced this issue Feb 22, 2024
Fix #135: better GPS point validator detection of invalid coordinate numeric conversions
@Ocramius Ocramius added this to the 2.48.0 milestone Feb 22, 2024
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
3 participants