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

Problem with using ReferencedNodes #682

Closed
sruehl opened this issue Aug 24, 2023 · 5 comments · Fixed by #683
Closed

Problem with using ReferencedNodes #682

sruehl opened this issue Aug 24, 2023 · 5 comments · Fixed by #683
Milestone

Comments

@sruehl
Copy link
Contributor

sruehl commented Aug 24, 2023

When using ReferencedNodes new Nodes will be created using the NodeID of a ExpandedNode. This NodeID has flags for namespace and server which (the values as we use Node not ExpandedNode) then are not passed when using those Nodes

// ReferencedNodes returns the nodes referenced by this node.
func (n *Node) ReferencedNodes(ctx context.Context, refs uint32, dir ua.BrowseDirection, mask ua.NodeClass, includeSubtypes bool) ([]*Node, error) {
	if refs == 0 {
		refs = id.References
	}
	var nodes []*Node
	res, err := n.References(ctx, refs, dir, mask, includeSubtypes)
	if err != nil {
		return nil, err
	}
	for _, r := range res {
		nodes = append(nodes, n.c.Node(r.NodeID.NodeID))
	}
	return nodes, nil
}

here you can see the problematic line: nodes = append(nodes, n.c.Node(r.NodeID.NodeID))

So I have a server which responds with that on a BrowseRequest call:
UA Secure Conversation Message: BrowseRequest
image
When using then those nodeIDs for read it produces a invalid request because of the flags and the server returns a error:
UA Secure Converation Message[Malformed Packet]
image
here in the request above you can see that the flags c0 are set from the retrieved ExpandedNode
ErrorMessage
image

@sruehl
Copy link
Contributor Author

sruehl commented Aug 24, 2023

@magiconair could it be that this todo is the root cause?
node_id.go:16 "// todo(fs): fix mask"

@sruehl
Copy link
Contributor Author

sruehl commented Aug 24, 2023

#683 fixes the issue

@kung-foo
Copy link
Member

@sruehl is there overlap with the issue described here? #619

@sruehl
Copy link
Contributor Author

sruehl commented Aug 28, 2023

I think it is basically the same Problem. Although I like the way #619 is a bit more efficient I would prefer having a NodeFromExpandedNode factory function which copies the values and set's the mask like it is done in #619. The function there called RemoveExpandedFlag doesn't make much sense outside the copy scenario. My approach does that externals so the NodeFromExpandedNode would be more efficient because it has access to the private fields. I could update the PR with that suggestion if wanted.

@sruehl
Copy link
Contributor Author

sruehl commented Aug 28, 2023

my approach == #683

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 a pull request may close this issue.

3 participants