yocontra / node-gdal-next

Node.js bindings for GDAL (Geospatial Data Abstraction Library) [Fork]
https://contra.io/node-gdal-next/
Apache License 2.0
75 stars 35 forks source link

crash when accessing dataset opened via the driver object #18

Closed mmomtchev closed 3 years ago

mmomtchev commented 3 years ago
console.log(dataset.driver.open('file.tiff', undefined));

will produce a segmentation fault which I traced to gdal_dataset.cpp:722

  if (raw->GetDriver() == nullptr  || !raw->GetDriver()->GetMetadataItem(GDAL_DCAP_RASTER)) {
    info.GetReturnValue().Set(Nan::Null());
    return;
  }

I "fixed" it by checking for nullptr, but it shouldn't happen, there must be an initialization missing somewhere

mmomtchev commented 3 years ago

According to the discussion on gdal-dev list (@rouault), this stems from us not being supposed to call GDALDriver->pfnOpen directly The problem is that there is no official GDAL entry point for constructing a GDALDataset at the GDALDriver level In fact, that open does not belong there at all We are supposed to construct GDALDataset by GDALOpen(ex) only which is at the main level And if we want to pass a list of drivers (or one driver as in this case), we must know its name In node-gdal a driver doesn't know his own name

There are only bad solutions to this problem

rouault commented 3 years ago

In node-gdal a driver doesn't know his own name

if you're able to do poDriver->pfnOpen, you can know the name of the driver with poDriver->GetDescription(), and that's the one to pass to the list of allowed drivers of GDALOpenEx()

mmomtchev commented 3 years ago

@rouault no solution for GDAL 1.x?

rouault commented 3 years ago

GDAL 1.x?

GDAL 1.x !!!!! Are you joking :-) ? Degraded mode: just use GDALOpen() then and ignores that the user has specified a driver.

mmomtchev commented 3 years ago

Ok, I swept the GDAL 1.x **** under the carpet, the nullptr is still there, but Node won't crash anymore, its just that certain fields of the dataset will be undefined when read from JS

yocontra commented 3 years ago

PR merged

rouault commented 3 years ago

actually while thinking about that later, for GDAL 1, you could manually set the poDriver member to the driver... That's just what GDALOpen() does.