filecoin-project / go-hamt-ipld

An implementation of a HAMT using ipld
MIT License
29 stars 25 forks source link

Internal HAMT error could cause panic #26

Open ZenGround0 opened 5 years ago

ZenGround0 commented 5 years ago

Is it ok that an inconsistently setup HAMT will cause a panic? The pointer returned here is dereferenced without nil checking.

An inconsistent setup is actually checked earlier within the method returning the pointer here but it silently passes nil just deferring the panic to the caller.

It seems like this method should return an error or else own the decision to panic when in a bad state and not check the index against the length of the Pointers array.

ZenGround0 commented 3 years ago

A year has passed and the link has drifted out of date, but @rvagg made the same observation and recommends adding an error in comment: https://github.com/filecoin-project/go-hamt-ipld/blob/master/hamt.go#L745