Skip to content

Add more nullable annotations #1183

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

Open
wants to merge 281 commits into
base: nullable-api
Choose a base branch
from

Conversation

daniel-lerch
Copy link
Contributor

@daniel-lerch daniel-lerch commented Aug 11, 2025

This pull request adds many more nullable annotations and enables nullable reference types for the entire MimeKit project.

There are 99 warnings left related to nullability. I find these hard to fix because they are related to fields that can be null but only under certain circumstances i.e. parser state, depending on the super class or which constructor was used. @jstedfast would you be willing to work on these remaining warnings?

P.S. You may want to merge master into nullable-api for this diff to show only files that I changed.

jstedfast and others added 30 commits November 18, 2024 09:57
Bumps [Newtonsoft.Json](https://github.com/JamesNK/Newtonsoft.Json) and [Microsoft.NET.Test.Sdk](https://github.com/microsoft/vstest). These dependencies needed to be updated together.

Updates `Newtonsoft.Json` from 13.0.3 to 13.0.1
- [Release notes](https://github.com/JamesNK/Newtonsoft.Json/releases)
- [Commits](JamesNK/Newtonsoft.Json@13.0.3...13.0.1)

Updates `Microsoft.NET.Test.Sdk` from 17.11.1 to 17.12.0
- [Release notes](https://github.com/microsoft/vstest/releases)
- [Changelog](https://github.com/microsoft/vstest/blob/main/docs/releases.md)
- [Commits](microsoft/vstest@v17.11.1...v17.12.0)

---
updated-dependencies:
- dependency-name: Newtonsoft.Json
  dependency-type: direct:production
  update-type: version-update:semver-patch
- dependency-name: Microsoft.NET.Test.Sdk
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…s.Unsafe (jstedfast#1105)

Bumps [System.Memory](https://github.com/dotnet/maintenance-packages), [System.Buffers](https://github.com/dotnet/maintenance-packages) and [System.Runtime.CompilerServices.Unsafe](https://github.com/dotnet/maintenance-packages). These dependencies needed to be updated together.

Updates `System.Memory` from 4.5.5 to 4.6.0
- [Release notes](https://github.com/dotnet/maintenance-packages/releases)
- [Commits](https://github.com/dotnet/maintenance-packages/commits)

Updates `System.Buffers` from 4.6.0 to 4.6.0
- [Release notes](https://github.com/dotnet/maintenance-packages/releases)
- [Commits](https://github.com/dotnet/maintenance-packages/commits)

Updates `System.Runtime.CompilerServices.Unsafe` from 6.1.0 to 6.1.0
- [Release notes](https://github.com/dotnet/maintenance-packages/releases)
- [Commits](https://github.com/dotnet/maintenance-packages/commits)

---
updated-dependencies:
- dependency-name: System.Memory
  dependency-type: direct:production
  update-type: version-update:semver-minor
- dependency-name: System.Buffers
  dependency-type: direct:production
  update-type: version-update:semver-patch
- dependency-name: System.Runtime.CompilerServices.Unsafe
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [altcover.global](https://github.com/SteveGilham/altcover) from 8.9.3 to 9.0.1.
- [Release notes](https://github.com/SteveGilham/altcover/releases)
- [Changelog](https://github.com/SteveGilham/altcover/blob/master/ReleaseNotes.md)
- [Commits](SteveGilham/altcover@release/v8.9.3...release/v9.0.1)

---
updated-dependencies:
- dependency-name: altcover.global
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [AltCover](https://github.com/SteveGilham/altcover) from 8.9.3 to 9.0.1.
- [Release notes](https://github.com/SteveGilham/altcover/releases)
- [Changelog](https://github.com/SteveGilham/altcover/blob/master/ReleaseNotes.md)
- [Commits](SteveGilham/altcover@release/v8.9.3...release/v9.0.1)

---
updated-dependencies:
- dependency-name: AltCover
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
This was causing some MailKit unit tests to fail because the default
for Encoding.GetEncoding(int codepage) must have changed between net6.0
and net8.0 to fall back to using "?" as a fallback string for incomplete
or invalud character sequences.

Anyway, explicitly use string.Empty as the fallback.
…s.Unsafe

(at least theoretically)

Also disabled IsAotCompatible for now since we're not there yet.
Until NUnit runs under the net8.0 runtime, we need to continue to target
net6.0 for the UnitTests so that they continue to run in CI.
Avoid duplicating the parsed List<InternetAddress> returned by an
internal parsing method when packing it into an InternetAddressList.
In the case described in issue jstedfast#1106, there are 1000's of "addresses"
which are really only quoted-strings separated by commas.

What this patch does is to prevent the parser from consuming the entire
string as 1 display-name, then deciding that there's no address (reached
end of string), so falling back to parsing the first qstring as a local-part
instead.

This was causing the parser to be O(N^2), so the larger that recipient list
got, the worse the performance got.

Fixes issue jstedfast#1106
Bumps [BouncyCastle.Cryptography](https://github.com/bcgit/bc-csharp) from 2.4.0 to 2.5.0.
- [Commits](bcgit/bc-csharp@release-2.4.0...release-2.5.0)

---
updated-dependencies:
- dependency-name: BouncyCastle.Cryptography
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…tleSecureMimeContext (jstedfast#1111)

While implementing IX509CertificateDatabase and extending BouncyCastleSecureMimeContext I ran into a few internal methods I would like to reuse for consistency.

Co-authored-by: Joseph Shook <[email protected]>
* Added support for domain-wide S/MIME certificates

Replaces pr jstedfast#1112

* Added unit test for smime.db upgrade (v1 -> v2)

* Updated to allow for multiple DnsNames per certificate

Also updated code to use constants for the table and column names.

* Generate a certificate with DnsNames and updated tests

* Fixed WindowsSecureMimeContext to work with domain-bound certificates

Added X509CertificateExtensions.GetSubjectDnsNames()
@jstedfast
Copy link
Owner

Is there a reason for <Nullable>enable</Nullable> in the .csproj and the #nullable enable in each .cs file?

@daniel-lerch
Copy link
Contributor Author

There is absolutely no reason. I just forgot to commit and push some changes. Before enabling nullable reference types for the entire project, I started with enabling it for some files only to have less warnings to work on in parallel.

@jstedfast
Copy link
Owner

Figured I should post an update. I just spent the last week migrating off my old workstation onto a new work laptop and just bought myself a personal laptop to use for MimeKit development (since my PC is stuck on Windows 10 and it was time for an upgrade).

I'll try to get to this soon, but this weekend will likely be spent setting up my new personal laptop.

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.

7 participants