don't go into an infinite loop when CONTENT_PATH_PREFIX is followed by /
#763
Conversation
|
WARNING!!! This PR is not attached to an issue. In most cases this is not advisable. Please see our PR docs for more information about how to attach this PR to an issue. |
|
Thank you for the contribution! In order to move this forward, we'd need to file an issue. Would you mind doing so: https://pulp.plan.io/projects/pulp/issues/new ? Also, I'm not sure if I fully understand the reproducer so if you could include some steps to reproduce this, that'd be great. |
pulpcore/content/handler.py
Outdated
| @@ -151,7 +151,7 @@ def _base_paths(path): | |||
| tree = [] | |||
| while True: | |||
| base = os.path.split(path)[0] | |||
| if not base: | |||
| if not base or base == '/': | |||
There was a problem hiding this comment.
@daviddavis in the example given this would match in both cases, so I was thinking this would break in too many cases. I didn't run it though, maybe in running it it makes sense?
There was a problem hiding this comment.
On chat @daviddavis explained that the example isn't indicative of the problem like /pulp/content// is. With an example like /pulp/content// I can see how this is solving the problem.
|
This works for me. Originally, the content app hangs and now it returns 404 when I hit However, the problem is not fully fixed. I think we need to check for 1 to N number of slashes. |
|
@daviddavis thanks for catching that, and thanks to that catch I realised we can just do commit amended to do so. |
|
Awesome, thanks. This looks good to me. In order to get the CI passing, you have to add "fixes #7066" to your commit message and a changelog entry ( |
|
This revision looks good to me. |
|
Hey @SimonPe. We'd love to get this merged. Do you think you could address the changelog and commit message changes? |
… by `/`
this would cause `path` to start with a `/`
and because
```
>>> os.path.split('/path')
('/', 'path')
>>> os.path.split('/')
('/', '')
```
`base` would never end up as `None` but stay as `/` resulting in an infinite loop.
fixes #7066
|
sorry, was a bit busy with other priorities the last couple of weeks, should be fine now. |
this would cause
pathto start with a/and because
basewould never end up asNonebut stay as/resulting in an infinite loop.Please be sure you have read our documentation on creating PRs:
https://docs.pulpproject.org/contributing/pull-request-walkthrough.html