Fix heavy regression on GitLab for version 3.0.0 #3000
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Hello there, First, thanks for the work made on mattermost. I found on new release 3.0.0 an heavy regression on SSO with gitlab, when I was trying to connect to it i get this stacktrace: ``` 2016/05/14 16:25:54 runtime error: invalid memory address or nil pointer dereference goroutine 16 [running]: runtime/debug.Stack(0x0, 0x0, 0x0) /usr/local/go/src/runtime/debug/stack.go:24 +0x80 runtime/debug.PrintStack() /usr/local/go/src/runtime/debug/stack.go:16 +0x18 github.com/gorilla/handlers.recoveryHandler.log(0x7f064598d840, 0xc8202bfbc0, 0x0, 0x1, 0xca20c0, 0xc82000a0b0) /var/lib/jenkins/jobs/mattermost-platform-release/workspace/src/github.com/mattermost/platform/Godeps/_workspace/src/github.com/gorilla/handlers/recovery.go:84 +0xb8 github.com/gorilla/handlers.recoveryHandler.ServeHTTP.func1(0x7f06459d4710, 0xc820323c70, 0x7f064598d840, 0xc8202bfbc0, 0x0, 0xc82038c101) /var/lib/jenkins/jobs/mattermost-platform-release/workspace/src/github.com/mattermost/platform/Godeps/_workspace/src/github.com/gorilla/handlers/recovery.go:69 +0x9b panic(0xca20c0, 0xc82000a0b0) /usr/local/go/src/runtime/panic.go:443 +0x4e9 github.com/mattermost/platform/model/gitlab.userFromGitLabUser(0xc8207f87d0, 0xc82026d9c0) /var/lib/jenkins/jobs/mattermost-platform-release/workspace/src/github.com/mattermost/platform/model/gitlab/gitlab.go:50 +0x2a9 github.com/mattermost/platform/model/gitlab.(*GitLabProvider).GetUserFromJson(0x11eb358, 0x7f06459a0a40, 0xc82026d9c0, 0x6) /var/lib/jenkins/jobs/mattermost-platform-release/workspace/src/github.com/mattermost/platform/model/gitlab/gitlab.go:90 +0x4c ``` This stack trace is relative how the code deal with pointer. And what I found is that you trying to pass a value to a pointer which is not initialized, so the compiler is not complaining because it doesn't check if pointers are initialized in a struct. So, I did a small fix for a small bug which break everything. I hope it will help
|
Thanks @ArthurHlt! Highly appreciated, will look into this, |
|
@it33 You're welcome |
| @@ -47,7 +47,8 @@ func userFromGitLabUser(glu *GitLabUser) *model.User { | |||
| } | |||
| strings.TrimSpace(user.Email) | |||
| user.Email = glu.Email | |||
| *user.AuthData = strconv.FormatInt(glu.Id, 10) | |||
There was a problem hiding this comment.
here user is created with &model.User{} and have AuthData which is a pointer on a string.
But the default of a pointer is nil, no memory is allocated and it's not pointing to an address and in this part of the code it tries to put the value from strconv.FormatInt(glu.Id, 10) in the memory where the address inside AuthData pointed which is nil.
|
Thanks @ArthurHlt, we're trying to get in touch with the people who can review and commit this and cut a 3.0.1 before May 16. We'll make sure to fix this before it goes into GitLab omnibus on May 22 |
|
@it33 ok no problem, thank you |
|
+1 |
1 similar comment
|
+1 |
jwilander
added
4: Reviews Complete
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hello there,
First, thanks for the work made on mattermost.
I found on new release 3.0.0 an heavy regression on SSO with gitlab, when I was trying to connect to it i get this stacktrace:
This stack trace is relative how the code deal with pointer.
And what I found is that you trying to pass a value to a pointer which is not initialized, so the compiler is not complaining because it doesn't check if pointers are initialized in a struct.
So, I did a small fix for a small bug which break everything. I hope it will help