tags.go: Bugfix for bad FindFirst() error handling and not returning if not found

- utility.go: Add GetExifData() to return flat tags and extra data and
  to allow guessing of tags for misplaced IFDs.

- exif-read-tool/main.go: Implement tag guessing

v2/common/type.go

@@ -130,13 +130,21 @@ var (
     typeNamesR = map[string]TagTypePrimitive{}
 )
 
+// Rational describes an unsigned rational value.
 type Rational struct {
+    // Numerator is the numerator of the rational value.
     Numerator uint32
+
+    // Denominator is the numerator of the rational value.
     Denominator uint32
 }
 
+// SignedRational describes a signed rational value.
 type SignedRational struct {
+    // Numerator is the numerator of the rational value.
     Numerator int32
+
+    // Denominator is the numerator of the rational value.
     Denominator int32
 }
 
@@ -425,6 +433,18 @@ func GetTypeByName(typeName string) (tagType TagTypePrimitive, found bool) {
     return tagType, found
 }
 
+// BasicTag describes a single tag for any purpose.
+type BasicTag struct {
+    // FqIfdPath is the fully-qualified IFD-path.
+    FqIfdPath string
+
+    // IfdPath is the unindexed IFD-path.
+    IfdPath string
+
+    // TagId is the tag-ID.
+    TagId uint16
+}
+
 func init() {
     for typeId, typeName := range TypeNames {
         typeNamesR[typeName] = typeId

v2/exif-read-tool/main.go

@@ -105,7 +105,7 @@ func main() {
 
 	// Run the parse.
 
-	entries, err := exif.GetFlatExifData(rawExif)
+	entries, _, err := exif.GetExifData(rawExif, true)
 	log.PanicIf(err)
 
 	// Write the thumbnail is requested and present.

v2/tags.go

@@ -272,7 +272,7 @@ var (
 // 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) {
+func (ti *TagIndex) FindFirst(id uint16, typeId exifcommon.TagTypePrimitive, ifdIdentities []*exifcommon.IfdIdentity) (it *IndexedTag, err error) {
 	defer func() {
 		if state := recover(); state != nil {
 			err = log.Wrap(state.(error))
@@ -293,8 +293,16 @@ func (ti *TagIndex) FindFirst(id uint16, ifdIdentities []*exifcommon.IfdIdentity
 			log.Panic(err)
 		}
 
+		// Even though the tag might be mislocated, the type should still be the
+		// same. Check this so we don't accidentally end-up on a complete
+		// irrelevant tag with a totally different data type. This attempts to
+		// mitigate producing garbage.
+		for _, supportedType := range it.SupportedTypes {
+			if supportedType == typeId {
 				return it, nil
 			}
+		}
+	}
 
 	return nil, ErrTagNotFound
 }

v2/tags_test.go

@@ -272,7 +272,7 @@ func TestTagIndex_FindFirst_HitOnFirst(t *testing.T) {
 	ti := NewTagIndex()
 
 	// ExifVersion
-	it, err := ti.FindFirst(0x9000, searchOrder)
+	it, err := ti.FindFirst(0x9000, exifcommon.TypeUndefined, searchOrder)
 	log.PanicIf(err)
 
 	if it.Is("IFD/Exif", 0x9000) != true {
@@ -290,7 +290,7 @@ func TestTagIndex_FindFirst_HitOnSecond(t *testing.T) {
 	ti := NewTagIndex()
 
 	// ProcessingSoftware
-	it, err := ti.FindFirst(0x000b, searchOrder)
+	it, err := ti.FindFirst(0x000b, exifcommon.TypeAscii, searchOrder)
 	log.PanicIf(err)
 
 	if it.Is("IFD", 0x000b) != true {
@@ -307,7 +307,7 @@ func TestTagIndex_FindFirst_DefaultOrder_Miss(t *testing.T) {
 
 	ti := NewTagIndex()
 
-	_, err := ti.FindFirst(0x1234, searchOrder)
+	_, err := ti.FindFirst(0x1234, exifcommon.TypeRational, searchOrder)
 	if err == nil {
 		t.Fatalf("Expected error for invalid tag.")
 	} else if err != ErrTagNotFound {
@@ -325,7 +325,7 @@ func TestTagIndex_FindFirst_ReverseDefaultOrder_HitOnSecond(t *testing.T) {
 	ti := NewTagIndex()
 
 	// ExifVersion
-	it, err := ti.FindFirst(0x9000, reverseSearchOrder)
+	it, err := ti.FindFirst(0x9000, exifcommon.TypeUndefined, reverseSearchOrder)
 	log.PanicIf(err)
 
 	if it.Is("IFD/Exif", 0x9000) != true {
@@ -343,7 +343,7 @@ func TestTagIndex_FindFirst_ReverseDefaultOrder_HitOnFirst(t *testing.T) {
 	ti := NewTagIndex()
 
 	// ProcessingSoftware
-	it, err := ti.FindFirst(0x000b, reverseSearchOrder)
+	it, err := ti.FindFirst(0x000b, exifcommon.TypeAscii, reverseSearchOrder)
 	log.PanicIf(err)
 
 	if it.Is("IFD", 0x000b) != true {
@@ -360,7 +360,7 @@ func TestTagIndex_FindFirst_ReverseDefaultOrder_Miss(t *testing.T) {
 
 	ti := NewTagIndex()
 
-	_, err := ti.FindFirst(0x1234, reverseSearchOrder)
+	_, err := ti.FindFirst(0x1234, exifcommon.TypeRational, reverseSearchOrder)
 	if err == nil {
 		t.Fatalf("Expected error for invalid tag.")
 	} else if err != ErrTagNotFound {

v2/utility.go

@@ -116,10 +116,20 @@ type ExifTag struct {
 
 // String returns a string representation.
 func (et ExifTag) String() string {
-    return fmt.Sprintf("ExifTag<IFD-PATH=[%s] TAG-ID=(0x%02x) TAG-NAME=[%s] TAG-TYPE=[%s] VALUE=[%v] VALUE-BYTES=(%d) CHILD-IFD-PATH=[%s]", et.IfdPath, et.TagId, et.TagName, et.TagTypeName, et.FormattedFirst, len(et.ValueBytes), et.ChildIfdPath)
+    return fmt.Sprintf(
+        "ExifTag<"+
+            "IFD-PATH=[%s] "+
+            "TAG-ID=(0x%02x) "+
+            "TAG-NAME=[%s] "+
+            "TAG-TYPE=[%s] "+
+            "VALUE=[%v] "+
+            "VALUE-BYTES=(%d) "+
+            "CHILD-IFD-PATH=[%s]",
+        et.IfdPath, et.TagId, et.TagName, et.TagTypeName, et.FormattedFirst,
+        len(et.ValueBytes), et.ChildIfdPath)
 }
 
-// RELEASE(dustin): In the next release, make this return a list of skipped tags, too.
+// RELEASE(dustin): In the next release, dump GetFlatExifData().
 
 // GetFlatExifData returns a simple, flat representation of all tags.
 func GetFlatExifData(exifData []byte) (exifTags []ExifTag, err error) {
@@ -129,6 +139,33 @@ func GetFlatExifData(exifData []byte) (exifTags []ExifTag, err error) {
         }
     }()
 
+    exifTags, _, err = GetExifData(exifData, false)
+    log.PanicIf(err)
+
+    return exifTags, nil
+}
+
+// MiscellaneousExifData is reports additional data collected during the parse.
+type MiscellaneousExifData struct {
+    // UnknownTags contains all tags that were invalid for their containing
+    // IFDs. The values represent alternative IFDs that were correctly matched
+    // to those tags and used instead.
+    UnknownTags map[exifcommon.BasicTag]exifcommon.BasicTag
+}
+
+// GetExifData returns a simple, flat representation of all tags along with
+// miscellaneous metadata..
+//
+// `doFindAlternativeIfds` indicates that we can attempt to find unknown tags in
+// other IFDs. This can help with malformed EXIF data, where tags might be in
+// IFDs other than specified by the EXIF specification.
+func GetExifData(exifData []byte, doFindAlternativeIfds bool) (exifTags []ExifTag, med MiscellaneousExifData, err error) {
+    defer func() {
+        if state := recover(); state != nil {
+            err = log.Wrap(state.(error))
+        }
+    }()
+
     eh, err := ParseExifHeader(exifData)
     log.PanicIf(err)
 
@@ -139,6 +176,8 @@ func GetFlatExifData(exifData []byte) (exifTags []ExifTag, err error) {
 
     exifTags = make([]ExifTag, 0)
 
+    med.UnknownTags = make(map[exifcommon.BasicTag]exifcommon.BasicTag)
+
     visitor := func(fqIfdPath string, ifdIndex int, ite *IfdTagEntry) (err error) {
         tagId := ite.TagId()
         ii := ite.ifdIdentity
@@ -151,20 +190,56 @@ func GetFlatExifData(exifData []byte) (exifTags []ExifTag, err error) {
 
             // This is an unknown tag.
 
+            originalBt := exifcommon.BasicTag{
+                FqIfdPath: ii.String(),
+                IfdPath:   ii.UnindexedString(),
+                TagId:     ite.tagId,
+            }
+
+            med.UnknownTags[originalBt] = exifcommon.BasicTag{}
+
+            var err2 error
+
+            it, err2 = ti.FindFirst(ite.tagId, ite.tagType, nil)
+            if err2 == nil {
+                utilityLogger.Warningf(nil,
+                    "Tag with ID (0x%04x) is not valid for IFD [%s], but it "+
+                        "*is* valid as tag [%s] under IFD [%s] and has the same "+
+                        "type [%s]. This EXIF blob was probably written by a buggy "+
+                        "implementation.",
+                    ite.tagId, ii.UnindexedString(), it.Name, it.IfdPath,
+                    ite.tagType)
+
+                if doFindAlternativeIfds == false {
+                    utilityLogger.Warningf(nil,
+                        "We were told to not acknolwedge the alternative IFD "+
+                            "for the misplaced tag. Skipping.")
+
+                    return nil
+                }
+
+                med.UnknownTags[originalBt] = exifcommon.BasicTag{
+                    IfdPath: it.IfdPath,
+                    TagId:   ite.tagId,
+                }
+            } else if err2 == ErrTagNotFound {
                 // This is supposed to be a convenience function and if we were
                 // to keep the name empty or set it to some placeholder, it
                 // might be mismanaged by the package that is calling us. If
                 // they want to specifically manage these types of tags, they
                 // can use more advanced functionality to specifically -handle
                 // unknown tags.
-            utilityLogger.Warningf(nil, "Tag with ID (0x%04x) in IFD [%s] is not recognized and will be ignored.", tagId, fqIfdPath)
+                utilityLogger.Warningf(nil,
-
+                    "Tag with ID (0x%04x) in IFD [%s] is not recognized and "+
-            it, err := ti.FindFirst(ite.tagId, nil)
+                        "will be ignored.", tagId, fqIfdPath)
-            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
+            } else {
+                log.Panic(err2)
+            }
+
+            // If we get here, we found the tag in a different place than
+            // prescribed in the standard.
         }
 
         tagName := it.Name
@@ -217,7 +292,7 @@ func GetFlatExifData(exifData []byte) (exifTags []ExifTag, err error) {
     err = ie.Scan(exifcommon.IfdStandardIfdIdentity, eh.FirstIfdOffset, visitor)
     log.PanicIf(err)
 
-    return exifTags, nil
+    return exifTags, med, nil
 }
 
 func GpsDegreesEquals(gi1, gi2 GpsDegrees) bool {