-
Notifications
You must be signed in to change notification settings - Fork 861
Improve CSharp Examples #8562
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
base: master
Are you sure you want to change the base?
Improve CSharp Examples #8562
Conversation
@@ -2425,23 +2425,27 @@ public static byte[] Curve25519ExportPublicKey(IntPtr key) | |||
return publicKey; | |||
} | |||
|
|||
/* Tuples not supported in older frameworks such as 3.5 */ | |||
/* public static (byte[] privateKey, byte[] publicKey) Curve25519ExportKeyRaw(IntPtr key) */ | |||
|
|||
/// <summary> | |||
/// Export both private and public keys from a Curve25519 key structure | |||
/// </summary> | |||
/// <param name="key">Curve25519 key structure</param> | |||
/// <returns>A tuple containing the private key and public key as byte arrays</returns> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update the returns tags to match the new return type (void).
Maybe we could move the comments at line 2428-2429 directly here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, good catch. updated & removed code that was commented out.
//var configAttr = (AssemblyConfigurationAttribute)Attribute.GetCustomAttribute( | ||
// Assembly.GetExecutingAssembly(), typeof(AssemblyConfigurationAttribute)); | ||
|
||
//return configAttr?.Configuration ?? "Unknown"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this commented code be removed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah yes, I did not intend to keep the code commented out.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a couple of minor things that I noticed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some more minor changes.
wrapper/CSharp/README.md
Outdated
Calling ctx Init from wolfSSL | ||
``` | ||
|
||
Note the `WOLFSSL_DLL` is used in the wolfssl `visibiility.h` and defines the `WOLFSSL_API` like this: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Spelling and maybe we could provide the full path to the header file?:
wolfssl/wolfssl/wolfcrypt/visibility.h
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good idea. updated
wrapper/CSharp/README.md
Outdated
|
||
### Build wolfSSL and install to arbitrary directory | ||
|
||
To have a n isolated instance of wolfSSL, in this case `$HOME/wolfssl-install-psk`: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Spelling.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh, good catch! fixed.
cb69f56
to
5bd701d
Compare
Jenkins retest this please. for |
Jenkins retest this please. for "Cannot contact wolf-linux-cloud-node" |
Please resolve merge conflicts |
6a26918
to
fcd98ce
Compare
d579dbe
to
61bce5e
Compare
Jenkins retest this please. for: "Remote call on wolf-linux-cloud-node-nnn failed. The channel is closing down or has closed down" |
Hi @dgarske - merge conflicts resolved. |
Please don't merge this yet. I am not sure if I want to have the VS2005 / .NET 3.5 support introduced this way. I've got a large portion of these changes in #8621 already. The changes made to CSharp for .NET 3.5 should hopefully be backwards compatible. And the actual project files I may want to keep elsewhere. |
Marked as draft per @dgarske #8562 (comment) |
Description
Updates C# Wrapper examples, adds features to wolfSSL C# wrapper, fixes Windows build-time warnings, revised code for compatibility with older frameworks missing language and library features otherwise found in recent versions..
App.config
settings:WOLFSSL_DLL_PATH
andWOLFSSL_CERTS_PATH
.See: https://gojimmypi.github.io/Using-wolfssl-in-Visual-Studio/
.github/workflows/visual-studio.yml
is intentionally separate from the existing win-csharp-test.yml workflow. This newvisual-studio.yml
is intended to grow to be repository-wide, checking all the Visual Studio project files everywhere for line endings, file encoding, and check build configurations for projects beyond just the C# wrapper. Actual functional tests are expected to remain in a separate workflow. See the WIP my_visual-studio.yml which is beyond the scope of this PR.NET 3.5 Compatibility for wolfSSL C# Wrappers
Overview
This PR adds compatibility with .NET Framework 3.5 to the wolfSSL C# wrappers and examples, enabling wolfSSL to be used in environments that require older .NET Framework versions.
Key Changes
Framework Targeting
App.config
files to usesupportedRuntime version="v2.0.50727"
instead of v4.0System.Core
,System.Xml.Linq
,System.Data.DataSetExtensions
)System.Configuration
reference for configuration managementDLL Import/Export Handling
curve25519.h
,ed25519.h
) to fix DLL import/export declarations for better compatibility, fixingwarning C4141: 'dllexport': used more than once
cpp.hint
file to assist Visual Studio with C++ identifier interpretationwolfSSL.cs
with improved DLL path handling for .NET 3.5 environmentssniffer.h
to notdefine SSL_SNIFFER_API __declspec(dllexport)
unlessSSL_SNIFFER_EXPORTS
foundBuild System
visual_studio.yml
) to test building with .NET 3.5include.am
to include new files in distributionError Handling and Logging
App.config
filesDocumentation
Benefits
Fixes zd#
Testing
How did you test?
Tested manually on Windows 2022, Visual Studio 2022. See also new workflow (build only) from my visual_studio.yml
Checklist