Skip to content

add more ldap results, ldap result from type uint8 to type uint16 #160

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
Oct 22, 2018
Merged

add more ldap results, ldap result from type uint8 to type uint16 #160

merged 5 commits into from
Oct 22, 2018

Conversation

wOvAN
Copy link
Contributor

@wOvAN wOvAN commented May 10, 2018

More result codes taken from https://ldap.com/ldap-result-code-reference/
Result type uint8 to uint16 as the actual values exceeds uint8

@wOvAN wOvAN closed this May 10, 2018
@wOvAN wOvAN reopened this May 10, 2018
@wOvAN
Copy link
Contributor Author

wOvAN commented May 10, 2018

smth wrong with golint in 1.6? "/bin/sh: 1: golint: not found"

liggitt
liggitt reviewed May 10, 2018
View reviewed changes
error.go Outdated
LDAPResultAssertionFailed = 122
LDAPResultAuthorizationDenied = 123
LDAPResultSyncRefreshRequired = 4096
LDAPResultNoOperation = 16654
Copy link
Contributor

Choose a reason for hiding this comment

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

the draft-zeilenga-ldap-noop draft does not specify a value for the noOperation result code... seems odd to embed a value some implementations just picked into this library

@wOvAN wOvAN closed this May 10, 2018
@wOvAN wOvAN reopened this May 10, 2018
@wOvAN
Copy link
Contributor Author

wOvAN commented May 10, 2018

removed nooperation, still don't get why pipeline 1.6 failes

@liggitt
Copy link
Contributor

liggitt commented May 10, 2018

still don't get why pipeline 1.6 failes

fixed in #161

@wOvAN
Copy link
Contributor Author

wOvAN commented May 10, 2018

maybe it's better to use uint32 cos winapi uses ulong (8 bytes) for result "WINLDAPAPI ULONG LDAPAPI ldap_simple_bindW( LDAP *ld, PWCHAR dn, PWCHAR passwd );" ?

@liggitt
Copy link
Contributor

liggitt commented May 10, 2018

The ldap spec says values over ~16000 aren't allowed, so uint16 is sufficient

@vetinari vetinari added this to the v3 milestone May 28, 2018
@vetinari vetinari mentioned this pull request May 28, 2018
Closed
@vetinari
Copy link
Contributor

@liggitt any objections merging this to master?

@liggitt
Copy link
Contributor

liggitt commented May 30, 2018 •
edited
Loading

I hadn't swept the new codes/messages yet. Once someone does that, no objections to merging

@vetinari vetinari mentioned this pull request Jun 29, 2018
Closed
johnweldon
johnweldon approved these changes Oct 22, 2018
View reviewed changes
Copy link
Member

@johnweldon johnweldon left a comment

Choose a reason for hiding this comment

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

Thanks

@johnweldon johnweldon merged commit 43d151a into go-ldap:master Oct 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@liggitt liggitt liggitt left review comments

@johnweldon johnweldon johnweldon approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v3
Development

Successfully merging this pull request may close these issues.
4 participants
@wOvAN @liggitt @vetinari @johnweldon