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

asText fails when we try to print the filter #226

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

grk-it
Copy link

@grk-it grk-it commented May 10, 2021

  • asText function errors out when we try to print the human friendly text for the filters. This commit tries to fix the same.
  • Updated tests related to the above change.

Context:

Steps to reproduce:

  1. Implemented the simple ldapserver as mentioned in the following blog:
    http://tonthon.blogspot.com/2011/02/ldaptor-ldap-with-twisted-server-side.html

Here's the contents of the folder ldaptor-example:

(venv) ➜  ldaptor-example ls
schema.py   server.py   venv

Python version:

(venv) ➜  ldaptor-example python --version
Python 3.8.0

To start the server:

(venv) ➜  ldaptor-example python server.py
2021-05-10 15:24:24-0500 [-] Log opened.
2021-05-10 15:24:24-0500 [-] LDAPServerFactory starting on 8080
2021-05-10 15:24:24-0500 [-] Starting factory <__main__.LDAPServerFactory object at 0x10fa13a60>
  1. Added the following lines in the file venv/lib/python3.8/site-packages/ldaptor/protocols/ldap/ldapserver.py:
265             reply(
266                 pureldap.LDAPSearchResultEntry(
267                     objectName=entry.dn.getText(),
268                     attributes=filtered_attribs,
269                 )
270             )
271         print(type(request.filter)) # <<<
272         print(request.filter.toWire())  # <<<
273         print(request.filter.asText()) # <<<
274         d = base.search(
275             filterObject=request.filter,
276             attributes=request.attributes,
277             scope=request.scope,
278             derefAliases=request.derefAliases,
279             sizeLimit=request.sizeLimit,
280             timeLimit=request.timeLimit,
281             typesOnly=request.typesOnly,
282             callback=_sendEntryToClient,
283         )
  1. Issued the following search:
➜  ~ ldapsearch -x -h 127.0.0.1 -p 8080  'uid=esteban'
# extended LDIF
#
# LDAPv3
# base <> (default) with scope subtree
# filter: uid=esteban
# requesting: ALL
#

# search result
search: 2
result: 80 Other (e.g., implementation specific) error
text: can only concatenate str (not "bytes") to str

# numResponses: 1

Here's the backtrace on the server side:

(venv) ➜  ldaptor-example python server.py
2021-05-10 15:31:40-0500 [-] Log opened.
2021-05-10 15:31:40-0500 [-] LDAPServerFactory starting on 8080
2021-05-10 15:31:40-0500 [-] Starting factory <__main__.LDAPServerFactory object at 0x10b906f70>
2021-05-10 15:31:42-0500 [-] <class 'ldaptor.protocols.pureldap.LDAPFilter_equalityMatch'>
2021-05-10 15:31:42-0500 [-] b'\xa3\x0e\x04\x03uid\x04\x07esteban'
2021-05-10 15:31:42-0500 [-] Unhandled Error
	Traceback (most recent call last):
	  File "/Users/grk/ldaptor-example/venv/lib/python3.8/site-packages/twisted/internet/defer.py", line 167, in maybeDeferred
	    result = f(*args, **kw)
	  File "/Users/grk/ldaptor-example/venv/lib/python3.8/site-packages/ldaptor/protocols/ldap/ldapserver.py", line 316, in handle_LDAPSearchRequest
	    d.addCallback(self._cbSearchGotBase, dn, request, reply)
	  File "/Users/grk/ldaptor-example/venv/lib/python3.8/site-packages/twisted/internet/defer.py", line 339, in addCallback
	    return self.addCallbacks(callback, callbackArgs=args, callbackKeywords=kw)
	  File "/Users/grk/ldaptor-example/venv/lib/python3.8/site-packages/twisted/internet/defer.py", line 330, in addCallbacks
	    self._runCallbacks()
	--- <exception caught here> ---
	  File "/Users/grk/ldaptor-example/venv/lib/python3.8/site-packages/twisted/internet/defer.py", line 662, in _runCallbacks
	    current.result = callback(current.result, *args, **kw)
	  File "/Users/grk/ldaptor-example/venv/lib/python3.8/site-packages/ldaptor/protocols/ldap/ldapserver.py", line 294, in _cbSearchLDAPError
	    reason.trap(ldaperrors.LDAPException)
	  File "/Users/grk/ldaptor-example/venv/lib/python3.8/site-packages/twisted/python/failure.py", line 450, in trap
	    self.raiseException()
	  File "/Users/grk/ldaptor-example/venv/lib/python3.8/site-packages/twisted/python/failure.py", line 475, in raiseException
	    raise self.value.with_traceback(self.tb)
	  File "/Users/grk/ldaptor-example/venv/lib/python3.8/site-packages/twisted/internet/defer.py", line 662, in _runCallbacks
	    current.result = callback(current.result, *args, **kw)
	  File "/Users/grk/ldaptor-example/venv/lib/python3.8/site-packages/ldaptor/protocols/ldap/ldapserver.py", line 273, in _cbSearchGotBase
	    print(request.filter.asText())
	  File "/Users/grk/ldaptor-example/venv/lib/python3.8/site-packages/ldaptor/protocols/pureldap.py", line 573, in asText
	    "("
	builtins.TypeError: can only concatenate str (not "bytes") to str

Here's the test report after running the tests in the local (after the fix):

===============================================================================
[SUCCESS!?!]
Reason: 'Not supported yet.'

ldaptor.test.test_server.TestSchema.testSimple
-------------------------------------------------------------------------------
Ran 672 tests in 1.526s

PASSED (unexpectedSuccesses=1, successes=671)
py38-test-dev run-test: commands[3] | coverage report --show-missing
No data to report.

   - `asText` function errors out when we try to print the human friendly text for the filters. This commit fixes the same.
   - Updated tests for the above the change.
@grk-it grk-it marked this pull request as ready for review May 11, 2021 01:06
@psi29a psi29a requested a review from graingert May 17, 2021 08:08
@codecov
Copy link

codecov bot commented May 17, 2021

Codecov Report

Merging #226 (29e26d6) into master (4e06f4e) will decrease coverage by 0.78%.
The diff coverage is 100.00%.

❗ Current head 29e26d6 differs from pull request most recent head 13d1b7b. Consider uploading reports for the commit 13d1b7b to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master     #226      +/-   ##
==========================================
- Coverage   83.28%   82.50%   -0.79%     
==========================================
  Files          87       87              
  Lines       11567    11794     +227     
  Branches     1184     1232      +48     
==========================================
+ Hits         9634     9731      +97     
- Misses       1815     1943     +128     
- Partials      118      120       +2     
Impacted Files Coverage Δ
ldaptor/protocols/pureldap.py 83.61% <100.00%> (-10.96%) ⬇️
ldaptor/test/test_ldapfilter.py 100.00% <100.00%> (ø)
ldaptor/test/test_pureldap.py 100.00% <100.00%> (ø)
ldaptor/test/test_autofill_samba.py 100.00% <0.00%> (ø)
ldaptor/protocols/ldap/distinguishedname.py 89.09% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4e06f4e...13d1b7b. Read the comment docs.

@grk-it
Copy link
Author

grk-it commented May 25, 2021

@psi29a @graingert Could any of you please help me with the approval for running the flow and the review subsequently.

@graingert graingert closed this Jul 9, 2021
@graingert graingert reopened this Jul 9, 2021
@graingert
Copy link
Member

hmm I only have permission to force merge this PR as administrator. I don't have permission to enable CI

@adiroiban
Copy link
Member

I have approved the run now. @graingert I think that you should now be owner of everything and in the future you should be able to approve the CI run for new contributors.

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