feat: Return error when popping last limit in reader

- Updated `PopLimit` in `pkg/dicomio/reader.go` to return an error when attempting to pop from an empty stack, preventing a panic.
- Added `ErrorLimitStackEmpty` to exported errors.
- Moved stack check to the beginning of `PopLimit` to avoid skipping bytes if the operation is invalid.
- Updated call sites in `read.go` to handle or ignore the returned error appropriately.

Author: google-labs-jules[bot]

pkg/dicomio/reader.go

@@ -18,6 +18,9 @@ var (
 	// the current buffer (or enough bytes left until the currently set limit)
 	// to complete the operation.
 	ErrorInsufficientBytesLeft = errors.New("not enough bytes left until buffer limit to complete this operation")
+	// ErrorLimitStackEmpty indicates that PopLimit was called on a reader with no
+	// limits on the stack.
+	ErrorLimitStackEmpty = errors.New("pop limit called on empty stack")
 )
 
 // LimitReadUntilEOF is a special dicomio.Reader limit indicating that there is no hard limit and the
@@ -176,15 +179,20 @@ func (r *Reader) PushLimit(n int64) error {
 
 // PopLimit removes the most recent limit set, and restores the limit before
 // that one.
-func (r *Reader) PopLimit() {
+func (r *Reader) PopLimit() error {
+	// TODO: return an error if trying to Pop the last limit off the slice
+	last := len(r.limitStack) - 1
+	if last < 0 {
+		return ErrorLimitStackEmpty
+	}
+
 	if r.bytesRead < r.limit && r.limit != LimitReadUntilEOF {
 		// didn't read all the way to the limit, so skip over what's left.
 		_ = r.Skip(r.limit - r.bytesRead)
 	}
-	// TODO: return an error if trying to Pop the last limit off the slice
-	last := len(r.limitStack) - 1
 	r.limit = r.limitStack[last]
 	r.limitStack = r.limitStack[:last]
+	return nil
 }
 
 func (r *Reader) IsLimitExhausted() bool {

read.go

@@ -234,7 +234,10 @@ func (r *reader) readHeader() ([]*Element, error) {
 		if err != nil {
 			return nil, err
 		}
-		defer r.rawReader.PopLimit()
+		defer func() {
+			// We can't really do anything if PopLimit fails here (it shouldn't)
+			_ = r.rawReader.PopLimit()
+		}()
 		for !r.rawReader.IsLimitExhausted() {
 			elem, err := r.readElement(nil, nil)
 			if err != nil {
@@ -624,7 +627,9 @@ func (r *reader) readSequence(t tag.Tag, vr string, vl uint32, d *Dataset) (Valu
 			// Append the Item element's dataset of elements to this Sequence's sequencesValue.
 			sequences.value = append(sequences.value, subElement.Value.(*SequenceItemValue))
 		}
-		r.rawReader.PopLimit()
+		if err := r.rawReader.PopLimit(); err != nil {
+			return nil, err
+		}
 	}
 
 	return &sequences, nil
@@ -667,7 +672,9 @@ func (r *reader) readSequenceItem(t tag.Tag, vr string, vl uint32, d *Dataset) (
 			sequenceItem.elements = append(sequenceItem.elements, subElem)
 			seqElements.Elements = append(seqElements.Elements, subElem)
 		}
-		r.rawReader.PopLimit()
+		if err := r.rawReader.PopLimit(); err != nil {
+			return nil, err
+		}
 	}
 
 	return &sequenceItem, nil
@@ -766,7 +773,9 @@ func (r *reader) readFloat(t tag.Tag, vr string, vl uint32) (Value, error) {
 			return nil, fmt.Errorf("error reading floating point element(%v) value: unsupported VR: %w", t, errorUnableToParseFloat)
 		}
 	}
-	r.rawReader.PopLimit()
+	if err := r.rawReader.PopLimit(); err != nil {
+		return nil, err
+	}
 	return retVal, nil
 }
 
@@ -822,7 +831,9 @@ func (r *reader) readInt(t tag.Tag, vr string, vl uint32) (Value, error) {
 			return nil, fmt.Errorf("unable to parse integer type due to unknown VR %v", vr)
 		}
 	}
-	r.rawReader.PopLimit()
+	if err := r.rawReader.PopLimit(); err != nil {
+		return nil, err
+	}
 	return retVal, err
 }