{"action":"created","issue":{"url":"https://api.github.com/repos/KSP-CKAN/CKAN/issues/2482","repository_url":"https://api.github.com/repos/KSP-CKAN/CKAN","labels_url":"https://api.github.com/repos/KSP-CKAN/CKAN/issues/2482/labels{/name}","comments_url":"https://api.github.com/repos/KSP-CKAN/CKAN/issues/2482/comments","events_url":"https://api.github.com/repos/KSP-CKAN/CKAN/issues/2482/events","html_url":"https://github.com/KSP-CKAN/CKAN/pull/2482","id":347763180,"node_id":"MDExOlB1bGxSZXF1ZXN0MjA2Mjc4MjYw","number":2482,"title":"Fix crash when initializing CKAN dirs at argumentless GUI startup","user":{"login":"HebaruSan","id":1559108,"node_id":"MDQ6VXNlcjE1NTkxMDg=","avatar_url":"https://avatars2.githubusercontent.com/u/1559108?v=4","gravatar_id":"","url":"https://api.github.com/users/HebaruSan","html_url":"https://github.com/HebaruSan","followers_url":"https://api.github.com/users/HebaruSan/followers","following_url":"https://api.github.com/users/HebaruSan/following{/other_user}","gists_url":"https://api.github.com/users/HebaruSan/gists{/gist_id}","starred_url":"https://api.github.com/users/HebaruSan/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/HebaruSan/subscriptions","organizations_url":"https://api.github.com/users/HebaruSan/orgs","repos_url":"https://api.github.com/users/HebaruSan/repos","events_url":"https://api.github.com/users/HebaruSan/events{/privacy}","received_events_url":"https://api.github.com/users/HebaruSan/received_events","type":"User","site_admin":false},"labels":[{"id":122535234,"node_id":"MDU6TGFiZWwxMjI1MzUyMzQ=","url":"https://api.github.com/repos/KSP-CKAN/CKAN/labels/Bug","name":"Bug","color":"e11d21","default":false},{"id":139464212,"node_id":"MDU6TGFiZWwxMzk0NjQyMTI=","url":"https://api.github.com/repos/KSP-CKAN/CKAN/labels/GUI","name":"GUI","color":"006b75","default":false},{"id":139897064,"node_id":"MDU6TGFiZWwxMzk4OTcwNjQ=","url":"https://api.github.com/repos/KSP-CKAN/CKAN/labels/Pull%20request","name":"Pull request","color":"1d76db","default":false},{"id":139516137,"node_id":"MDU6TGFiZWwxMzk1MTYxMzc=","url":"https://api.github.com/repos/KSP-CKAN/CKAN/labels/ckan.dll","name":"ckan.dll","color":"006b75","default":false}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":4,"created_at":"2018-08-06T03:03:23Z","updated_at":"2018-08-06T14:01:53Z","closed_at":null,"author_association":"MEMBER","pull_request":{"url":"https://api.github.com/repos/KSP-CKAN/CKAN/pulls/2482","html_url":"https://github.com/KSP-CKAN/CKAN/pull/2482","diff_url":"https://github.com/KSP-CKAN/CKAN/pull/2482.diff","patch_url":"https://github.com/KSP-CKAN/CKAN/pull/2482.patch"},"body":"## Problem\r\n\r\nStarting in CKAN 1.25.2, If you run the GUI without arguments, and it needs to initialize your CKAN folder, you'll see this:\r\n\r\n```\r\n$ mono ckan.exe \r\nexception inside UnhandledException handler: Object reference not set to an instance of an object\r\n\r\n[ERROR] FATAL UNHANDLED EXCEPTION: System.NullReferenceException: Object reference not set to an instance of an object\r\n  at CKAN.GUIUser.DisplayMessage (System.String message, System.Object[] args) [0x00000] in <352d74f237564d2e9ff313cc9ac1989e>:0 \r\n  at CKAN.NullUser.RaiseMessage (System.String message, System.Object[] args) [0x00000] in <352d74f237564d2e9ff313cc9ac1989e>:0 \r\n  at CKAN.KSP.SetupCkanDirectories () [0x000dd] in <352d74f237564d2e9ff313cc9ac1989e>:0 \r\n  at CKAN.KSP..ctor (System.String gameDir, System.String name, CKAN.IUser user) [0x00038] in <352d74f237564d2e9ff313cc9ac1989e>:0 \r\n  at CKAN.KSPManager.LoadInstancesFromRegistry () [0x00060] in <352d74f237564d2e9ff313cc9ac1989e>:0 \r\n  at CKAN.KSPManager..ctor (CKAN.IUser user, CKAN.IWin32Registry win32_registry) [0x00028] in <352d74f237564d2e9ff313cc9ac1989e>:0 \r\n  at CKAN.Main..ctor (System.String[] cmdlineArgs, CKAN.KSPManager mgr, CKAN.GUIUser user, System.Boolean showConsole) [0x00039] in <352d74f237564d2e9ff313cc9ac1989e>:0 \r\n  at (wrapper remoting-invoke-with-check) CKAN.Main..ctor(string[],CKAN.KSPManager,CKAN.GUIUser,bool)\r\n  at CKAN.GUI.Main_ (System.String[] args, CKAN.KSPManager manager, System.Boolean showConsole) [0x0003b] in <352d74f237564d2e9ff313cc9ac1989e>:0 \r\n  at CKAN.CmdLine.MainClass.Gui (CKAN.KSPManager manager, CKAN.CmdLine.GuiOptions options, System.String[] args) [0x00008] in <352d74f237564d2e9ff313cc9ac1989e>:0 \r\n  at CKAN.CmdLine.MainClass.Main (System.String[] args) [0x000a1] in <352d74f237564d2e9ff313cc9ac1989e>:0 \r\n```\r\n\r\nThis doesn't happen if you run `ckan gui`, which the .deb package does.\r\n\r\n## Causes\r\n\r\nMany different things had to go subtly wrong for this to happen.\r\n\r\nWhen CKAN creates the child folders of the CKAN folder, it (somewhat pointlessly) uses its `IUser` reference to announce the folders it's creating:\r\n\r\nhttps://github.com/KSP-CKAN/CKAN/blob/7e0552316a1bdd72577e86537d0cf34c96eeb59c/Core/KSP.cs#L77-L115\r\n\r\nThese are never seen because only `NullUser` objects were passed as `User` here previously, and that class discards the messages. (They may be displayed when using CmdLine, I'm not sure.) This code is called from `KSPManager`'s constructor.\r\n\r\nAs of #2449, it's possible for a `GUIUser` to be passed instead. `GUIUser` is not a well encapsulated class; it relies on external code to set its properties so it can work properly. Specifically, if its `displayMessage` property isn't set, then calling `GUIUser.DisplayMessage` will throw an exception:\r\n\r\nhttps://github.com/KSP-CKAN/CKAN/blob/7e0552316a1bdd72577e86537d0cf34c96eeb59c/GUI/GUIUser.cs#L10\r\nhttps://github.com/KSP-CKAN/CKAN/blob/7e0552316a1bdd72577e86537d0cf34c96eeb59c/GUI/GUIUser.cs#L22-L25\r\n\r\nIn the `NullUser` family of which `GUIUser` is a part, `DisplayMessage` is a synonym for `RaiseMessage`, so that exception could happen when we try to announce that we created a directory:\r\n\r\nhttps://github.com/KSP-CKAN/CKAN/blob/7e0552316a1bdd72577e86537d0cf34c96eeb59c/Core/User.cs#L74-L77\r\n\r\nAnd sure enough, as of #2449, we can end up calling `SetupCkanDirectories` (via `KSPManager`'s constructor) before that property is set:\r\n\r\nhttps://github.com/KSP-CKAN/CKAN/blob/7e0552316a1bdd72577e86537d0cf34c96eeb59c/GUI/Main.cs#L166-L170\r\n\r\nThis vulnerability is only hit if you run \"ckan\" or \"mono ckan\" without any other parameters, because a special block of code detects that case and `null` is passed here to let GUI make its own `KSPManager` object:\r\n\r\nhttps://github.com/KSP-CKAN/CKAN/blob/7e0552316a1bdd72577e86537d0cf34c96eeb59c/Cmdline/Main.cs#L58-L62\r\n\r\nIn all other cases, the command line parsing code creates the `KSPManager` object here so it can find the default game instance, which avoids the issue since it doesn't use `GUIUser`:\r\n\r\nhttps://github.com/KSP-CKAN/CKAN/blob/7e0552316a1bdd72577e86537d0cf34c96eeb59c/Cmdline/Main.cs#L125-L128\r\n\r\n## Changes\r\n\r\nThe two different sets of logic for defaulting to \"gui\" when no other command is present are now merged into one for consistency. Instead of checking the number of arguments twice, we use the Linq function `All` to check the arguments, which returns true if the list is empty, and instead of prepending \"gui\" to the arg list in one case and directly calling `Gui` in the other, we now always prepend \"gui\" to allow full processing of the arguments consistent with other cases. Technically this would have been enough to fix the crash by itself (since now the argumentless case works the same as \"ckan gui\", which doesn't crash), but the other problem areas warrant some attention as well to increase general robustness.\r\n\r\nNow `GUIUser` checks if its properties are null before calling them, so this situation won't cause null reference exceptions again. This also would have fixed the crash by itself.\r\n\r\nNow if GUI needs to instantiate its own `KSPManager`, it will set the required `GUIUser` properties before it does that. This also would have fixed the crash by itself.\r\n\r\nFixes #2483."},"comment":{"url":"https://api.github.com/repos/KSP-CKAN/CKAN/issues/comments/410718363","html_url":"https://github.com/KSP-CKAN/CKAN/pull/2482#issuecomment-410718363","issue_url":"https://api.github.com/repos/KSP-CKAN/CKAN/issues/2482","id":410718363,"node_id":"MDEyOklzc3VlQ29tbWVudDQxMDcxODM2Mw==","user":{"login":"The-Spanish-Inquisition1","id":42143988,"node_id":"MDQ6VXNlcjQyMTQzOTg4","avatar_url":"https://avatars0.githubusercontent.com/u/42143988?v=4","gravatar_id":"","url":"https://api.github.com/users/The-Spanish-Inquisition1","html_url":"https://github.com/The-Spanish-Inquisition1","followers_url":"https://api.github.com/users/The-Spanish-Inquisition1/followers","following_url":"https://api.github.com/users/The-Spanish-Inquisition1/following{/other_user}","gists_url":"https://api.github.com/users/The-Spanish-Inquisition1/gists{/gist_id}","starred_url":"https://api.github.com/users/The-Spanish-Inquisition1/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/The-Spanish-Inquisition1/subscriptions","organizations_url":"https://api.github.com/users/The-Spanish-Inquisition1/orgs","repos_url":"https://api.github.com/users/The-Spanish-Inquisition1/repos","events_url":"https://api.github.com/users/The-Spanish-Inquisition1/events{/privacy}","received_events_url":"https://api.github.com/users/The-Spanish-Inquisition1/received_events","type":"User","site_admin":false},"created_at":"2018-08-06T14:01:53Z","updated_at":"2018-08-06T14:01:53Z","author_association":"NONE","body":"I'm still getting the error.\r\n\r\n![ckan bug 2](https://user-images.githubusercontent.com/42143988/43721022-b24c2fa8-995f-11e8-90b3-2776ab23d980.PNG)\r\n"}}
{"actor":{"display_login":"The-Spanish-Inquisition1"}}