Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

proposal: errors: Fix infinite looping bug #70746

Closed
jordan-bonecutter opened this issue Dec 10, 2024 · 3 comments
Closed

proposal: errors: Fix infinite looping bug #70746

jordan-bonecutter opened this issue Dec 10, 2024 · 3 comments
Labels
Milestone

Comments

@jordan-bonecutter
Copy link

jordan-bonecutter commented Dec 10, 2024

Proposal Details

Consider the following error:

package main

import (
	"errors"
	"io"
)

type CustomWrapper struct {
	Err error
}

func (c CustomWrapper) Error() string {
	return "error"
}

func (c CustomWrapper) Unwrap() error {
	return c.Err
}

func main() {
	var customWrapper = new(CustomWrapper)
	customWrapper.Err = customWrapper
	if errors.Is(customWrapper, io.EOF) {
		panic("eof")
	}
}

This program waits forever in the errors.Is call. I propose to update the is function to be the following:

func is(err, target error, targetComparable bool) bool {
	const maxDepth = 1_000_000
	for range maxDepth {
		if targetComparable && err == target {
			return true
		}
		if x, ok := err.(interface{ Is(error) bool }); ok && x.Is(target) {
			return true
		}
		switch x := err.(type) {
		case interface{ Unwrap() error }:
			err = x.Unwrap()
			if err == nil {
				return false
			}
		case interface{ Unwrap() []error }:
			for _, err := range x.Unwrap() {
				if is(err, target, targetComparable) {
					return true
				}
			}
			return false
		default:
			return false
		}
	}
	return false
}

A better fix for this would be to keep a map of all the error nodes visited and return true if visiting the same node. This is tough as error is not guaranteed to be comparable and thus cannot be a map key (without handling panics). Open to suggestions but I think this is something that should be fixed.

@gopherbot gopherbot added this to the Proposal milestone Dec 10, 2024
@jordan-bonecutter
Copy link
Author

jordan-bonecutter commented Dec 10, 2024

This is very close to #34957

@seankhliao
Copy link
Member

I think the closing comment of #34957 applies, this should not be the errors package's responsibility

@seankhliao seankhliao closed this as not planned Won't fix, can't repro, duplicate, stale Dec 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants