Navigation Menu

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

encoding/gob: panics encoding nil pointer - reopen #31664

Open
mateuszmmeteo opened this issue Apr 24, 2019 · 7 comments
Open

encoding/gob: panics encoding nil pointer - reopen #31664

mateuszmmeteo opened this issue Apr 24, 2019 · 7 comments
Labels
NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@mateuszmmeteo
Copy link

mateuszmmeteo commented Apr 24, 2019

This isssue is related to #3704 (cannot be reopened)

Currently encoding/gob panics if it is not able to do something. It's incorrect as this cannot be caught and supported on code level to provide "workaround" or failsafe activity to recover.

I'm not convinced by "that's the way it should react" in response of related message.

panic: gob: cannot encode nil pointer of type *makelibrary.MiddlewareRequest [recovered]
	panic: gob: cannot encode nil pointer of type *makelibrary.MiddlewareRequest

The code:

func SerializeObject(object interface{}) []byte {
	var serializationBuffer bytes.Buffer
	enc := gob.NewEncoder(&serializationBuffer)
	err := enc.Encode(object)
	if err != nil {
		log.Println("Cannot serialize object. Error is: " + err.Error())
		return []byte{}
	}
	return serializationBuffer.Bytes()
}

As you see I don't care about panic here. It can be empty in my case.

This error could be supported on code level if it's catchable - in my context for example I can skip object that has nil pointer because it will not be deserialized back.

Platform details:
Go version: 1.12
GOOS = linux
GOARCH = amd64

I would expect to have error returned so it will be caught and workaround could be done.

@mateuszmmeteo mateuszmmeteo changed the title encoder/gob goes panic on encoding nil pointer - reopen encoding/gob goes panic on encoding nil pointer - reopen Apr 24, 2019
@mateuszmmeteo
Copy link
Author

mateuszmmeteo commented Apr 24, 2019

Solved by additional implementaiton of check. Still enc.Encode() should return error and not panic.

func SerializeObject(object interface{}) []byte {
	var serializationBuffer bytes.Buffer

        // workaround
	if object == nil || (reflect.ValueOf(object).Kind() == reflect.Ptr && reflect.ValueOf(object).IsNil()) {
		return []byte{}
	}

	enc := gob.NewEncoder(&serializationBuffer)
	err := enc.Encode(object)  // panic here
	if err != nil {
		log.Println("Cannot serialize object. Error is: " + err.Error())
		return []byte{}
	}
	return serializationBuffer.Bytes()
}

@mateuszmmeteo
Copy link
Author

And the diff:

diff --git a/src/encoding/gob/encoder.go b/src/encoding/gob/encoder.go
index 53e2cace16..ea3cbe9397 100644
--- a/src/encoding/gob/encoder.go
+++ b/src/encoding/gob/encoder.go
@@ -214,13 +214,12 @@ func (enc *Encoder) sendTypeId(state *encoderState, ut *userTypeInfo) {
 
 // EncodeValue transmits the data item represented by the reflection value,
 // guaranteeing that all necessary type information has been transmitted first.
-// Passing a nil pointer to EncodeValue will panic, as they cannot be transmitted by gob.
 func (enc *Encoder) EncodeValue(value reflect.Value) error {
        if value.Kind() == reflect.Invalid {
                return errors.New("gob: cannot encode nil value")
        }
        if value.Kind() == reflect.Ptr && value.IsNil() {
-               panic("gob: cannot encode nil pointer of type " + value.Type().String())
+               return errors.New("gob: cannot encode nil pointer of type " + value.Type().String())
        }
 
        // Make sure we're single-threaded through here, so multiple

@bradfitz bradfitz changed the title encoding/gob goes panic on encoding nil pointer - reopen encoding/gob: panics encoding nil pointer - reopen Apr 25, 2019
@bradfitz
Copy link
Contributor

/cc @robpike

@bradfitz bradfitz added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Apr 25, 2019
@bradfitz bradfitz added this to the Unplanned milestone Apr 25, 2019
@mateuszmierzwinski
Copy link

mateuszmierzwinski commented Apr 25, 2019

@gopherbot
Copy link

Change https://golang.org/cl/173781 mentions this issue: encoding/gob: Removal of panic from EncodeValue

@callthingsoff
Copy link
Contributor

Hi, any updates?

@gopherbot
Copy link

Change https://go.dev/cl/554415 mentions this issue: encoding/gob: remove panic on nil pointer values in EncodeValue

@dmitshur dmitshur modified the milestones: Unplanned, Go1.23 Jan 8, 2024
@dmitshur dmitshur added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Jan 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants