Code

Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#15378 closed (fixed)

geodjango layermapping save method missed an except

Reported by: kunitoki Owned by: jbronn
Component: GIS Version: 1.2
Severity: Keywords: layermapping OGR_F_GetGeometryRef
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: UI/UX:

Description

If you use layermapping and you get an OGR error in _save method, then it is impossible to continue due to a missed except (thus the strict parameter is useless then):

django.contrib.gis.gdal.error.OGRException: Invalid geometry pointer returned from "OGR_F_GetGeometryRef".

Attachments (3)

layermapping.patch (736 bytes) - added by kunitoki 3 years ago.
add one more except to catch OGR exceptions in layermapping
15378.1.diff (3.2 KB) - added by jbronn 3 years ago.
Test case and data that reproduces the problem.
15378.2.diff (4.0 KB) - added by jbronn 3 years ago.
Now catch OGRException and raise LayerMapError in right place.

Download all attachments as: .zip

Change History (11)

Changed 3 years ago by kunitoki

add one more except to catch OGR exceptions in layermapping

comment:1 Changed 3 years ago by kunitoki

  • milestone set to 1.3
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

comment:2 Changed 3 years ago by russellm

  • Triage Stage changed from Unreviewed to Accepted

comment:3 Changed 3 years ago by jbronn

  • Owner changed from nobody to jbronn
  • Status changed from new to assigned

I've confirmed this exists with an invalid shapefile.

Changed 3 years ago by jbronn

Test case and data that reproduces the problem.

comment:4 Changed 3 years ago by jbronn

  • Patch needs improvement set

I'm -1 on the implementation layermapping.patch, as I think the exception should be caught in the methods where it is raised (e.g., in methods that LayerMapping.feature_kwargs() calls), and then caught as a LayerMapError in LayerMapping.save().

However, my patch has implemented tests and includes a shapefile which recreates the issue.

Changed 3 years ago by jbronn

Now catch OGRException and raise LayerMapError in right place.

comment:5 Changed 3 years ago by jbronn

  • Resolution set to fixed
  • Status changed from assigned to closed

In [15813]:

Fixed #15378 -- Now properly handle OGR layers that have features with invalid geometries. Thanks, kunitoki for bug report and initial patch.

comment:6 Changed 3 years ago by jbronn

In [15814]:

[1.2.X] Fixed #15378 -- Now properly handle OGR layers that have features with invalid geometries. Thanks, kunitoki for bug report and initial patch.

Backport of r15813 from trunk.

comment:7 Changed 3 years ago by jbronn

In [15815]:

[1.2.X] Previous changeset did not include new test data, stupid hgsubversion. Refs #15378.

comment:8 Changed 3 years ago by jacob

  • milestone 1.3 deleted

Milestone 1.3 deleted

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.