Skip to content

Conversation

@ManiacTwister
Copy link
Contributor

In the gitlabmodule log.Fatal is called when the module receives invalid/unexpected json data, which leads to program termination with exit status 1 - therefore it is possible to remotely "crash" CptHook. I think Log.Panic is more appropriate here?

@fleaz
Copy link
Owner

fleaz commented Jan 14, 2019

Hi,
thanks for reporting this and and an even bigger thanks to already providing a PR :)
The current behaviour is in fact not great.

According to the Logrus documentation Log.Panic calls the panic() function to gracefully shutdown stuff after logging the error. Wouldn't it be more sensible to just use Log.Error() in the cases where we receive malformed json?

@ManiacTwister ManiacTwister changed the title [GitLab] Use log.Panic instead of log.Fatal for json parsing errors [GitLab] Use log.Error instead of log.Fatal for json parsing errors Jan 14, 2019
@ManiacTwister
Copy link
Contributor Author

Yeah makes sense :D
Changed it to Log.Error

@fleaz fleaz changed the base branch from master to development January 14, 2019 19:14
@fleaz
Copy link
Owner

fleaz commented Jan 22, 2019

Hi @ManiacTwister I fixed some issues with my branches and now this PR should be clean again if you rebase against development. Then I will be happy to merge it :)

@ManiacTwister
Copy link
Contributor Author

@fleaz rebased, looks good now

@fleaz fleaz merged commit 194d161 into fleaz:development Jan 23, 2019
@fleaz
Copy link
Owner

fleaz commented Jan 23, 2019

Awesome. Thanks :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants