Turn restrictions

[stephend]

https://github.com/opennewzealand/linz2osm/issues/115

Current status:

• have made model changes to LINZ2OSM to support layers that are a "relation" containing other layers
• model changes to track versioned data
• have done some uploads of NZOpenGPS data so that I can test API queries needed for export
• in the middle of updating NZOpenGPS parser to include some extra info I'll need (geometries for nodes, as well as road segments).
• updated workslice map to get geometries from join table for "relation" layers
• export support for relations
• configured tagging for turn restriction layer (on dev)
• specify whether turn prohibited is left, right etc.
• copy tagging onto production
• overpass API query to get existing node and way IDs within export
• updated versions of NZOpenGPS data on production (already tested this process for Wellington).

[rcoup]

I'm concerned at the amount of raw SQL string construction - is there a way we can sanitise/check layer & attribute names to ensure no spaces/semicolons/quotes/etc? Maybe just via a Django model RegexValidator

[stephend]

is there a way we can sanitise/check layer & attribute names to ensure no spaces/semicolons/quotes/etc?

I take your point, but we can't simply use validation. Layers can only be created through the admin interface, and attribute names are pulled directly from database column names. That's not to suggest that admins (or people who host data services that we pull from) can't inject things, but it's not something random people can do. Even then, layer and attribute names are sanitised when we load them into the database...

If you're deeply concerned about the SQL construction, it'll have to be through escaping at string construction time.

[rcoup]

Even then, layer and attribute names are sanitised when we load them into the database...

So, isn't that an acceptable approach then? If ;foo.bar-- isn't an acceptable layer name & it gets sanitised then errors caused by bad names shouldn't happen.

[stephend]

So, isn't that an acceptable approach then? If ;foo.bar-- isn't an acceptable layer name & it gets sanitised then errors caused by bad names shouldn't happen.

Yeah, there's no actual way to exploit this in practice unless you /already/ have write access to the database. It doesn't hugely concern me, except that the chain of things protecting it is fairly long and complicated, rather than being Defence In Depth.

[rcoup]

It's more about weird names causing SQL crashes than malicious intent. Since as you say, users who can mess with it already have DB access

[stephend]

It's more about weird names causing SQL crashes than malicious intent. Since as you say, users who can mess with it already have DB access

Fair enough. That said, it doesn't seem quite so urgent, given how concerned we are with getting turn restrictions done ASAP. I'll ticket separately?

[rcoup]

Even then, layer and attribute names are sanitised when we load them into the database...

From that^ I was assuming it's already done? (if not, ticket separately)

[stephend]

From that^ I was assuming it's already done? (if not, ticket separately)

They're done in the helper scripts that load the data into the database from source (by ogr2ogr, which has an option to sanitise names to things that don't need double-quotes in SQL).

Which seems fairly disconnected from the actual use of that information: in particular, there's nothing to require sanitisation if someone loads in data directly from a geo database, or without using a helper script.

[rcoup]

LGTM, could the turn angle bit have some docs just to make it clear what's what and where it comes from?