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

Fix empty point behaviour in wkb, ewkb, and hexewkb formats #187

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

tw99
Copy link

@tw99 tw99 commented Dec 3, 2021

Issue

When loading an empty point geometry encoded in one of the above formats, the coordinate doubles unpacked in WKB.class#getPoint are both NAN

  function getPoint(&$mem) {
    $point_coords = unpack("d*", fread($mem,$this->dimension*8));
    if (!empty($point_coords)) {
      return new Point($point_coords[1],$point_coords[2]);
    }
    else {
      return new Point(); // EMPTY point
    }
  }

Because of the NAN values, the empty point is able to bypass the safeguards in the Point.class constructor.

    // Basic validation on x and y
    if (!is_numeric($x) || !is_numeric($y)) {
      throw new Exception("Cannot construct Point. x and y should be numeric");
    }

This results in an invalid Point object, where $point->isEmpty() is false.

Observations

Postgres seems to implicitly encode POINT EMPTY as POINT(NAN, NAN), and by default returns the hexewkb representation.

Postgres:

Screen Shot 2021-12-02 at 11 08 25 PM

>>> select st_ashexewkb(st_geomfromtext('POINT EMPTY'));
=> "0101000000000000000000F87F000000000000F87F"

Verifying in PHP:

>>> bin2hex(pack('cLdd', 1, 1, NAN, NAN))
=> "0101000000000000000000f87f000000000000f87f"

Problems

1. Reading the above empty point binary with GeoPhp:

>>> geoPHP::load('0101000000000000000000F87F000000000000F87F')->asArray()
=> [
     NAN,
     NAN,
   ]

>>> geoPHP::load('0101000000000000000000F87F000000000000F87F')->isEmpty()
=> false

2. Empty Point binary output representation is invalid.

>>> bin2hex(geoPHP::load('0101000000000000000000F87F000000000000F87F')->out('ewkb'))
=> "0101000000"
>>> select st_astext('0101000000'::geometry)

=> [XX000] ERROR: WKB structure does not match expected size! Position: 18

Solutions

  • Check for is_nan in the Point.class constructor and create an empty point in that case.
  • Properly encode NAN values for an empty point binary representation.
  • Add test fixture for an empty point in ewkb format

*Note: this solution works for empty point geometries with different srid's as well.

@tw99 tw99 force-pushed the fix-loading-empty-point-wkb branch from e5eebbf to 8dc6bce Compare December 3, 2021 05:01
itamair added a commit to itamair/geoPHP that referenced this pull request Feb 4, 2023
@itamair
Copy link

itamair commented Feb 4, 2023

This repo looks kind of abandoned/un-maintanied
FYI, this has been embedded/fixed into this fork repo:
https://github.com/itamair/geoPHP
throughout this commit: itamair@7de893d

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.

2 participants