-
Notifications
You must be signed in to change notification settings - Fork 32
ENG-29 - Implemented IronRDP Web Proxy on gateway #1188
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: main
Are you sure you want to change the base?
Conversation
|
✅ Build Completed with Success, Version=1188.0.0-34866b8 |
|
✅ Build Completed with Success, Version=1188.0.0-d1e8747 |
|
✅ Build Completed with Success, Version=1188.0.0-9fcc390 |
| defer cancel() | ||
| defer func() { | ||
| s.deadline = nil | ||
| }() |
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.
Use a single defer statement:
defer func() {
cancel()
s.deadline = nil
}()| // UnmarshalContextExplicit fills 'out' (a pointer to struct) from a DER | ||
| // value that is a SEQUENCE whose elements are context-specific EXPLICIT tags. | ||
| // Struct fields must use tags like `asn1:"tag:0"` to map context tags to fields. | ||
| func UnmarshalContextExplicit(der []byte, out interface{}) error { |
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.
Is there any reason to not parse der and set the contents of the type directly?
You could apply logic of optional by using pointers.
From what I've seen, it will require less code to properly set the values into the proper type::
func UnmarshalContextExplicit(der []byte, into *RDCleanPathPdu) error {
// parse der and set values directly to the struct attributes
}Try to avoid using the reflect library, you'll lose the type safety and it could lead to runtime panics.
| // are emitted in struct field order. Slices are encoded as a single SEQUENCE OF | ||
| // unless the element is []byte (OCTET STRING), which is encoded as a single | ||
| // OCTET STRING wrapper. | ||
| func MarshalContextExplicit(v interface{}) ([]byte, error) { |
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.
It seems this function is not used outside this package, make it private by renaming the function to marshalContextExplicit.
Is there any reason to not set the contents directly in a pointer known struct (*RDCleanPathPdu)?
From what I've seen, it will require less code to properly set the values into the proper type.
Try to avoid using the reflect library, you'll lose the type safety and it could lead to runtime panics.
| } | ||
|
|
||
| func (r *RDCleanPathPdu) Encode() []byte { | ||
| data, _ := MarshalContextExplicit(r) |
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.
Silently discard errors will make the program harder to troubleshoot, make sure to return the error for the caller to handle it properly.
| } | ||
|
|
||
| func (r *NegotiatePdu) Encode() []byte { | ||
| data, _ := MarshalContextExplicit(r) |
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.
Silently discard errors will make the program harder to troubleshoot, make sure to return the error for the caller to handle it properly.
| X224ConnectionPDU: buildGenericRdpErrorPacket(), | ||
| } | ||
|
|
||
| _ = ws.WriteMessage(websocket.BinaryMessage, response.Encode()) |
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.
At least log this error, we must know when a packet is not being sent.
| if server, ok := store.Pop(instanceKey).(*RDPProxy); ok { | ||
|
|
||
| if serverAny, ok := store.LoadAndDelete(instanceKey); ok { | ||
| server := serverAny.(*RDPProxy) |
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.
This line could panic, you must cast it safely and check for the rest of the code to see if server is not nil:
server, _ := serverAny.(*RDPProxy)
📝 Description
This implements what's necessary to connect to RDP Gateway using a IronRDP Web Client.
🔗 Related Issue
Partially fixes ENG-29
🚀 Type of Change
📋 Changes Made
/ironwebsocket endpoint to gateway (so web client can connect to it)🧪 Testing
Tests performed: