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

Tolerate null repo URLs #4077

Merged
merged 2 commits into from
Apr 12, 2024
Merged

Conversation

HebaruSan
Copy link
Member

@HebaruSan HebaruSan commented Apr 12, 2024

Problem

A Discord user reported an exception at startup:

5393 [1] ERROR CKAN.GUI.Main (null) - Object reference not set to an instance of an object.
System.NullReferenceException: Object reference not set to an instance of an object.
   at CKAN.RepositoryDataManager.<>c__DisplayClass7_0.<Update>b__1(Repository r)
   at System.Linq.Enumerable.WhereEnumerableIterator`1.MoveNext()
   at System.Linq.Buffer`1..ctor(IEnumerable`1 source)
   at System.Linq.Enumerable.ToArray[TSource](IEnumerable`1 source)
   at CKAN.RepositoryDataManager.Update(Repository[] repos, IGame game, Boolean skipETags, NetAsyncDownloader downloader, IUser user)
   at CKAN.GUI.Main.UpdateRepo(Object sender, DoWorkEventArgs e)
   at System.ComponentModel.BackgroundWorker.OnDoWork(DoWorkEventArgs e)
   at System.ComponentModel.BackgroundWorker.WorkerThreadStart(Object argument)

Cause

A code audit suggests that this exception could be thrown if Repository.url is null, which is not supposed to be possible but could happen if registry.json has the repo URL set to the empty string. (The deserializer throws an exception instead if it's null in the file, so we can rule that out.)

However, when I tried this, I got many other exceptions in other earlier spots in the code that access this property. At time of writing we have asked follow-up questions for further investigation but haven't heard back yet. The same user said they saw an earlier error message relating to GUIConfig.xml (we don't know which one), so it's possible they're experiencing some disk corruption.

Changes

  • Now various places that access Repository.url are updated to handle null values better.
  • If you have no repositories at all when you attempt a refresh, a default repo called default was being added. Now it's changed to <Game shortname>-<localized equivalent of "default">, consistent with the repo we initially add when you add the instance.

@HebaruSan HebaruSan added Bug Something is not working as intended Core (ckan.dll) Issues affecting the core part of CKAN Registry Issues affecting the registry labels Apr 12, 2024
@HebaruSan HebaruSan force-pushed the fix/null-repo-url-nre branch from a9a1874 to 1d11749 Compare April 12, 2024 17:22
@HebaruSan HebaruSan merged commit 4de7a79 into KSP-CKAN:master Apr 12, 2024
8 checks passed
@HebaruSan HebaruSan deleted the fix/null-repo-url-nre branch April 12, 2024 17:34
@HebaruSan HebaruSan mentioned this pull request Jun 29, 2024
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is not working as intended Core (ckan.dll) Issues affecting the core part of CKAN Registry Issues affecting the registry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant