From de4b8921bde3f269755bdcbe91a9ccc9fceb6a64 Mon Sep 17 00:00:00 2001 From: Dustin Oprea Date: Mon, 25 May 2020 19:22:47 -0400 Subject: [PATCH] Log guesses about implementation incongruities for invalid tags 2020/05/25 19:21:33 exif.utility: [WARNING] Tag with ID (0x0201) in IFD [IFD/Exif] is not recognized and will be ignored. 2020/05/25 19:21:33 exif.utility: [WARNING] (cont'd) Tag [JPEGInterchangeFormat] with the same ID has been found in IFD [IFD] and may be related. The tag you were looking for might have been written to the wrong IFD by a buggy implementation. 2020/05/25 19:21:33 exif.utility: [WARNING] Tag with ID (0x0202) in IFD [IFD/Exif] is not recognized and will be ignored. 2020/05/25 19:21:33 exif.utility: [WARNING] (cont'd) Tag [JPEGInterchangeFormatLength] with the same ID has been found in IFD [IFD] and may be related. The tag you were looking for might have been written to the wrong IFD by a buggy implementation. Closes #38 --- v2/ifd_builder.go | 2 +- v2/tags.go | 70 ++++++++- v2/tags_test.go | 361 +++++++++++++++++++++++++++++++++++++++++++++- v2/utility.go | 5 + 4 files changed, 431 insertions(+), 7 deletions(-) diff --git a/v2/ifd_builder.go b/v2/ifd_builder.go index dbc3526..7b5df19 100644 --- a/v2/ifd_builder.go +++ b/v2/ifd_builder.go @@ -188,7 +188,7 @@ func NewStandardBuilderTag(ifdPath string, it *IndexedTag, byteOrder binary.Byte // If there is more than one supported type, we'll go with the larger to // encode with. It'll use the same amount of fixed-space, and we'll // eliminate unnecessary overflows/issues. - tagType := it.WidestSupportedType() + tagType := it.GetEncodingType(value) var rawBytes []byte if it.DoesSupportType(exifcommon.TypeUndefined) == true { diff --git a/v2/tags.go b/v2/tags.go index 90812e9..416c315 100644 --- a/v2/tags.go +++ b/v2/tags.go @@ -113,7 +113,7 @@ func (it *IndexedTag) Is(ifdPath string, id uint16) bool { // WidestSupportedType returns the largest type that this tag's value can // occupy -func (it *IndexedTag) WidestSupportedType() exifcommon.TagTypePrimitive { +func (it *IndexedTag) GetEncodingType(value interface{}) exifcommon.TagTypePrimitive { if len(it.SupportedTypes) == 0 { log.Panicf("IndexedTag [%s] (%d) has no supported types.", it.IfdPath, it.Id) } else if len(it.SupportedTypes) == 1 { @@ -122,19 +122,34 @@ func (it *IndexedTag) WidestSupportedType() exifcommon.TagTypePrimitive { supportsLong := false supportsShort := false + supportsRational := false + supportsSignedRational := false for _, supportedType := range it.SupportedTypes { if supportedType == exifcommon.TypeLong { supportsLong = true } else if supportedType == exifcommon.TypeShort { supportsShort = true + } else if supportedType == exifcommon.TypeRational { + supportsRational = true + } else if supportedType == exifcommon.TypeSignedRational { + supportsSignedRational = true } } - // If it supports both long and short ints. This is currently our common - // and only case. The moment more are added to our tags database, we'll have - // to add more checks here if we add more than just a LONG and just a SHORT. + // We specifically check for the cases that we know to expect. + if supportsLong == true && supportsShort == true { return exifcommon.TypeLong + } else if supportsRational == true && supportsSignedRational == true { + if value == nil { + log.Panicf("GetEncodingType: require value to be given") + } + + if _, ok := value.(exifcommon.SignedRational); ok == true { + return exifcommon.TypeSignedRational + } else { + return exifcommon.TypeRational + } } log.Panicf("WidestSupportedType() case is not handled for tag [%s] (0x%04x): %v", it.IfdPath, it.Id, it.SupportedTypes) @@ -237,6 +252,53 @@ func (ti *TagIndex) Get(ii *exifcommon.IfdIdentity, id uint16) (it *IndexedTag, return it, nil } +var ( + // tagGuessDefaultIfdIdentities describes which IFDs we'll look for a given + // tag-ID in, if it's not found where it's supposed to be. We suppose that + // Exif-IFD tags might be found in IFD0 or IFD1, or IFD0/IFD1 tags might be + // found in the Exif IFD. This is the only thing we've seen so far. So, this + // is the limit of our guessing. + tagGuessDefaultIfdIdentities = []*exifcommon.IfdIdentity{ + exifcommon.IfdExifStandardIfdIdentity, + exifcommon.IfdStandardIfdIdentity, + } +) + +// FindFirst looks for the given tag-ID in each of the given IFDs in the given +// order. If `fqIfdPaths` is `nil` then use a default search order. This defies +// the standard, which requires each tag to exist in certain IFDs. This is a +// contingency to make recommendations for malformed data. +// +// Things *can* end badly here, in that the same tag-ID in different IFDs might +// describe different data and different ata-types, and our decode might then +// produce binary and non-printable data. +func (ti *TagIndex) FindFirst(id uint16, ifdIdentities []*exifcommon.IfdIdentity) (it *IndexedTag, err error) { + defer func() { + if state := recover(); state != nil { + err = log.Wrap(state.(error)) + } + }() + + if ifdIdentities == nil { + ifdIdentities = tagGuessDefaultIfdIdentities + } + + for _, ii := range ifdIdentities { + it, err := ti.Get(ii, id) + if err != nil { + if err == ErrTagNotFound { + continue + } + + log.Panic(err) + } + + return it, nil + } + + return nil, ErrTagNotFound +} + // GetWithName returns information about the non-IFD tag given a tag name. func (ti *TagIndex) GetWithName(ii *exifcommon.IfdIdentity, name string) (it *IndexedTag, err error) { defer func() { diff --git a/v2/tags_test.go b/v2/tags_test.go index e9dafe7..75c28c4 100644 --- a/v2/tags_test.go +++ b/v2/tags_test.go @@ -1,6 +1,7 @@ package exif import ( + "reflect" "testing" "github.com/dsoprea/go-logging" @@ -8,7 +9,238 @@ import ( "github.com/dsoprea/go-exif/v2/common" ) -func TestGet(t *testing.T) { +func TestIndexedTag_String(t *testing.T) { + it := &IndexedTag{ + Id: 0xb, + Name: "some_name", + IfdPath: "ifd/path", + SupportedTypes: []exifcommon.TagTypePrimitive{ + exifcommon.TagTypePrimitive(11), + exifcommon.TagTypePrimitive(22), + }, + } + + if it.String() != "TAG" { + t.Fatalf("String output not correct: [%s]", it.String()) + } +} + +func TestIndexedTag_IsName_True(t *testing.T) { + it := &IndexedTag{ + Name: "some_name", + IfdPath: "ifd/path", + } + + if it.IsName("ifd/path", "some_name") != true { + t.Fatalf("IsName is not true.") + } +} + +func TestIndexedTag_IsName_FalseOnName(t *testing.T) { + it := &IndexedTag{ + Name: "some_name", + IfdPath: "ifd/path", + } + + if it.IsName("ifd/path", "some_name2") != false { + t.Fatalf("IsName is not false.") + } +} + +func TestIndexedTag_IsName_FalseOnIfdPath(t *testing.T) { + it := &IndexedTag{ + Name: "some_name", + IfdPath: "ifd/path", + } + + if it.IsName("ifd/path2", "some_name") != false { + t.Fatalf("IsName is not false.") + } +} + +func TestIndexedTag_Is_True(t *testing.T) { + it := &IndexedTag{ + Id: 0x11, + IfdPath: "ifd/path", + } + + if it.Is("ifd/path", 0x11) != true { + t.Fatalf("Is is not true.") + } +} + +func TestIndexedTag_Is_FalseOnId(t *testing.T) { + it := &IndexedTag{ + Id: 0x11, + IfdPath: "ifd/path", + } + + if it.Is("ifd/path", 0x12) != false { + t.Fatalf("Is is not false.") + } +} + +func TestIndexedTag_Is_FalseOnIfdName(t *testing.T) { + it := &IndexedTag{ + Id: 0x11, + IfdPath: "ifd/path", + } + + if it.Is("ifd/path2", 0x11) != false { + t.Fatalf("Is is not false.") + } +} + +func TestIndexedTag_GetEncodingType_WorksWithOneType(t *testing.T) { + it := &IndexedTag{ + Id: 0xb, + Name: "some_name", + IfdPath: "ifd/path", + SupportedTypes: []exifcommon.TagTypePrimitive{ + exifcommon.TypeRational, + }, + } + + if it.GetEncodingType(nil) != exifcommon.TypeRational { + t.Fatalf("Expected the one type that was set.") + } +} + +func TestIndexedTag_GetEncodingType_FailsOnEmpty(t *testing.T) { + // This also looks for an empty to reference the first spot, invalidly. + + defer func() { + errRaw := recover() + if errRaw == nil { + t.Fatalf("Expected failure due to empty supported-types.") + } + + err := errRaw.(error) + if err.Error() != "IndexedTag [] (0) has no supported types." { + log.Panic(err) + } + }() + + it := &IndexedTag{ + SupportedTypes: []exifcommon.TagTypePrimitive{}, + } + + it.GetEncodingType(nil) +} + +func TestIndexedTag_GetEncodingType_PreferLongOverShort(t *testing.T) { + it := &IndexedTag{ + Id: 0xb, + Name: "some_name", + IfdPath: "ifd/path", + SupportedTypes: []exifcommon.TagTypePrimitive{ + exifcommon.TypeShort, + }, + } + + if it.GetEncodingType(nil) != exifcommon.TypeShort { + t.Fatalf("Expected the second (LONG) type to be returned.") + } + + it = &IndexedTag{ + Id: 0xb, + Name: "some_name", + IfdPath: "ifd/path", + SupportedTypes: []exifcommon.TagTypePrimitive{ + exifcommon.TypeShort, + exifcommon.TypeLong, + }, + } + + if it.GetEncodingType(nil) != exifcommon.TypeLong { + t.Fatalf("Expected the second (LONG) type to be returned.") + } +} + +func TestIndexedTag_GetEncodingType_BothRationalTypes(t *testing.T) { + it := &IndexedTag{ + Id: 0xb, + Name: "some_name", + IfdPath: "ifd/path", + SupportedTypes: []exifcommon.TagTypePrimitive{ + exifcommon.TypeRational, + exifcommon.TypeSignedRational, + }, + } + + v1 := exifcommon.Rational{} + + if it.GetEncodingType(v1) != exifcommon.TypeRational { + t.Fatalf("Expected the second (RATIONAL) type to be returned.") + } + + v2 := exifcommon.SignedRational{} + + if it.GetEncodingType(v2) != exifcommon.TypeSignedRational { + t.Fatalf("Expected the second (SIGNED RATIONAL) type to be returned.") + } +} + +func TestIndexedTag_DoesSupportType(t *testing.T) { + it := &IndexedTag{ + Id: 0xb, + Name: "some_name", + IfdPath: "ifd/path", + SupportedTypes: []exifcommon.TagTypePrimitive{ + exifcommon.TypeRational, + exifcommon.TypeSignedRational, + }, + } + + if it.DoesSupportType(exifcommon.TypeRational) != true { + t.Fatalf("Does not support unsigned-rational.") + } else if it.DoesSupportType(exifcommon.TypeSignedRational) != true { + t.Fatalf("Does not support signed-rational.") + } else if it.DoesSupportType(exifcommon.TypeLong) != false { + t.Fatalf("Does not support long.") + } +} + +func TestNewTagIndex(t *testing.T) { + ti := NewTagIndex() + + if ti.tagsByIfd == nil { + t.Fatalf("tagsByIfd is nil.") + } else if ti.tagsByIfdR == nil { + t.Fatalf("tagsByIfdR is nil.") + } +} + +func TestTagIndex_Add(t *testing.T) { + ti := NewTagIndex() + + if len(ti.tagsByIfd) != 0 { + t.Fatalf("tagsByIfd should be empty initially.") + } else if len(ti.tagsByIfdR) != 0 { + t.Fatalf("tagsByIfdR should be empty initially.") + } + + it := &IndexedTag{ + Id: 0xb, + Name: "some_name", + IfdPath: "ifd/path", + SupportedTypes: []exifcommon.TagTypePrimitive{ + exifcommon.TypeRational, + exifcommon.TypeSignedRational, + }, + } + + err := ti.Add(it) + log.PanicIf(err) + + if reflect.DeepEqual(ti.tagsByIfd[it.IfdPath][it.Id], it) != true { + t.Fatalf("Not present in forward lookup.") + } else if reflect.DeepEqual(ti.tagsByIfdR[it.IfdPath][it.Name], it) != true { + t.Fatalf("Not present in reverse lookup.") + } +} + +func TestTagIndex_Get(t *testing.T) { ti := NewTagIndex() it, err := ti.Get(exifcommon.IfdStandardIfdIdentity, 0x10f) @@ -19,7 +251,7 @@ func TestGet(t *testing.T) { } } -func TestGetWithName(t *testing.T) { +func TestTagIndex_GetWithName(t *testing.T) { ti := NewTagIndex() it, err := ti.GetWithName(exifcommon.IfdStandardIfdIdentity, "Make") @@ -29,3 +261,128 @@ func TestGetWithName(t *testing.T) { t.Fatalf("tag info not correct") } } + +func TestTagIndex_FindFirst_HitOnFirst(t *testing.T) { + + searchOrder := []*exifcommon.IfdIdentity{ + exifcommon.IfdExifStandardIfdIdentity, + exifcommon.IfdStandardIfdIdentity, + } + + ti := NewTagIndex() + + // ExifVersion + it, err := ti.FindFirst(0x9000, searchOrder) + log.PanicIf(err) + + if it.Is("IFD/Exif", 0x9000) != true { + t.Fatalf("Returned tag is not correct.") + } +} + +func TestTagIndex_FindFirst_HitOnSecond(t *testing.T) { + + searchOrder := []*exifcommon.IfdIdentity{ + exifcommon.IfdExifStandardIfdIdentity, + exifcommon.IfdStandardIfdIdentity, + } + + ti := NewTagIndex() + + // ProcessingSoftware + it, err := ti.FindFirst(0x000b, searchOrder) + log.PanicIf(err) + + if it.Is("IFD", 0x000b) != true { + t.Fatalf("Returned tag is not correct.") + } +} + +func TestTagIndex_FindFirst_DefaultOrder_Miss(t *testing.T) { + + searchOrder := []*exifcommon.IfdIdentity{ + exifcommon.IfdExifStandardIfdIdentity, + exifcommon.IfdStandardIfdIdentity, + } + + ti := NewTagIndex() + + _, err := ti.FindFirst(0x1234, searchOrder) + if err == nil { + t.Fatalf("Expected error for invalid tag.") + } else if err != ErrTagNotFound { + log.Panic(err) + } +} + +func TestTagIndex_FindFirst_ReverseDefaultOrder_HitOnSecond(t *testing.T) { + + reverseSearchOrder := []*exifcommon.IfdIdentity{ + exifcommon.IfdStandardIfdIdentity, + exifcommon.IfdExifStandardIfdIdentity, + } + + ti := NewTagIndex() + + // ExifVersion + it, err := ti.FindFirst(0x9000, reverseSearchOrder) + log.PanicIf(err) + + if it.Is("IFD/Exif", 0x9000) != true { + t.Fatalf("Returned tag is not correct.") + } +} + +func TestTagIndex_FindFirst_ReverseDefaultOrder_HitOnFirst(t *testing.T) { + + reverseSearchOrder := []*exifcommon.IfdIdentity{ + exifcommon.IfdStandardIfdIdentity, + exifcommon.IfdExifStandardIfdIdentity, + } + + ti := NewTagIndex() + + // ProcessingSoftware + it, err := ti.FindFirst(0x000b, reverseSearchOrder) + log.PanicIf(err) + + if it.Is("IFD", 0x000b) != true { + t.Fatalf("Returned tag is not correct.") + } +} + +func TestTagIndex_FindFirst_ReverseDefaultOrder_Miss(t *testing.T) { + + reverseSearchOrder := []*exifcommon.IfdIdentity{ + exifcommon.IfdStandardIfdIdentity, + exifcommon.IfdExifStandardIfdIdentity, + } + + ti := NewTagIndex() + + _, err := ti.FindFirst(0x1234, reverseSearchOrder) + if err == nil { + t.Fatalf("Expected error for invalid tag.") + } else if err != ErrTagNotFound { + log.Panic(err) + } +} + +func TestLoadStandardTags(t *testing.T) { + ti := NewTagIndex() + + if len(ti.tagsByIfd) != 0 { + t.Fatalf("tagsByIfd should be empty initially.") + } else if len(ti.tagsByIfdR) != 0 { + t.Fatalf("tagsByIfdR should be empty initially.") + } + + err := LoadStandardTags(ti) + log.PanicIf(err) + + if len(ti.tagsByIfd) == 0 { + t.Fatalf("tagsByIfd should be non-empty at the end.") + } else if len(ti.tagsByIfdR) == 0 { + t.Fatalf("tagsByIfdR should be non-empty at the end.") + } +} diff --git a/v2/utility.go b/v2/utility.go index e05178e..5133861 100644 --- a/v2/utility.go +++ b/v2/utility.go @@ -159,6 +159,11 @@ func GetFlatExifData(exifData []byte) (exifTags []ExifTag, err error) { // unknown tags. utilityLogger.Warningf(nil, "Tag with ID (0x%04x) in IFD [%s] is not recognized and will be ignored.", tagId, fqIfdPath) + it, err := ti.FindFirst(ite.tagId, nil) + if err == nil { + utilityLogger.Warningf(nil, "(cont'd) Tag [%s] with the same ID has been found in IFD [%s] and may be related. The tag you were looking for might have been written to the wrong IFD by a buggy implementation.", it.Name, it.IfdPath) + } + return nil }