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.

Sign up for GitHub

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

Jump to bottom

vulnerability to PROJ6/GDAL3 #7

Closed
rsbivand opened this issue Feb 4, 2020 · 7 comments
Closed

vulnerability to PROJ6/GDAL3 #7

rsbivand opened this issue Feb 4, 2020 · 7 comments
Labels
high-priority

Comments

@rsbivand
Copy link

rsbivand commented Feb 4, 2020

Running revdeps from sp (sp (my github fork) with development rgdal from R-Forge):

> library(testthat)
> library(lutz)
> 
> test_check("lutz")
Error : foo is not a valid time zone. See ?OlsonNames
OGR: Corrupt data
── 1. Error: tz_lookup.sf works (@test-tz_lookup2.R#36)  ───────────────────────
OGR error
Backtrace:
  1. testthat::expect_equal(...)
  4. lutz::tz_lookup(pt, 3005, method = "accurate")
  6. lutz:::tz_lookup_accurate.sfc(x, crs)
  8. lutz:::tz_lookup_accurate.sf(x_sf, crs = crs)
  9. lutz:::fix_sf(x, crs)
 10. sf:::Ops.crs(sf::st_crs(x), sf::st_crs(4326))
 11. sf:::Ops.crs(e1, e2)
 12. sf:::CPL_crs_equivalent(e1$proj4string, e2$proj4string)

OGR: Corrupt data
── 2. Error: tz_lookup.SpatialPoints works (@test-tz_lookup2.R#52)  ────────────
OGR error
Backtrace:
  1. testthat::expect_equal(...)
  4. lutz::tz_lookup(pt, 3005, method = "accurate")
  6. lutz:::tz_lookup_accurate.SpatialPoints(x, crs)
  8. lutz:::tz_lookup_accurate.sf(x, crs)
  9. lutz:::fix_sf(x, crs)
 10. sf:::Ops.crs(sf::st_crs(x), sf::st_crs(4326))
 11. sf:::Ops.crs(e1, e2)
 12. sf:::CPL_crs_equivalent(e1$proj4string, e2$proj4string)

══ testthat results  ═══════════════════════════════════════════════════════════
[ OK: 1084 | SKIPPED: 0 | WARNINGS: 12 | FAILED: 2 ]
1. Error: tz_lookup.sf works (@test-tz_lookup2.R#36) 
2. Error: tz_lookup.SpatialPoints works (@test-tz_lookup2.R#52) 

Error: testthat unit tests failed
Execution halted

See:

http://rgdal.r-forge.r-project.org/articles/PROJ6_GDAL3.html
r-spatial/sf#1231
r-spatial/sf#1187
r-spatial/sf#1146
r-spatial/discuss#28

for background. See:

r-spatial/discuss#28 (comment)

for a way of testing fixes in a docker container contributed by Jakub Nowosad.
@ateucher
Copy link
Owner

ateucher commented Feb 4, 2020

Thanks for the heads-up @rsbivand

ateucher added a commit that referenced this issue Feb 4, 2020
@ateucher
Add GH ACtion to check with GDAL3/PROJ6 #7
f97c3aa
ateucher added a commit that referenced this issue Feb 4, 2020
@ateucher
Matrix build - install dev sp and rgdal #7
f9602a7
@ateucher
Copy link
Owner

ateucher commented Feb 4, 2020

@rsbivand did you get these errors with a dev version of sf installed? It looks like that's where the error is occurring (not with sp/rgdal)...

@rsbivand
Copy link
Author

rsbivand commented Feb 5, 2020

Development versions of sp, rgdal, sf (SetFromUserInput branch), run overnigt:

testthat.Rout.fail.sp134rgdal153sf083GDAL304PROJ630.txt

Development versions of sp, rgdal, released sf 0.8-1

testthat.Rout.failsp134rgdal153sf081GDAL304PROJ630.txt

Development versions of sp, rgdal, old released sf 0.8-0:

testthat.Rout.fail.sp134rgdal153sf080GDAL304PROJ630.txt

Released sp, rgdal, old released sf 0.8-0:

testthat.Routsp132rgdal148sf080GDAL303PROJ630.txt

To replicate you'd need

remotes::install_github("rsbivand/sp", force=TRUE)
install.packages("rgdal", repos="http://R-Forge.R-project.org")

on PROJ 630/GDAL304 or similar. And enjoy diving into testthat code ...

in test-tz_lookup2 l.35 this looks wrong:

  expect_error(tz_lookup_coords(pts, method = "accurate"),
               "It looks like you are trying to get the tz of an sf/sfc or SpatialPoints object") # nolint
Error : It looks like you are trying to get the tz of an sf/sfc or SpatialPoints object! Use tz_lookup() instead.

l. 57

> tz <- tz_lookup_coords(coords[2], coords[1], method = "accurate")
Warning message:
In tz_lookup_accurate.sf(ll_sf) :
  Some points are in areas with more than one time zone defined.These are often disputed areas and should be treated with care.

But running that file line by line gives nothing. I've another 60 packages to handle, so I'll leave this to you - it is projection-related.

@rsbivand
Copy link
Author

rsbivand commented Feb 5, 2020

On my laptop with:

[1] ‘1.3.4’
> packageVersion("rgdal")
[1] ‘1.5.3’
> packageVersion("sf")
[1] ‘0.8.2’
> sf_extSoftVersion()
Error in sf_extSoftVersion() : 
  could not find function "sf_extSoftVersion"
> sf::sf_extSoftVersion()
                 GEOS                  GDAL                proj.4 
              "3.8.0" "3.1.0dev-0d64124ea4"               "7.0.0" 
       GDAL_with_GEOS            USE_PROJ_H 
               "true"                "true" 
> rgdal::rgdal_extSoftVersion()
                 GDAL        GDAL_with_GEOS                  PROJ 
"3.1.0dev-0d64124ea4"                "TRUE"               "7.0.0" 
                   sp 
              "1.3-4"

no problem:

testthat.Rout.txt

So I need to (re)-re-check settings on my desktop.

@ateucher
Copy link
Owner

ateucher commented Feb 5, 2020

Thanks very much for the detail @rsbivand. I haven't been able to replicate the errors using the Nowasad/rspatial_proj6 docker image and development versions of sf, sp, and rgdal, though as you can see I do get warnings related to proj4 strings:

> devtools::test()
Loading lutz
Testing lutz
✓ |  OK F W S | Context
⠏ |  10       | time-zone utilitiesError : foo is not a valid time zone. See ?OlsonNames
✓ |  12       | time-zone utilities [2.5 s]
✓ |  26   10   | tz_lookup works [0.8 s]
──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
test-tz_lookup.R:61: warning: tz_lookup.SpatialPoints works
Discarded datum WGS_1984 in CRS definition,
 but +towgs84= values preserved

test-tz_lookup.R:61: warning: tz_lookup.SpatialPoints works
Discarded datum Unknown_based_on_WGS84_ellipsoid in CRS definition,
 but +towgs84= values preserved

test-tz_lookup.R:61: warning: tz_lookup.SpatialPoints works
Discarded datum WGS_1984 in CRS definition,
 but +towgs84= values preserved

test-tz_lookup.R:62: warning: tz_lookup.SpatialPoints works
Discarded datum WGS_1984 in CRS definition,
 but +towgs84= values preserved

test-tz_lookup.R:62: warning: tz_lookup.SpatialPoints works
Discarded datum Unknown_based_on_WGS84_ellipsoid in CRS definition,
 but +towgs84= values preserved

test-tz_lookup.R:62: warning: tz_lookup.SpatialPoints works
Discarded datum WGS_1984 in CRS definition,
 but +towgs84= values preserved

test-tz_lookup.R:65: warning: tz_lookup.SpatialPoints works
Discarded datum Unknown_based_on_GRS80_ellipsoid in CRS definition,
 but +towgs84= values preserved

test-tz_lookup.R:65: warning: tz_lookup.SpatialPoints works
Discarded datum Unknown_based_on_GRS80_ellipsoid in CRS definition,
 but +towgs84= values preserved

test-tz_lookup.R:65: warning: tz_lookup.SpatialPoints works
Discarded datum WGS_1984 in CRS definition,
 but +towgs84= values preserved

test-tz_lookup.R:68: warning: tz_lookup.SpatialPoints works
Discarded datum WGS_1984 in CRS definition,
 but +towgs84= values preserved
──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
✓ |  29       | test-tzlookup2.R [4.3 s]
✓ | 1020       | tz-lookup js edge cases [0.8 s]

══ Results ═══════════════════════════════════════════════════════════════════════════════════════════════════════════
Duration: 8.5 s

OK:       1087
Failed:   0
Warnings: 10
Skipped:  0

> devtools::session_info()
─ Session info ─────────────────────────────────────────────────────────────────────────────────────────────────────
 setting  value                       
 version  R version 3.6.2 (2019-12-12)
 os       Debian GNU/Linux 10 (buster)
 system   x86_64, linux-gnu           
 ui       RStudio                     
 language (EN)                        
 collate  en_US.UTF-8                 
 ctype    en_US.UTF-8                 
 tz       Etc/UTC                     
 date     2020-02-05                  

─ Packages ─────────────────────────────────────────────────────────────────────────────────────────────────────────
 ! package     * version date       lib source                       
   assertthat    0.2.1   2019-03-21 [1] CRAN (R 3.6.2)               
   backports     1.1.5   2019-10-02 [1] CRAN (R 3.6.2)               
   callr         3.4.1   2020-01-24 [1] CRAN (R 3.6.2)               
   class         7.3-15  2019-01-01 [2] CRAN (R 3.6.2)               
   classInt      0.4-2   2019-10-17 [1] CRAN (R 3.6.2)               
   cli           2.0.1   2020-01-08 [1] CRAN (R 3.6.2)               
   colorspace    1.4-1   2019-03-18 [1] CRAN (R 3.6.2)               
   crayon        1.3.4   2017-09-16 [1] CRAN (R 3.6.2)               
   DBI           1.1.0   2019-12-15 [1] CRAN (R 3.6.2)               
   desc          1.2.0   2018-05-01 [1] CRAN (R 3.6.2)               
   devtools      2.2.1   2019-09-24 [1] CRAN (R 3.6.2)               
   digest        0.6.23  2019-11-23 [1] CRAN (R 3.6.2)               
   e1071         1.7-3   2019-11-26 [1] CRAN (R 3.6.2)               
   ellipsis      0.3.0   2019-09-20 [1] CRAN (R 3.6.2)               
   fansi         0.4.1   2020-01-08 [1] CRAN (R 3.6.2)               
   fs            1.3.1   2019-05-06 [1] CRAN (R 3.6.2)               
   ggplot2       3.2.1   2019-08-10 [1] CRAN (R 3.6.2)               
   glue          1.3.1   2019-03-12 [1] CRAN (R 3.6.2)               
   gtable        0.3.0   2019-03-25 [1] CRAN (R 3.6.2)               
   KernSmooth    2.23-16 2019-10-15 [2] CRAN (R 3.6.2)               
   lattice       0.20-38 2018-11-04 [2] CRAN (R 3.6.2)               
   lazyeval      0.2.2   2019-03-15 [1] CRAN (R 3.6.2)               
   lifecycle     0.1.0   2019-08-01 [1] CRAN (R 3.6.2)               
   lubridate     1.7.4   2018-04-11 [1] CRAN (R 3.6.2)               
 P lutz        * 0.3.1   2019-07-19 [?] CRAN (R 3.6.2)               
   magrittr      1.5     2014-11-22 [1] CRAN (R 3.6.2)               
   memoise       1.1.0   2017-04-21 [1] CRAN (R 3.6.2)               
   munsell       0.5.0   2018-06-12 [1] CRAN (R 3.6.2)               
   pillar        1.4.3   2019-12-20 [1] CRAN (R 3.6.2)               
   pkgbuild      1.0.6   2019-10-09 [1] CRAN (R 3.6.2)               
   pkgconfig     2.0.3   2019-09-22 [1] CRAN (R 3.6.2)               
   pkgload       1.0.2   2018-10-29 [1] CRAN (R 3.6.2)               
   prettyunits   1.1.1   2020-01-24 [1] CRAN (R 3.6.2)               
   processx      3.4.1   2019-07-18 [1] CRAN (R 3.6.2)               
   ps            1.3.0   2018-12-21 [1] CRAN (R 3.6.2)               
   R6            2.4.1   2019-11-12 [1] CRAN (R 3.6.2)               
   Rcpp          1.0.3   2019-11-08 [1] CRAN (R 3.6.2)               
   remotes       2.1.0   2019-06-24 [1] CRAN (R 3.6.2)               
   rgdal         1.5-4   2020-02-05 [1] R-Forge (R 3.6.2)            
   rlang         0.4.4   2020-01-28 [1] CRAN (R 3.6.2)               
   rprojroot     1.3-2   2018-01-03 [1] CRAN (R 3.6.2)               
   rstudioapi    0.10    2019-03-19 [1] CRAN (R 3.6.2)               
   scales        1.1.0   2019-11-18 [1] CRAN (R 3.6.2)               
   sessioninfo   1.1.1   2018-11-05 [1] CRAN (R 3.6.2)               
   sf            0.8-2   2020-02-05 [1] Github (r-spatial/sf@edcccbe)
   sp            1.3-4   2020-02-05 [1] Github (rsbivand/sp@38d9ae9) 
   stringi       1.4.5   2020-01-11 [1] CRAN (R 3.6.2)               
   stringr       1.4.0   2019-02-10 [1] CRAN (R 3.6.2)               
   testthat    * 2.3.1   2019-12-01 [1] CRAN (R 3.6.2)               
   tibble        2.1.3   2019-06-06 [1] CRAN (R 3.6.2)               
   units         0.6-5   2019-10-08 [1] CRAN (R 3.6.2)               
   usethis       1.5.1   2019-07-04 [1] CRAN (R 3.6.2)               
   withr         2.1.2   2018-03-15 [1] CRAN (R 3.6.2)               

[1] /usr/local/lib/R/site-library
[2] /usr/local/lib/R/library
[3] /usr/lib/R/library

 P ── Loaded and on-disk path mismatch.

> sf::sf_extSoftVersion()
          GEOS           GDAL         proj.4 GDAL_with_GEOS     USE_PROJ_H 
       "3.8.0"        "3.0.2"        "6.2.1"        "false"         "true" 

> rgdal::rgdal_extSoftVersion()
          GDAL GDAL_with_GEOS           PROJ             sp 
       "3.0.2"        "FALSE"        "6.2.1"        "1.3-4"

@rsbivand
Copy link
Author

rsbivand commented Feb 5, 2020

OK. The warnings are work in progress, and can be muted a bit when the packages get to release. The problem is that in GDAL when reading CRS in files, the exportToProj4() function is used to create the string for CRS and crs objects. As https://gdal.org/api/ogr_srs_api.html#_CPPv416OSRExportToProj420OGRSpatialReferenceHPPc says, the +datum and +towgs84 components are effectively now not to be relied on, so the warnings check the output Proj4 string against the internal CRS representation before export. Going forward, sf and rgdal will export WKT2 representations, so that read files don't degrade (we hope).

@ateucher
Copy link
Owner

ateucher commented Feb 5, 2020

Thanks very much, I really appreciate all the work you're doing to support the transition. I will keep an eye on this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
high-priority
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@ateucher @rsbivand