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

Fix NRE on failed queue inflate #2866

Merged
merged 1 commit into from
Sep 11, 2019

Conversation

HebaruSan
Copy link
Member

Problem

If the queue inflator from #2798 encounters an error while inflating a netkan, an exception is thrown while trying to process the error:

544 [1] INFO CKAN.NetKAN.Processors.Inflator (null) - Using user-supplied cache at ckan_cache
1773837 [1] INFO CKAN.NetKAN.Processors.QueueHandler (null) - Inflating CustomDesignSpacesuits-Steampunk
1773840 [1] INFO CKAN.NetKAN.Validators.HasIdentifierValidator (null) - Validating that metadata contains an identifier
1773840 [1] INFO CKAN.NetKAN.Validators.KrefValidator (null) - Validating that metadata contains valid or null $kref
1773848 [1] INFO CKAN.NetKAN.Processors.Inflator (null) - Input successfully passed pre-validation
1774796 [1] INFO CKAN.NetKAN.Processors.QueueHandler (null) - Inflation failed, sending error: Ssl error:1000007d:SSL routines:OPENSSL_internal:CERTIFICATE_VERIFY_FAILED
  at /build/mono-5.20.1.34/external/boringssl/ssl/handshake_client.c:1132
1774797 [1] FATAL CKAN.NetKAN.Program (null) - Object reference not set to an instance of an object

(That Object reference not set to an instance of an object line shouldn't be there. The rest are fine.)

Cause

If inflation fails, we don't have a ckan object, so we pass null to sendCkan:

sendCkan(null, netkan, false, e.Message);

private void sendCkan(Metadata ckan, Metadata netkan, bool success, string err = null)

This is passed to Program.CkanFileName:

StringValue = Program.CkanFileName(ckan)

... which assumes it can't be null:

CKAN/Netkan/Program.cs

Lines 167 to 177 in d47a5b7

internal static string CkanFileName(Metadata metadata)
{
return Path.Combine(
Options.OutputDir,
string.Format(
"{0}-{1}.ckan",
metadata.Identifier,
metadata.Version.ToString().Replace(':', '-')
)
);
}

The NRE would be thrown when we try to access .Identifier or .Version.

Changes

Now the FileName outgoing queue attribute is optional, only sent when the ckan parameter is non-null, because without it we don't know what the filename should be.

Fixes #2865.

@HebaruSan HebaruSan added Bug Something is not working as intended Easy This is easy to fix Pull request Netkan Issues affecting the netkan data labels Sep 10, 2019
@HebaruSan HebaruSan requested a review from techman83 September 10, 2019 16:40
@techman83
Copy link
Member

@HebaruSan as always, love your work! I also have a sneaking suspicion that I might also not be account for a failure on the indexer side 😉

@techman83 techman83 merged commit 910d89b into KSP-CKAN:master Sep 11, 2019
@HebaruSan HebaruSan deleted the fix/queue-err-crash branch September 11, 2019 01:38
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 Easy This is easy to fix Netkan Issues affecting the netkan data
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] Netkan QueueHandler Crashes on SSL Certificate Failure
2 participants