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

Remove creating double slash in url #327

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

matesaki
Copy link

@matesaki matesaki commented May 3, 2022

Merging domain url and path url creates double slash
which causes on localhost error 404 (Page not found).

Signed-off-by: Martin Sakin [email protected]

@@ -215,6 +215,8 @@ def get_connection(self, method, url, body, headers):
sep = '/'
else:
sep = ''
if self.url.endswith('/') and url.startswith('/'):
Copy link
Member

Choose a reason for hiding this comment

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

what url are you trying to parse here?

Copy link
Member

Choose a reason for hiding this comment

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

@matesaki i'm mainly puzzled why the code above does work.
also add a unittest for this

Copy link
Author

Choose a reason for hiding this comment

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

Condition above check if there is no slash before merging. If true, add one.
This new condition check if there are to many slashes. If true, remove one.

It may be confusing because there are two variables url, first Ends with '/' and second Starts with '/'.

Trying to make a test ...

Copy link
Author

Choose a reason for hiding this comment

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

don't know how to test it

Copy link
Member

@stdweird stdweird May 5, 2022

Choose a reason for hiding this comment

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

i think the whole code block, incl the previous lines can become

url = url.lstrip('/')
if self.url.endswith('/'):
    sep = ''
else:
    sep = '/'

Copy link
Author

Choose a reason for hiding this comment

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

yes, this is equivalent
(rebased)

@matesaki
Copy link
Author

matesaki commented May 4, 2022

When I running scripts form vsc-ugent.git, then in get_connection() function I got:
self.url: http://localhost:8000/api/
sep:
url: /monitoring/time/1651612143/
... and merging creates: http://localhost:8000/api//monitoring/time/1651612143/
This leads to error 404 (Page not found).
I'm running vsc-accountpage localy with DEBUG mode, so, maybe it is more sensitive.

Merging domain url and path url create double slash
which causes on localhost error 404 (Page not found).

Signed-off-by: Martin Sakin <[email protected]>
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