feat(ffi): expose API for RDCleanPath#855
feat(ffi): expose API for RDCleanPath#855irvingoujAtDevolution wants to merge 8 commits intomasterfrom
Conversation
irvingoujAtDevolution
force-pushed
the
ffi-gateway-websocket
branch
from
019e1d4 to
4277e8e
Compare
There was a problem hiding this comment.
I suggest we make this file ignored
There was a problem hiding this comment.
Is it a VS Code file? Sure, we can ignore that. Note that you can locally decide what to ignore using the $GIT_DIR/info/exclude file. It’s the same format as gitignore.
|
Please, fix the errors reported by the CI |
| if (count == 0) | ||
| { | ||
| // Note: this is particularly important, if we send a zero-length frame, | ||
| // somehow Gateway will raise TLS issue during the proxy. | ||
| return; // Nothing to write |
There was a problem hiding this comment.
note: I’m not sure why you get a TLS issue at this point, since you are supposed to have an already established TLS connection at this point, but SendAsync with an empty ArraySegment will result in a WebSocket frame to be send. A "Binary" frame that contains an empty byte array.
| public override void Flush() | ||
| { | ||
| } |
There was a problem hiding this comment.
question: Nothing to do here? Could you add a comment explaining why?
There was a problem hiding this comment.
The flush is never called as well, I'm not an expert in C# but I think there's some related to the "can seek" property. However, since this does not block us in any meaningful way, I;ll just leave a comment.
There was a problem hiding this comment.
Flush is something that the consumer may call himself, in this case, us. It’s supposed to flush the write buffer, and immediately send the contents on the wire. Maybe it’s not required for ClientWebSocket because it already flushes before completing future returned by SendAsync, etc functions. The tungstenite library for Rust does expose a flush function, but it’s not the case for the C# class found in the standard library.
CBenoit
changed the title
CBenoit
changed the title
|
Also, ideally I would like two PRs:
|
irvingoujAtDevolution
force-pushed
the
ffi-gateway-websocket
branch
from
08790f9 to
30ddbf4
Compare
Coverage Report 🤖 ⚙️Past: New: Diff: -0.02% [this comment will be updated automatically] |
I created this one dedicated for FFI changes, after it's merged I can rebase current PR to master, because the changes in FFI source would not compile without generated C# code. |
Great, thank you! |
2 participants
Expose RDCleanPath via FFI, enable IronRDP .NET connections via RDCleanPath.
Issue: ARC-310