diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 72fd1718..e66421ad 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -72,12 +72,12 @@ jobs: restore-keys: | ${{ runner.os }}-go- - - name: Get C dependencies + - name: Get C dependencies and 3rd-party tools run: | sudo add-apt-repository ppa:strukturag/libheif sudo add-apt-repository ppa:strukturag/libde265 sudo apt-get update - sudo apt-get install -y libdlib-dev libblas-dev libatlas-base-dev liblapack-dev libjpeg-turbo8-dev libheif-dev + sudo apt-get install -y libdlib-dev libblas-dev libatlas-base-dev liblapack-dev libjpeg-turbo8-dev libheif-dev ffmpeg exiftool - name: Get GO dependencies run: | diff --git a/.gitignore b/.gitignore index 3fa8eb6a..5d83cd15 100644 --- a/.gitignore +++ b/.gitignore @@ -13,8 +13,9 @@ testing.env # docker docker-compose.yml /Makefile -storage -database +/storage +/database +/tmp # dependencies node_modules/ diff --git a/api/database/database.go b/api/database/database.go index 01e5b411..f3ab5804 100644 --- a/api/database/database.go +++ b/api/database/database.go @@ -8,6 +8,7 @@ import ( "time" "github.com/photoview/photoview/api/database/drivers" + "github.com/photoview/photoview/api/database/migrations" "github.com/photoview/photoview/api/graphql/models" "github.com/photoview/photoview/api/utils" "github.com/pkg/errors" @@ -63,7 +64,7 @@ func GetSqliteAddress(path string) (*url.URL, error) { queryValues.Add("cache", "shared") queryValues.Add("mode", "rwc") // queryValues.Add("_busy_timeout", "60000") // 1 minute - queryValues.Add("_journal_mode", "WAL") // Write-Ahead Logging (WAL) mode + queryValues.Add("_journal_mode", "WAL") // Write-Ahead Logging (WAL) mode queryValues.Add("_locking_mode", "NORMAL") // allows concurrent reads and writes address.RawQuery = queryValues.Encode() @@ -194,6 +195,11 @@ func MigrateDatabase(db *gorm.DB) error { log.Printf("Failed to run exif fields migration: %v\n", err) } + // Remove invalid GPS data from DB + if err := migrations.MigrateForExifGPSCorrection(db); err != nil { + log.Printf("Failed to run exif GPS correction migration: %v\n", err) + } + return nil } diff --git a/api/database/migrations/exif_invalid_gps.go b/api/database/migrations/exif_invalid_gps.go new file mode 100644 index 00000000..c7c33c88 --- /dev/null +++ b/api/database/migrations/exif_invalid_gps.go @@ -0,0 +1,23 @@ +package migrations + +import ( + "github.com/photoview/photoview/api/graphql/models" + "github.com/pkg/errors" + "gorm.io/gorm" +) + +// MigrateForExifGPSCorrection finds and removes invalid GPS data from media_exif table +func MigrateForExifGPSCorrection(db *gorm.DB) error { + return db.Transaction(func(tx *gorm.DB) error { + if err := tx.Model(&models.MediaEXIF{}). + Where("ABS(gps_longitude) > ?", 90). + Or("ABS(gps_latitude) > ?", 90). + Updates(map[string]interface{}{ + "gps_latitude": nil, + "gps_longitude": nil, + }).Error; err != nil { + return errors.Wrap(err, "failed to remove invalid GPS data from media_exif table") + } + return nil + }) +} diff --git a/api/database/migrations/exif_invalid_gps_test.go b/api/database/migrations/exif_invalid_gps_test.go new file mode 100644 index 00000000..6f9647ce --- /dev/null +++ b/api/database/migrations/exif_invalid_gps_test.go @@ -0,0 +1,74 @@ +package migrations_test + +import ( + "bufio" + "math" + "os" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/photoview/photoview/api/database/migrations" + "github.com/photoview/photoview/api/graphql/models" + "github.com/photoview/photoview/api/test_utils" +) + +func TestExifMigration(t *testing.T) { + envFile, err := os.Open("/home/runner/work/photoview/photoview/api/testing.env") + if err != nil { + t.Fatalf("Failed to open environment file: %v", err) + } + defer envFile.Close() + + scanner := bufio.NewScanner(envFile) + for scanner.Scan() { + line := scanner.Text() + parts := strings.SplitN(line, "=", 2) + if len(parts) != 2 { + t.Fatalf("Invalid line in environment file: %s", line) + } + key, value := parts[0], strings.Trim(parts[1], "'") + os.Setenv(key, value) + } + + db := test_utils.DatabaseTest(t) + defer db.Exec("DELETE FROM media_exif") // Clean up after test + + // Create test data + exifEntries := []models.MediaEXIF{ + {GPSLatitude: floatPtr(90.1), GPSLongitude: floatPtr(90.0)}, // Invalid GPSLatitude + {GPSLatitude: floatPtr(-90.1), GPSLongitude: floatPtr(-90.0)}, // Invalid GPSLatitude + {GPSLatitude: floatPtr(90.0), GPSLongitude: floatPtr(90.1)}, // Invalid GPSLongitude + {GPSLatitude: floatPtr(-90.0), GPSLongitude: floatPtr(-90.1)}, // Invalid GPSLongitude + {GPSLatitude: floatPtr(90.0), GPSLongitude: floatPtr(90.0)}, // Valid GPS data + {GPSLatitude: floatPtr(-90.0), GPSLongitude: floatPtr(-90.0)}, // Valid GPS data + {GPSLatitude: floatPtr(90.1), GPSLongitude: floatPtr(90.1)}, // Invalid GPSLatitude and GPSLongitude + {GPSLatitude: floatPtr(-90.1), GPSLongitude: floatPtr(-90.1)}, // Invalid GPSLatitude and GPSLongitude + } + + // Insert test data + for _, entry := range exifEntries { + assert.NoError(t, db.Create(&entry).Error) + } + + // Run migration + assert.NoError(t, migrations.MigrateForExifGPSCorrection(db)) + + // Validate the results + var results []models.MediaEXIF + assert.NoError(t, db.Find(&results).Error) + + for _, entry := range results { + if entry.GPSLatitude != nil { + assert.LessOrEqual(t, math.Abs(*entry.GPSLatitude), 90.0, "GPSLatitude should be within [-90, 90]: %+v", entry) + } + if entry.GPSLongitude != nil { + assert.LessOrEqual(t, math.Abs(*entry.GPSLongitude), 90.0, "GPSLongitude should be within [-90, 90]: %+v", entry) + } + } +} + +func floatPtr(f float64) *float64 { + return &f +} diff --git a/api/scanner/exif/exif_parser_external.go b/api/scanner/exif/exif_parser_external.go index e5483e70..2eb905e8 100644 --- a/api/scanner/exif/exif_parser_external.go +++ b/api/scanner/exif/exif_parser_external.go @@ -63,6 +63,31 @@ func sanitizeEXIF(exif *models.MediaEXIF) { } } +func extractValidGpsData(fileInfo *exiftool.FileMetadata, media_path string) (*float64, *float64) { + var GPSLat, GPSLong *float64 + + // GPS coordinates - longitude + longitudeRaw, err := fileInfo.GetFloat("GPSLongitude") + if err == nil { + GPSLong = &longitudeRaw + } + + // GPS coordinates - latitude + latitudeRaw, err := fileInfo.GetFloat("GPSLatitude") + if err == nil { + GPSLat = &latitudeRaw + } + + // GPS data validation + if (GPSLat != nil && math.Abs(*GPSLat) > 90) || (GPSLong != nil && math.Abs(*GPSLong) > 90) { + log.Printf( + "Incorrect GPS data in the %s Exif data: %f, %f, while expected values between '-90' and '90'. Ignoring GPS data.", + media_path, *GPSLat, *GPSLong) + return nil, nil + } + return GPSLat, GPSLong +} + func (p *externalExifParser) ParseExif(media_path string) (returnExif *models.MediaEXIF, returnErr error) { // ExifTool - No print conversion mode if p.et == nil { @@ -182,18 +207,10 @@ func (p *externalExifParser) ParseExif(media_path string) (returnExif *models.Me newExif.ExposureProgram = &expProgram } - // GPS coordinates - longitude - longitudeRaw, err := fileInfo.GetFloat("GPSLongitude") - if err == nil { + // Get GPS data + newExif.GPSLatitude, newExif.GPSLongitude = extractValidGpsData(&fileInfo, media_path) + if (newExif.GPSLatitude != nil) && (newExif.GPSLongitude != nil) { found_exif = true - newExif.GPSLongitude = &longitudeRaw - } - - // GPS coordinates - latitude - latitudeRaw, err := fileInfo.GetFloat("GPSLatitude") - if err == nil { - found_exif = true - newExif.GPSLatitude = &latitudeRaw } if !found_exif { diff --git a/api/scanner/exif/exif_parser_internal.go b/api/scanner/exif/exif_parser_internal.go index 4244b86d..398635dc 100644 --- a/api/scanner/exif/exif_parser_internal.go +++ b/api/scanner/exif/exif_parser_internal.go @@ -3,6 +3,7 @@ package exif import ( "fmt" "log" + "math" "math/big" "os" "time" @@ -142,8 +143,16 @@ func (p internalExifParser) ParseExif(media_path string) (returnExif *models.Med lat, long, err := exifTags.LatLong() if err == nil { - newExif.GPSLatitude = &lat - newExif.GPSLongitude = &long + if math.Abs(lat) > 90 || math.Abs(long) > 90 { + returnExif = &newExif + log.Printf( + "Incorrect GPS data in the %s Exif data: %f, %f, while expected values between '-90' and '90'. Ignoring GPS data.", + media_path, long, lat) + return + } else { + newExif.GPSLatitude = &lat + newExif.GPSLongitude = &long + } } returnExif = &newExif diff --git a/api/scanner/exif/exif_test.go b/api/scanner/exif/exif_test.go index ca0ae43d..a96640f1 100644 --- a/api/scanner/exif/exif_test.go +++ b/api/scanner/exif/exif_test.go @@ -43,11 +43,12 @@ func TestExifParsers(t *testing.T) { images := []struct { path string - assert func(t *testing.T, exif *models.MediaEXIF) + assert func(t *testing.T, exif *models.MediaEXIF, err error) }{ { path: "./test_data/bird.jpg", - assert: func(t *testing.T, exif *models.MediaEXIF) { + assert: func(t *testing.T, exif *models.MediaEXIF, err error) { + assert.NoError(t, err) assert.EqualValues(t, *exif.Description, "Photo of a Bird") assert.WithinDuration(t, *exif.DateShot, time.Unix(1336318784, 0).UTC(), time.Minute) assert.EqualValues(t, *exif.Camera, "Canon EOS 600D") @@ -65,16 +66,61 @@ func TestExifParsers(t *testing.T) { }, { path: "./test_data/stripped.jpg", - assert: func(t *testing.T, exif *models.MediaEXIF) { - assert.Nil(t, exif) + assert: func(t *testing.T, exif *models.MediaEXIF, err error) { + assert.NoError(t, err) + if exif == nil { + assert.Nil(t, exif) + } else { + assert.Equal(t, 0, exif.ID) + assert.True(t, exif.CreatedAt.IsZero()) + assert.True(t, exif.UpdatedAt.IsZero()) + assert.Nil(t, exif.Description) + assert.Nil(t, exif.Camera) + assert.Nil(t, exif.Maker) + assert.Nil(t, exif.Lens) + assert.Nil(t, exif.Exposure) + assert.Nil(t, exif.Aperture) + assert.Nil(t, exif.Iso) + assert.Nil(t, exif.FocalLength) + assert.Nil(t, exif.Flash) + assert.Nil(t, exif.Orientation) + assert.Nil(t, exif.ExposureProgram) + assert.Nil(t, exif.GPSLatitude) + assert.Nil(t, exif.GPSLongitude) + } }, }, { path: "./test_data/bad-exif.jpg", - assert: func(t *testing.T, exif *models.MediaEXIF) { + assert: func(t *testing.T, exif *models.MediaEXIF, err error) { + assert.NoError(t, err) assert.Nil(t, exif.Exposure) }, }, + { + path: "./test_data/IncorrectGPS.jpg", + assert: func(t *testing.T, exif *models.MediaEXIF, err error) { + assert.Nil(t, exif.GPSLatitude, + "GPSLatitude expected to be NULL for an incorrect input data: %+v", exif.GPSLatitude) + assert.Nil(t, exif.GPSLongitude, + "GPSLongitude expected to be NULL for an incorrect input data: %+v", exif.GPSLongitude) + }, + }, + { + path: "./test_data/CorrectGPS.jpg", + assert: func(t *testing.T, exif *models.MediaEXIF, err error) { + const precision = 1e-7 + assert.NoError(t, err) + assert.NotNil(t, exif.GPSLatitude, + "GPSLatitude expected to be Not-NULL for a correct input data: %+v", exif.GPSLatitude) + assert.NotNil(t, exif.GPSLongitude, + "GPSLongitude expected to be Not-NULL for a correct input data: %+v", exif.GPSLongitude) + assert.InDelta(t, *exif.GPSLatitude, 44.478997222222226, precision, + "The exact value from input data is expected: %+v", exif.GPSLatitude) + assert.InDelta(t, *exif.GPSLongitude, 11.297922222222223, precision, + "The exact value from input data is expected: %+v", exif.GPSLongitude) + }, + }, } for _, p := range parsers { @@ -90,9 +136,7 @@ func TestExifParsers(t *testing.T) { exif, err := p.parser.ParseExif(img.path) - if assert.NoError(t, err) { - img.assert(t, exif) - } + img.assert(t, exif, err) }) } } diff --git a/api/scanner/exif/test_data/CorrectGPS.jpg b/api/scanner/exif/test_data/CorrectGPS.jpg new file mode 100644 index 00000000..6a7667b0 Binary files /dev/null and b/api/scanner/exif/test_data/CorrectGPS.jpg differ diff --git a/api/scanner/exif/test_data/IncorrectGPS.jpg b/api/scanner/exif/test_data/IncorrectGPS.jpg new file mode 100755 index 00000000..18584b72 Binary files /dev/null and b/api/scanner/exif/test_data/IncorrectGPS.jpg differ