From 9aa2497f7bded2353c0ea98e4e68ae68edea90b5 Mon Sep 17 00:00:00 2001 From: Dustin Oprea Date: Wed, 1 Jan 2020 09:05:06 -0500 Subject: [PATCH] Revert "type.go: Deinegrated `ErrUnhandledUnknownTypedTag`" This reverts commit 4f2f9044e60d16b189cd13acf41089dd83c79c0e. Reintroduces errors from `UndefinedValue()` when the undefined-type is unhandled. - We now just return it rather than panic with it so that we can check for it directly rather than use `log.Is()`. - We've updated the checks across the code accordingly. --- README.md | 10 ++++++--- exif-read-tool/main.go | 8 ++++++- exif_test.go | 8 ++++++- ifd_builder.go | 21 +++++++++---------- ifd_enumerate.go | 47 +++++++++++++++++++++++++++--------------- tags_unknown.go | 18 +++++++--------- type.go | 5 +++++ utility.go | 12 +++++++++-- 8 files changed, 83 insertions(+), 46 deletions(-) diff --git a/README.md b/README.md index 3547f8b..f41529f 100644 --- a/README.md +++ b/README.md @@ -186,9 +186,13 @@ visitor := func(fqIfdPath string, ifdIndex int, tagId uint16, tagType exif.TagTy valueString := "" if tagType.Type() == exif.TypeUndefined { value, err := exif.UndefinedValue(ifdPath, tagId, valueContext, tagType.ByteOrder()) - log.PanicIf(err) - - valueString = fmt.Sprintf("%v", value) + if log.Is(err, exif.ErrUnhandledUnknownTypedTag) { + valueString = "!UNDEFINED!" + } else if err != nil { + panic(err) + } else { + valueString = fmt.Sprintf("%v", value) + } } else { valueString, err = tagType.ResolveAsString(valueContext, true) if err != nil { diff --git a/exif-read-tool/main.go b/exif-read-tool/main.go index 074ca2d..d1400e1 100644 --- a/exif-read-tool/main.go +++ b/exif-read-tool/main.go @@ -109,7 +109,13 @@ func main() { if tagType.Type() == exif.TypeUndefined { var err error value, err = valueContext.Undefined() - log.PanicIf(err) + if err != nil { + if err == exif.ErrUnhandledUnknownTypedTag { + value = nil + } else { + log.Panic(err) + } + } valueString = fmt.Sprintf("%v", value) } else { diff --git a/exif_test.go b/exif_test.go index b4464a7..769f277 100644 --- a/exif_test.go +++ b/exif_test.go @@ -82,7 +82,13 @@ func TestVisit(t *testing.T) { valueString := "" if tagType.Type() == TypeUndefined { value, err := valueContext.Undefined() - log.PanicIf(err) + if err != nil { + if err == ErrUnhandledUnknownTypedTag { + valueString = "!UNDEFINED!" + } else { + log.Panic(err) + } + } valueString = fmt.Sprintf("%v", value) } else { diff --git a/ifd_builder.go b/ifd_builder.go index e28d67b..3cf1bac 100644 --- a/ifd_builder.go +++ b/ifd_builder.go @@ -1119,20 +1119,19 @@ func (ib *IfdBuilder) AddTagsFromExisting(ifd *Ifd, itevr *IfdTagEntryValueResol // It's an undefined-type value. Try to process, or skip if // we don't know how to. - x, err := valueContext.Undefined() - log.PanicIf(err) + undefinedInterface, err := valueContext.Undefined() + if err != nil { + if err == ErrUnhandledUnknownTypedTag { + // It's an undefined-type tag that we don't handle. If + // we don't know how to handle it, we can't know how + // many bytes it is and we must skip it. + continue + } - // TODO(dustin): !! Add test for this. - _, isUnknownUndefined := x.(TagUnknownType_UnknownValue) - - if isUnknownUndefined == true { - // It's an undefined-type tag that we don't handle. If we - // don't know how to handle it, we can't know how many bytes - // it is and we must skip it. - continue + log.Panic(err) } - undefined, ok := x.(UnknownTagValue) + undefined, ok := undefinedInterface.(UnknownTagValue) if ok != true { log.Panicf("unexpected value returned from undefined-value processor") } diff --git a/ifd_enumerate.go b/ifd_enumerate.go index 82eef7d..317e847 100644 --- a/ifd_enumerate.go +++ b/ifd_enumerate.go @@ -240,24 +240,31 @@ func (ie *IfdEnumerate) resolveTagValue(ite *IfdTagEntry) (valueBytes []byte, is valueContext := ie.GetValueContext(ite) value, err := valueContext.Undefined() - log.PanicIf(err) + if err != nil { + if err == ErrUnhandledUnknownTypedTag { + valueBytes = []byte(UnparseableUnknownTagValuePlaceholder) + return valueBytes, true, nil + } - switch value.(type) { - case []byte: - return value.([]byte), false, nil - case TagUnknownType_UnknownValue: - b := []byte(value.(TagUnknownType_UnknownValue)) - return b, false, nil - case string: - return []byte(value.(string)), false, nil - case UnknownTagValue: - valueBytes, err := value.(UnknownTagValue).ValueBytes() - log.PanicIf(err) + log.Panic(err) + } else { + switch value.(type) { + case []byte: + return value.([]byte), false, nil + case TagUnknownType_UnknownValue: + b := []byte(value.(TagUnknownType_UnknownValue)) + return b, false, nil + case string: + return []byte(value.(string)), false, nil + case UnknownTagValue: + valueBytes, err := value.(UnknownTagValue).ValueBytes() + log.PanicIf(err) - return valueBytes, false, nil - default: - // TODO(dustin): !! Finish translating the rest of the types (make reusable and replace into other similar implementations?) - log.Panicf("can not produce bytes for unknown-type tag (0x%04x) (1): [%s]", ite.TagId, reflect.TypeOf(value)) + return valueBytes, false, nil + default: + // TODO(dustin): !! Finish translating the rest of the types (make reusable and replace into other similar implementations?) + log.Panicf("can not produce bytes for unknown-type tag (0x%04x) (1): [%s]", ite.TagId, reflect.TypeOf(value)) + } } } else { originalType := NewTagType(ite.TagType, ie.byteOrder) @@ -709,7 +716,13 @@ func (ifd *Ifd) printTagTree(populateValues bool, index, level int, nextLink boo var err error value, err = ifd.TagValue(tag) - log.PanicIf(err) + if err != nil { + if err == ErrUnhandledUnknownTypedTag { + value = UnparseableUnknownTagValuePlaceholder + } else { + log.Panic(err) + } + } } fmt.Printf("%s - TAG: %s NAME=[%s] VALUE=[%v]\n", indent, tag, tagName, value) diff --git a/tags_unknown.go b/tags_unknown.go index bf09ebe..a8ac3f9 100644 --- a/tags_unknown.go +++ b/tags_unknown.go @@ -11,6 +11,10 @@ import ( "github.com/dsoprea/go-logging" ) +const ( + UnparseableUnknownTagValuePlaceholder = "!UNKNOWN" +) + const ( TagUnknownType_9298_UserComment_Encoding_ASCII = iota TagUnknownType_9298_UserComment_Encoding_JIS = iota @@ -387,15 +391,7 @@ func UndefinedValue(ifdPath string, tagId uint16, valueContext interface{}, byte // // 0xa40b is device-specific and unhandled. - // Return encapsulated data rather than an error so that we can at least - // print/profile the opaque data. - - // TODO(dustin): This won't ever work. The unit-count isn't necessarily correct. Revert to returning an error. - valueContextPtr.SetUnknownValueType(TypeByte) - - valueBytes, err := valueContextPtr.ReadBytes() - log.PanicIf(err) - - tutuv := TagUnknownType_UnknownValue(valueBytes) - return tutuv, nil + // We have no choice but to return the error. We have no way of knowing how + // much data there is without already knowing what data-type this tag is. + return nil, ErrUnhandledUnknownTypedTag } diff --git a/type.go b/type.go index 0782c6f..0d635b7 100644 --- a/type.go +++ b/type.go @@ -84,6 +84,11 @@ var ( // ErrWrongType is used when we try to parse anything other than the // current type. ErrWrongType = errors.New("wrong type, can not parse") + + // ErrUnhandledUnknownTag is used when we try to parse a tag that's + // recorded as an "unknown" type but not a documented tag (therefore + // leaving us not knowning how to read it). + ErrUnhandledUnknownTypedTag = errors.New("not a standard unknown-typed tag") ) type Rational struct { diff --git a/utility.go b/utility.go index 677910f..d7f3006 100644 --- a/utility.go +++ b/utility.go @@ -182,10 +182,18 @@ func GetFlatExifData(exifData []byte) (exifTags []ExifTag, err error) { } value, err := ifd.TagValue(ite) - log.PanicIf(err) + if err != nil { + if err == ErrUnhandledUnknownTypedTag { + value = UnparseableUnknownTagValuePlaceholder + } else { + log.Panic(err) + } + } valueBytes, err := ifd.TagValueBytes(ite) - log.PanicIf(err) + if err != nil && err != ErrUnhandledUnknownTypedTag { + log.Panic(err) + } et := ExifTag{ IfdPath: ifd.IfdPath,