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

fix: remove matchedDN and diagnosticMessage from SearchResponseEntry #58

Merged
merged 1 commit into from
Jan 15, 2024

Conversation

xunleii
Copy link
Contributor

@xunleii xunleii commented Dec 20, 2023

Hi,

Firstly, thanks for this library !!

This PR removes matchedDN and diagnosticMessage from SearchEntryResponse because they should not be found here.

SearchResultEntry ::= [APPLICATION 4] SEQUENCE {
     objectName      LDAPDN,
     attributes      PartialAttributeList }

PartialAttributeList ::= SEQUENCE OF
       partialAttribute PartialAttribute

LDAPDN ::= LDAPString
           -- Constrained to  [RFC4514]

LDAPString ::= OCTET STRING -- UTF-8 encoded,
              -- [ISO10646] characters

PartialAttribute ::= SEQUENCE {
     type       AttributeDescription,
     vals       SET OF value AttributeValue }

AttributeDescription ::= LDAPString
           -- Constrained to
           -- [RFC4512]

AttributeValue ::= OCTET STRING

source: https://ldap.com/ldapv3-wire-protocol-reference-search/,

@jimlambrt
Copy link
Owner

If there's no rush and you don't mind: I'll take a look after the holiday.

@xunleii
Copy link
Contributor Author

xunleii commented Dec 24, 2023

Yes, of course! 😄

Happy holidays and Merry Christmas!

Copy link
Owner

@jimlambrt jimlambrt left a comment

Choose a reason for hiding this comment

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

This looks great. Would you mind rebase on main so CI will be happy before we approve and merge?

@jimlambrt
Copy link
Owner

@xunleii I know things are always busy after the holidays, but I'm just following up about rebasing this so we can merge it.

@xunleii
Copy link
Contributor Author

xunleii commented Jan 6, 2024

@xunleii I know things are always busy after the holidays, but I'm just following up about rebasing this so we can merge it.

Hi, sorry, I've seen your message but totally forgot just after.
I'll try to do that as soon as possible (this night or tomorrow)

Thanks for the reminder 😄

Following some LDAP UI and Wireshark, these two elements should not be
found inside a SearchResponseEntry (matchedDN doesn't make sense on this
kind of message).

Also, following https://ldap.com/ldapv3-wire-protocol-reference-search/,
I didn't found anything about them:

```
SearchResultEntry ::= [APPLICATION 4] SEQUENCE {
     objectName      LDAPDN,
     attributes      PartialAttributeList }

PartialAttributeList ::= SEQUENCE OF
       partialAttribute PartialAttribute

LDAPDN ::= LDAPString
           -- Constrained to  [RFC4514]

LDAPString ::= OCTET STRING -- UTF-8 encoded,
              -- [ISO10646] characters

PartialAttribute ::= SEQUENCE {
     type       AttributeDescription,
     vals       SET OF value AttributeValue }

AttributeDescription ::= LDAPString
           -- Constrained to
           -- [RFC4512]

AttributeValue ::= OCTET STRING
```
Copy link
Owner

@jimlambrt jimlambrt left a comment

Choose a reason for hiding this comment

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

ty!

@jimlambrt jimlambrt merged commit a87f8ca into jimlambrt:main Jan 15, 2024
4 checks passed
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.

None yet

2 participants