search

hpcugent / vsc-base

Basic Python libraries used by UGent's HPC group
Other
14 stars 51 forks source link

URL encode the domain and path of URLs #305

Closed lexming closed 3 years ago

lexming commented 3 years ago

This PR adds URL encoding of the domain and path of URLs passed to the rest api. Currently, it only URL encodes the query string of URLs. However, it is possible that the rest client receives non conformant URLs and those should be encoded as well to avoid unnecessary exceptions from urllib.

Specifically, this is an issue with group names in the VSC account page that have white spaces, for instance leuvenall-fluent group, as those names can be part of the path of URLs. The tool sync_django_ldap from vsc-administration fails to complete the LDAP sync due to this issue.

boegel commented 3 years ago

@lexming Tests are failing in Jenkins with:

$ pip3 install --ignore-installed --user tox
...
PermissionError: [Errno 13] Permission denied: '/var/lib/jenkins/.local/lib'
script returned exit code 2

That's because Jenkins is using the Jenkinsfile from current master rather than the one updated in the PR, because you're not a member of the team that has write access to this repo (so you're on "untrusted" contributor)... :-/

boegel commented 3 years ago

@lexming Can we cover this change in test/rest.py?

lexming commented 3 years ago

Hold on this a moment, this fixes the issues but it might be for the wrong reasons.

lexming commented 3 years ago

One issue in the VSC rest API has been already solved and another one is in the hands of vsc-tech. So we can proceed with this PR, the fix here still applies and the domain and path of the URL have to be URL encoded because there are entities in the data base with white spaces in their names (and potentially any other non-conformant character with URLs).

@boegel I thought about a way to test this, but it is not trivial. Current tests use the rest API of github, but the problem there is that there are no entries in github encoded with the standard % codes. It replaces everything in the URL with dashes. So creating a specific wiki page named "Test rest API", will be converted to "test-rest-api". Using the VSC account page is probably out of the table due to the authentication requirements. Do you have any other idea?

stdweird commented 3 years ago

@lexming use mock?

lexming commented 3 years ago

This issue will be solved in the interface of the VSC account page by not allowing non-conformant group names. Therefore, this PR is not needed.