Skip to content

fix: remove matchedDN and diagnosticMessage from SearchResponseEntry#58

Merged
jimlambrt merged 1 commit intojimlambrt:mainfrom
xunleii:main
Jan 15, 2024
Merged

fix: remove matchedDN and diagnosticMessage from SearchResponseEntry#58
jimlambrt merged 1 commit intojimlambrt:mainfrom
xunleii:main

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
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.

2 participants