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

add DirSync Control / Search #110

Closed
wants to merge 12 commits into from
Closed

Conversation

vetinari
Copy link
Contributor

this implements the dirsync control as described in
https://msdn.microsoft.com/en-us/library/aa366978(v=vs.85).aspx

check ExampleDirSync() in search_test.go how to use

vetinari and others added 2 commits April 27, 2017 16:20
this implements the dirsync control as described in
https://msdn.microsoft.com/en-us/library/aa366978(v=vs.85).aspx

check ExampleDirSync() in search_test.go how to use
@johnweldon
Copy link
Member

@liggitt will you get a chance to review this on the theoretical merits? I'm pretty out of touch with LDAP stuff for a few years now. I'll review on technical merits.
@vetinari Thanks !

control.go Outdated
value.Description += " (DirSync)"
c := new(ControlDirSync)
if value.Value != nil {
valueChildren := ber.DecodePacket(value.Data.Bytes())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason DecodePacketErr is not used and checked here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

laziness? ;-) this started as a copy of the paging control above...

control.go Outdated
value.Description = "DirSync Control Value"
value.Children[0].Description = "Flags"
value.Children[1].Description = "MaxAttrCnt"
value.Children[2].Description = "Cookie"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this safe; assuming Children has at least 3 elements?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the spec says there must be always 3, the cookie will be an empty string if it's unset (which only happens when the client sends the first time

@@ -29,3 +31,48 @@ func TestNewEntry(t *testing.T) {
iteration = iteration + 1
}
}

func ExampleDirSync() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there tests that could be added too?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, encoding / decoding in control_test.go

@vetinari
Copy link
Contributor Author

vetinari commented May 2, 2017

the race fix would also fix the build of PR #104

conn.go Outdated
@@ -96,6 +96,7 @@ type Conn struct {
once sync.Once
outstandingRequests uint
messageMutex sync.Mutex
timeoutMutex sync.Mutex
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vetinari
Copy link
Contributor Author

waiting for #168 ...

@johnweldon
Copy link
Member

Can we clean this up and get it ready to merge?

@johnweldon
Copy link
Member

@vetinari any chance you'd want to update this PR too?

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.

3 participants