From 7745cfe74c914e4c38b88d5cc1dfb702b31b8b2e Mon Sep 17 00:00:00 2001 From: Dustin Oprea Date: Wed, 6 Jun 2018 04:37:43 -0400 Subject: [PATCH] exif: Deimplemented IsExif(). - exif - Began deconstructing Exif type in favor of just defining those methods as functions. - Added additional tests. --- README.md | 4 +- exif-read-tool/main.go | 18 ++----- exif.go | 78 +++++++++++++++--------------- exif_test.go | 97 +++++++++++++++++++++----------------- ifd_builder_encode_test.go | 12 ++--- ifd_builder_test.go | 34 +++++-------- ifd_enumerate.go | 5 -- ifd_enumerate_test.go | 54 +++++++-------------- ifd_tag_entry.go | 5 -- 9 files changed, 132 insertions(+), 175 deletions(-) diff --git a/README.md b/README.md index e3cf724..9b78fd1 100644 --- a/README.md +++ b/README.md @@ -141,8 +141,6 @@ log.PanicIf(err) data, err := ioutil.ReadAll(f) log.PanicIf(err) -e := exif.NewExif() - foundAt := -1 for i := 0; i < len(data); i++ { if e.IsExif(data[i:i + 6]) == true { @@ -191,7 +189,7 @@ visitor := func(ii exif.IfdIdentity, ifdIndex int, tagId uint16, tagType exif.Ta return nil } -err = e.Visit(data[foundAt:], visitor) +err = Visit(data[foundAt:], visitor) log.PanicIf(err) ``` diff --git a/exif-read-tool/main.go b/exif-read-tool/main.go index 2e6d3f0..9270117 100644 --- a/exif-read-tool/main.go +++ b/exif-read-tool/main.go @@ -68,20 +68,8 @@ func main() { data, err := ioutil.ReadAll(f) log.PanicIf(err) - e := exif.NewExif() - - foundAt := -1 - for i := 0; i < len(data); i++ { - if exif.IsExif(data[i:i + 6]) == true { - foundAt = i - break - } - } - - if foundAt == -1 { - fmt.Printf("EXIF data not found.\n") - os.Exit(-1) - } + rawExif, err := exif.SearchAndExtractExif(data) + log.PanicIf(err) // Run the parse. @@ -143,7 +131,7 @@ func main() { return nil } - _, err = e.Visit(data[foundAt:], visitor) + _, err = exif.Visit(rawExif, visitor) log.PanicIf(err) if printAsJsonArg == true { diff --git a/exif.go b/exif.go index a36d578..5f2760d 100644 --- a/exif.go +++ b/exif.go @@ -52,28 +52,42 @@ var ( ErrExifHeaderError = errors.New("exif header error") ) -// TODO(dustin): !! Remove all usage of this in favor of ParseExifHeader. -func IsExif(data []byte) (ok bool) { - if bytes.Compare(data[:6], ExifHeaderPrefixBytes) == 0 { - return true + +// SearchAndExtractExif returns a slice from the beginning of the EXIF data the +// end of the file (it's not practical to try and calculate where the data +// actually ends). +func SearchAndExtractExif(data []byte) (rawExif []byte, err error) { + defer func() { + if state := recover(); state != nil { + err := log.Wrap(state.(error)) + log.Panic(err) + } + }() + + // Search for the beginning of the EXIF information. The EXIF is near the + // beginning of our/most JPEGs, so this has a very low cost. + + foundAt := -1 + for i := 0; i < len(data); i++ { + if _, err := ParseExifHeader(data[i:]); err == nil { + foundAt = i + break + } else if log.Is(err, ErrNotExif) == false { + log.Panic(err) + } } - return false + if foundAt == -1 { + log.Panicf("EXIF start not found") + } + + return data[foundAt:], nil } - -// TODO(dustin): Isolated this to its own packge breakout the methods as independent function. There's no use for a dedicated struct. - - -type Exif struct { - -} - -func NewExif() *Exif { - return new(Exif) -} - -func (e *Exif) SearchAndExtractExif(filepath string) (rawExif []byte, err error) { +// SearchFileAndExtractExif returns a slice from the beginning of the EXIF data +// to the end of the file (it's not practical to try and calculate where the +// data actually ends). +func SearchFileAndExtractExif(filepath string) (rawExif []byte, err error) { defer func() { if state := recover(); state != nil { err := log.Wrap(state.(error)) @@ -91,22 +105,10 @@ func (e *Exif) SearchAndExtractExif(filepath string) (rawExif []byte, err error) data, err := ioutil.ReadAll(f) log.PanicIf(err) - // Search for the beginning of the EXIF information. The EXIF is near the - // beginning of our/most JPEGs, so this has a very low cost. + rawExif, err = SearchAndExtractExif(data) + log.PanicIf(err) - foundAt := -1 - for i := 0; i < len(data); i++ { - if IsExif(data[i:i + 6]) == true { - foundAt = i - break - } - } - - if foundAt == -1 { - log.Panicf("EXIF start not found") - } - - return data[foundAt:], nil + return rawExif, nil } @@ -123,7 +125,7 @@ func (eh ExifHeader) String() string { // // This will panic with ErrNotExif on any data errors so that we can double as // an EXIF-detection routine. -func (e *Exif) ParseExifHeader(data []byte) (eh ExifHeader, err error) { +func ParseExifHeader(data []byte) (eh ExifHeader, err error) { defer func() { if state := recover(); state != nil { err = log.Wrap(state.(error)) @@ -169,14 +171,14 @@ func (e *Exif) ParseExifHeader(data []byte) (eh ExifHeader, err error) { } // Visit recursively invokes a callback for every tag. -func (e *Exif) Visit(exifData []byte, visitor TagVisitor) (eh ExifHeader, err error) { +func Visit(exifData []byte, visitor TagVisitor) (eh ExifHeader, err error) { defer func() { if state := recover(); state != nil { err = log.Wrap(state.(error)) } }() - eh, err = e.ParseExifHeader(exifData) + eh, err = ParseExifHeader(exifData) log.PanicIf(err) ie := NewIfdEnumerate(exifData, eh.ByteOrder) @@ -188,14 +190,14 @@ func (e *Exif) Visit(exifData []byte, visitor TagVisitor) (eh ExifHeader, err er } // Collect recursively builds a static structure of all IFDs and tags. -func (e *Exif) Collect(exifData []byte) (eh ExifHeader, index IfdIndex, err error) { +func Collect(exifData []byte) (eh ExifHeader, index IfdIndex, err error) { defer func() { if state := recover(); state != nil { err = log.Wrap(state.(error)) } }() - eh, err = e.ParseExifHeader(exifData) + eh, err = ParseExifHeader(exifData) log.PanicIf(err) ie := NewIfdEnumerate(exifData, eh.ByteOrder) diff --git a/exif_test.go b/exif_test.go index bfd7f0a..28fd253 100644 --- a/exif_test.go +++ b/exif_test.go @@ -6,6 +6,7 @@ import ( "path" "fmt" "reflect" + "bytes" "io/ioutil" "encoding/binary" @@ -15,22 +16,11 @@ import ( var ( assetsPath = "" + testExifData = make([]byte, 0) ) -func TestIsExif_True(t *testing.T) { - if ok := IsExif([]byte("Exif\000\000")); ok != true { - t.Fatalf("expected true") - } -} - -func TestIsExif_False(t *testing.T) { - if ok := IsExif([]byte("something unexpected")); ok != false { - t.Fatalf("expected false") - } -} - -func TestExif_Visit(t *testing.T) { +func TestVisit(t *testing.T) { defer func() { if state := recover(); state != nil { err := log.Wrap(state.(error)) @@ -52,13 +42,13 @@ func TestExif_Visit(t *testing.T) { // Search for the beginning of the EXIF information. The EXIF is near the // very beginning of our/most JPEGs, so this has a very low cost. - e := NewExif() - foundAt := -1 for i := 0; i < len(data); i++ { - if IsExif(data[i:i + 6]) == true { + if _, err := ParseExifHeader(data[i:]); err == nil { foundAt = i break + } else if log.Is(err, ErrNotExif) == false { + log.Panic(err) } } @@ -110,7 +100,7 @@ func TestExif_Visit(t *testing.T) { return nil } - _, err = e.Visit(data[foundAt:], visitor) + _, err = Visit(data[foundAt:], visitor) log.PanicIf(err) // for _, line := range tags { @@ -200,7 +190,35 @@ func TestExif_Visit(t *testing.T) { } } -func TestExif_Collect(t *testing.T) { +func TestSearchFileAndExtractExif(t *testing.T) { + filepath := path.Join(assetsPath, "NDM_8901.jpg") + + // Returns a slice starting with the EXIF data and going to the end of the + // image. + rawExif, err := SearchFileAndExtractExif(filepath) + log.PanicIf(err) + + if bytes.Compare(rawExif[:len(testExifData)], testExifData) != 0 { + t.Fatalf("found EXIF data not correct") + } +} + +func TestSearchAndExtractExif(t *testing.T) { + filepath := path.Join(assetsPath, "NDM_8901.jpg") + + imageData, err := ioutil.ReadFile(filepath) + log.PanicIf(err) + + + rawExif, err := SearchAndExtractExif(imageData) + log.PanicIf(err) + + if bytes.Compare(rawExif[:len(testExifData)], testExifData) != 0 { + t.Fatalf("found EXIF data not correct") + } +} + +func TestCollect(t *testing.T) { defer func() { if state := recover(); state != nil { err := log.Wrap(state.(error)) @@ -208,14 +226,12 @@ func TestExif_Collect(t *testing.T) { } }() - e := NewExif() - filepath := path.Join(assetsPath, "NDM_8901.jpg") - rawExif, err := e.SearchAndExtractExif(filepath) + rawExif, err := SearchFileAndExtractExif(filepath) log.PanicIf(err) - _, index, err := e.Collect(rawExif) + _, index, err := Collect(rawExif) log.PanicIf(err) rootIfd := index.RootIfd @@ -336,20 +352,8 @@ func TestExif_Collect(t *testing.T) { } } -func TestExif_ParseExifHeader(t *testing.T) { - filepath := path.Join(assetsPath, "NDM_8901.jpg.exif") - - exifData, err := ioutil.ReadFile(filepath) - log.PanicIf(err) - -// TODO(dustin): !! We're currently built to expect the JPEG EXIF header-prefix, but our test-data doesn't have that.So, artificially prefix it, for now. - data := make([]byte, len(exifData) + len(ExifHeaderPrefixBytes)) - copy(data[0:], ExifHeaderPrefixBytes) - copy(data[len(ExifHeaderPrefixBytes):], exifData) - - e := NewExif() - - eh, err := e.ParseExifHeader(data) +func TestParseExifHeader(t *testing.T) { + eh, err := ParseExifHeader(testExifData) log.PanicIf(err) if eh.ByteOrder != binary.LittleEndian { @@ -363,9 +367,7 @@ func TestExif_BuildAndParseExifHeader(t *testing.T) { headerBytes, err := BuildExifHeader(TestDefaultByteOrder, 0x11223344) log.PanicIf(err) - e := NewExif() - - eh, err := e.ParseExifHeader(headerBytes) + eh, err := ParseExifHeader(headerBytes) log.PanicIf(err) if eh.ByteOrder != TestDefaultByteOrder { @@ -379,9 +381,7 @@ func ExampleBuildExifHeader() { headerBytes, err := BuildExifHeader(TestDefaultByteOrder, 0x11223344) log.PanicIf(err) - e := NewExif() - - eh, err := e.ParseExifHeader(headerBytes) + eh, err := ParseExifHeader(headerBytes) log.PanicIf(err) fmt.Printf("%v\n", eh) @@ -395,4 +395,17 @@ func init() { } assetsPath = path.Join(goPath, "src", "github.com", "dsoprea", "go-exif", "assets") + + + // Load test EXIF data. + + filepath := path.Join(assetsPath, "NDM_8901.jpg.exif") + + exifData, err := ioutil.ReadFile(filepath) + log.PanicIf(err) + +// TODO(dustin): !! We're currently built to expect the JPEG EXIF header-prefix, but our test-data doesn't have that.So, artificially prefix it, for now. + testExifData = make([]byte, len(exifData) + len(ExifHeaderPrefixBytes)) + copy(testExifData[0:], ExifHeaderPrefixBytes) + copy(testExifData[len(ExifHeaderPrefixBytes):], exifData) } diff --git a/ifd_builder_encode_test.go b/ifd_builder_encode_test.go index add5b3e..09dbb7a 100644 --- a/ifd_builder_encode_test.go +++ b/ifd_builder_encode_test.go @@ -516,9 +516,7 @@ func getExifSimpleTestIb() *IfdBuilder { } func validateExifSimpleTestIb(exifData []byte, t *testing.T) { - e := NewExif() - - eh, index, err := e.Collect(exifData) + eh, index, err := Collect(exifData) log.PanicIf(err) if eh.ByteOrder != TestDefaultByteOrder { @@ -818,9 +816,7 @@ func Test_IfdByteEncoder_EncodeToExif_WithChildAndSibling(t *testing.T) { // Parse. - e := NewExif() - - _, index, err := e.Collect(exifData) + _, index, err := Collect(exifData) log.PanicIf(err) tagsDump := index.RootIfd.DumpTree() @@ -900,9 +896,7 @@ func ExampleIfdByteEncoder_EncodeToExif() { // Parse it so we can see it. - e := NewExif() - - _, index, err := e.Collect(exifData) + _, index, err := Collect(exifData) log.PanicIf(err) // addressableData is the byte-slice where the allocated data can be diff --git a/ifd_builder_test.go b/ifd_builder_test.go index 0893a08..d7d3c3f 100644 --- a/ifd_builder_test.go +++ b/ifd_builder_test.go @@ -1211,14 +1211,12 @@ func Test_IfdBuilder_CreateIfdBuilderFromExistingChain(t *testing.T) { } }() - e := NewExif() - filepath := path.Join(assetsPath, "NDM_8901.jpg") - rawExif, err := e.SearchAndExtractExif(filepath) + rawExif, err := SearchFileAndExtractExif(filepath) log.PanicIf(err) - _, index, err := e.Collect(rawExif) + _, index, err := Collect(rawExif) log.PanicIf(err) itevr := NewIfdTagEntryValueResolver(rawExif, index.RootIfd.ByteOrder) @@ -1293,14 +1291,12 @@ func Test_IfdBuilder_CreateIfdBuilderFromExistingChain(t *testing.T) { func Test_IfdBuilder_CreateIfdBuilderFromExistingChain_RealData(t *testing.T) { filepath := path.Join(assetsPath, "NDM_8901.jpg") - e := NewExif() - - rawExif, err := e.SearchAndExtractExif(filepath) + rawExif, err := SearchFileAndExtractExif(filepath) log.PanicIf(err) // Decode from binary. - _, originalIndex, err := e.Collect(rawExif) + _, originalIndex, err := Collect(rawExif) log.PanicIf(err) originalThumbnailData, err := originalIndex.RootIfd.NextIfd.Thumbnail() @@ -1320,7 +1316,7 @@ func Test_IfdBuilder_CreateIfdBuilderFromExistingChain_RealData(t *testing.T) { // Parse again. - _, recoveredIndex, err := e.Collect(updatedExif) + _, recoveredIndex, err := Collect(updatedExif) log.PanicIf(err) recoveredTags := recoveredIndex.RootIfd.DumpTags() @@ -1407,14 +1403,12 @@ func Test_IfdBuilder_CreateIfdBuilderFromExistingChain_RealData(t *testing.T) { func Test_IfdBuilder_CreateIfdBuilderFromExistingChain_RealData_WithUpdate(t *testing.T) { filepath := path.Join(assetsPath, "NDM_8901.jpg") - e := NewExif() - - rawExif, err := e.SearchAndExtractExif(filepath) + rawExif, err := SearchFileAndExtractExif(filepath) log.PanicIf(err) // Decode from binary. - _, originalIndex, err := e.Collect(rawExif) + _, originalIndex, err := Collect(rawExif) log.PanicIf(err) originalThumbnailData, err := originalIndex.RootIfd.NextIfd.Thumbnail() @@ -1452,7 +1446,7 @@ func Test_IfdBuilder_CreateIfdBuilderFromExistingChain_RealData_WithUpdate(t *te // Parse again. - _, recoveredIndex, err := e.Collect(updatedExif) + _, recoveredIndex, err := Collect(updatedExif) log.PanicIf(err) recoveredTags := recoveredIndex.RootIfd.DumpTags() @@ -1547,12 +1541,10 @@ func Test_IfdBuilder_CreateIfdBuilderFromExistingChain_RealData_WithUpdate(t *te func ExampleIfd_Thumbnail() { filepath := path.Join(assetsPath, "NDM_8901.jpg") - e := NewExif() - - rawExif, err := e.SearchAndExtractExif(filepath) + rawExif, err := SearchFileAndExtractExif(filepath) log.PanicIf(err) - _, index, err := e.Collect(rawExif) + _, index, err := Collect(rawExif) log.PanicIf(err) thumbnailData, err := index.RootIfd.NextIfd.Thumbnail() @@ -1565,12 +1557,10 @@ func ExampleIfd_Thumbnail() { func ExampleBuilderTag_SetValue() { filepath := path.Join(assetsPath, "NDM_8901.jpg") - e := NewExif() - - rawExif, err := e.SearchAndExtractExif(filepath) + rawExif, err := SearchFileAndExtractExif(filepath) log.PanicIf(err) - _, index, err := e.Collect(rawExif) + _, index, err := Collect(rawExif) log.PanicIf(err) // Create builder. diff --git a/ifd_enumerate.go b/ifd_enumerate.go index 9256b07..e33efeb 100644 --- a/ifd_enumerate.go +++ b/ifd_enumerate.go @@ -99,11 +99,6 @@ type IfdEnumerate struct { } func NewIfdEnumerate(exifData []byte, byteOrder binary.ByteOrder) *IfdEnumerate { - // Make it obvious what data we expect and when we don't get it. - if IsExif(exifData) == false { - log.Panicf("not exif data") - } - return &IfdEnumerate{ exifData: exifData, buffer: bytes.NewBuffer(exifData), diff --git a/ifd_enumerate_test.go b/ifd_enumerate_test.go index 79ad734..b221b27 100644 --- a/ifd_enumerate_test.go +++ b/ifd_enumerate_test.go @@ -46,14 +46,12 @@ func TestIfdTagEntry_ValueBytes_RealData(t *testing.T) { } }() - e := NewExif() - filepath := path.Join(assetsPath, "NDM_8901.jpg") - rawExif, err := e.SearchAndExtractExif(filepath) + rawExif, err := SearchFileAndExtractExif(filepath) log.PanicIf(err) - eh, index, err := e.Collect(rawExif) + eh, index, err := Collect(rawExif) log.PanicIf(err) var ite *IfdTagEntry @@ -90,14 +88,12 @@ func TestIfdTagEntry_Resolver_ValueBytes(t *testing.T) { } }() - e := NewExif() - filepath := path.Join(assetsPath, "NDM_8901.jpg") - rawExif, err := e.SearchAndExtractExif(filepath) + rawExif, err := SearchFileAndExtractExif(filepath) log.PanicIf(err) - eh, index, err := e.Collect(rawExif) + eh, index, err := Collect(rawExif) log.PanicIf(err) var ite *IfdTagEntry @@ -135,14 +131,12 @@ func TestIfdTagEntry_Resolver_ValueBytes__Unknown_Field_And_Nonroot_Ifd(t *testi } }() - e := NewExif() - filepath := path.Join(assetsPath, "NDM_8901.jpg") - rawExif, err := e.SearchAndExtractExif(filepath) + rawExif, err := SearchFileAndExtractExif(filepath) log.PanicIf(err) - eh, index, err := e.Collect(rawExif) + eh, index, err := Collect(rawExif) log.PanicIf(err) ii, _ := IfdIdOrFail(IfdStandard, IfdExif) @@ -177,12 +171,10 @@ func TestIfdTagEntry_Resolver_ValueBytes__Unknown_Field_And_Nonroot_Ifd(t *testi func Test_Ifd_FindTagWithId_Hit(t *testing.T) { filepath := path.Join(assetsPath, "NDM_8901.jpg") - e := NewExif() - - rawExif, err := e.SearchAndExtractExif(filepath) + rawExif, err := SearchFileAndExtractExif(filepath) log.PanicIf(err) - _, index, err := e.Collect(rawExif) + _, index, err := Collect(rawExif) log.PanicIf(err) ifd := index.RootIfd @@ -198,12 +190,10 @@ func Test_Ifd_FindTagWithId_Hit(t *testing.T) { func Test_Ifd_FindTagWithId_Miss(t *testing.T) { filepath := path.Join(assetsPath, "NDM_8901.jpg") - e := NewExif() - - rawExif, err := e.SearchAndExtractExif(filepath) + rawExif, err := SearchFileAndExtractExif(filepath) log.PanicIf(err) - _, index, err := e.Collect(rawExif) + _, index, err := Collect(rawExif) log.PanicIf(err) ifd := index.RootIfd @@ -219,12 +209,10 @@ func Test_Ifd_FindTagWithId_Miss(t *testing.T) { func Test_Ifd_FindTagWithName_Hit(t *testing.T) { filepath := path.Join(assetsPath, "NDM_8901.jpg") - e := NewExif() - - rawExif, err := e.SearchAndExtractExif(filepath) + rawExif, err := SearchFileAndExtractExif(filepath) log.PanicIf(err) - _, index, err := e.Collect(rawExif) + _, index, err := Collect(rawExif) log.PanicIf(err) ifd := index.RootIfd @@ -240,12 +228,10 @@ func Test_Ifd_FindTagWithName_Hit(t *testing.T) { func Test_Ifd_FindTagWithName_Miss(t *testing.T) { filepath := path.Join(assetsPath, "NDM_8901.jpg") - e := NewExif() - - rawExif, err := e.SearchAndExtractExif(filepath) + rawExif, err := SearchFileAndExtractExif(filepath) log.PanicIf(err) - _, index, err := e.Collect(rawExif) + _, index, err := Collect(rawExif) log.PanicIf(err) ifd := index.RootIfd @@ -261,12 +247,10 @@ func Test_Ifd_FindTagWithName_Miss(t *testing.T) { func Test_Ifd_FindTagWithName_NonStandard(t *testing.T) { filepath := path.Join(assetsPath, "NDM_8901.jpg") - e := NewExif() - - rawExif, err := e.SearchAndExtractExif(filepath) + rawExif, err := SearchFileAndExtractExif(filepath) log.PanicIf(err) - _, index, err := e.Collect(rawExif) + _, index, err := Collect(rawExif) log.PanicIf(err) ifd := index.RootIfd @@ -282,12 +266,10 @@ func Test_Ifd_FindTagWithName_NonStandard(t *testing.T) { func Test_Ifd_Thumbnail(t *testing.T) { filepath := path.Join(assetsPath, "NDM_8901.jpg") - e := NewExif() - - rawExif, err := e.SearchAndExtractExif(filepath) + rawExif, err := SearchFileAndExtractExif(filepath) log.PanicIf(err) - _, index, err := e.Collect(rawExif) + _, index, err := Collect(rawExif) log.PanicIf(err) ifd := index.RootIfd diff --git a/ifd_tag_entry.go b/ifd_tag_entry.go index 81e7651..75073d3 100644 --- a/ifd_tag_entry.go +++ b/ifd_tag_entry.go @@ -163,11 +163,6 @@ type IfdTagEntryValueResolver struct { } func NewIfdTagEntryValueResolver(exifData []byte, byteOrder binary.ByteOrder) (itevr *IfdTagEntryValueResolver) { - // Make it obvious what data we expect and when we don't get it. - if IsExif(exifData) == false { - log.Panicf("not exif data") - } - return &IfdTagEntryValueResolver{ addressableData: exifData[ExifAddressableAreaStart:], byteOrder: byteOrder,