rwcarlsen / goexif

Decode embedded EXIF meta data from image files.
BSD 2-Clause "Simplified" License
627 stars 134 forks source link

panic: division by zero #51

Open mholt opened 7 years ago

mholt commented 7 years ago

I'm trying to track down how to reproduce this error, but:

panic: division by zero

goroutine 14 [running]:
panic(0x2d48a0, 0xc42120e7a0)
    /usr/local/go/src/runtime/panic.go:500 +0x1a1
math/big.(*Rat).SetFrac64(0xc420d99740, 0x0, 0x0, 0x13)
    /usr/local/go/src/math/big/rat.go:311 +0x147
math/big.NewRat(0x0, 0x0, 0x0)
    /usr/local/go/src/math/big/rat.go:25 +0x4f
github.com/rwcarlsen/goexif/tiff.(*Tag).Rat(0xc4207d4000, 0x0, 0x34b1b7, 0xb, 0xc420253dd8)
    /Users/matt/Dev/src/github.com/rwcarlsen/goexif/tiff/tag.go:329 +0x7a

So this line must mean that d is 0.

My calling code:

// altitude
rawAlt, err := x.Get(exif.GPSAltitude)
if err != nil {
    return nil, fmt.Errorf("getting altitude from EXIF: %v", err)
}
alt, err := rawAlt.Rat(0) // *boom*
if err != nil {
    return nil, fmt.Errorf("converting altitude value: %v", err)
}
altFlt, _ := alt.Float64()
gmcnaughton commented 7 years ago

Can you link to a sample image that repros the panic?

The docs for Rat say "It panics for zero deminators or if i is out of range.", so it sounds like the panic is expected, based on a zero denominator in the exif data (though maybe returning an error instead would make it easier to use!)

mholt commented 7 years ago

Hmm, I don't have the sample image anymore, I'd have to dig to find it. Which I could do, but... later.

I really wish it didn't panic. :) Libraries shouldn't panic. It should return an error value.

gmcnaughton commented 7 years ago

Totally agree!

FWIW, looking at the open pull requests, it looks like active development on this library has moved to https://github.com/xor-gate/goexif2.

It might be worth checking to see whether that fork has the same bug, and if so re-reporting it there.

xor-gate commented 2 years ago

I'm commenting on this old issue as i'm the author of goexif2 the fork of rwcarlsen/goexif. This issue is still not resolved but was resolved in goexif2. I have archived goexif2 as both projects are diverged. I would like to see all the patches I did integrated into rwcarlsen/goexif so it could be rock solid.

@rwcarlsen what do you think about merging the patches I did on goexif2 to make it more mature back to goexif?

mholt commented 2 years ago

@rwcarlsen Any thoughts on that? ^

rwcarlsen commented 2 years ago

That sounds fine to me. @mholt Would you be willing to take lead? I can add you as a collaborator here. I'm busy trying to build a house in my spare time right now.

mholt commented 2 years ago

@rwcarlsen Thanks for the invitation. I am a bit busy lately myself; but if I can circle back to this I will let you know!

This is a tough time to build a house, best of luck! That's exciting though.