You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@libcloud.apache.org by GitBox <gi...@apache.org> on 2020/12/19 11:12:16 UTC
[GitHub] [libcloud] Kami opened a new pull request #1531: Allow duplicates slashes in request URL paths
Kami opened a new pull request #1531:
URL: https://github.com/apache/libcloud/pull/1531
This pull request adds a workaround / fix for the issue reported in #1529.
## Background, Context
It appears that in the past Libcloud supported URL paths with double slashes (e.g. ``/my-bucket/foo//1.txt``), but once moving to requests we added some code for path sanitization so we don't support that use case anymore.
It appears that change was originally added in - fedace00261bcbb0a45bda003b968101a897d4ed.
From technical perspective, I think it should be fine to support double slashes since that's usually handled / stripped on the server side (e.g. either inside the web server or inside the app), but I'm sure there was a valid reason for that change - with so many providers we support we've seen all kind of weird API issues and implementation so I assume that change was added to guard against such issues.
## Proposed Fix / Solution
I tried a couple of things as a possible fix / workaround.
It seems like that doing no path sanitization brakes a whole lot of other things so in the end I decided to go with a module level variable which can be changed by the user.
This should be much safer change since it won't affect anyone but users who explicitly opt-in.
I've done some testing and it appears to be working correctly end to end, but there could potentially still be edge cases hiding in the other parts of the code base.
## TODO
- [ ] Upgrade notes entry
- [ ] Documentation entry (S3 driver)
Resolves #1529
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [libcloud] codecov-io edited a comment on pull request #1531: Allow duplicates slashes in request URL paths
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #1531:
URL: https://github.com/apache/libcloud/pull/1531#issuecomment-748465400
# [Codecov](https://codecov.io/gh/apache/libcloud/pull/1531?src=pr&el=h1) Report
> Merging [#1531](https://codecov.io/gh/apache/libcloud/pull/1531?src=pr&el=desc) (b67de60) into [trunk](https://codecov.io/gh/apache/libcloud/commit/f35f7f09df7235125d8cc8b21dbc8dd75f3903e8?el=desc) (f35f7f0) will **increase** coverage by `0.00%`.
> The diff coverage is `100.00%`.
[](https://codecov.io/gh/apache/libcloud/pull/1531?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## trunk #1531 +/- ##
=======================================
Coverage 83.07% 83.07%
=======================================
Files 394 394
Lines 84718 84736 +18
Branches 9000 9001 +1
=======================================
+ Hits 70381 70397 +16
- Misses 11273 11274 +1
- Partials 3064 3065 +1
```
| [Impacted Files](https://codecov.io/gh/apache/libcloud/pull/1531?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [libcloud/common/base.py](https://codecov.io/gh/apache/libcloud/pull/1531/diff?src=pr&el=tree#diff-bGliY2xvdWQvY29tbW9uL2Jhc2UucHk=) | `90.57% <100.00%> (+0.08%)` | :arrow_up: |
| [libcloud/test/test\_connection.py](https://codecov.io/gh/apache/libcloud/pull/1531/diff?src=pr&el=tree#diff-bGliY2xvdWQvdGVzdC90ZXN0X2Nvbm5lY3Rpb24ucHk=) | `98.25% <100.00%> (+0.09%)` | :arrow_up: |
| [libcloud/test/dns/test\_base.py](https://codecov.io/gh/apache/libcloud/pull/1531/diff?src=pr&el=tree#diff-bGliY2xvdWQvdGVzdC9kbnMvdGVzdF9iYXNlLnB5) | `97.01% <0.00%> (-2.99%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/libcloud/pull/1531?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/libcloud/pull/1531?src=pr&el=footer). Last update [f35f7f0...b67de60](https://codecov.io/gh/apache/libcloud/pull/1531?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [libcloud] asfgit merged pull request #1531: Allow duplicates slashes in request URL paths
Posted by GitBox <gi...@apache.org>.
asfgit merged pull request #1531:
URL: https://github.com/apache/libcloud/pull/1531
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [libcloud] codecov-io commented on pull request #1531: Allow duplicates slashes in request URL paths
Posted by GitBox <gi...@apache.org>.
codecov-io commented on pull request #1531:
URL: https://github.com/apache/libcloud/pull/1531#issuecomment-748465400
# [Codecov](https://codecov.io/gh/apache/libcloud/pull/1531?src=pr&el=h1) Report
> Merging [#1531](https://codecov.io/gh/apache/libcloud/pull/1531?src=pr&el=desc) (e6efd8b) into [trunk](https://codecov.io/gh/apache/libcloud/commit/f35f7f09df7235125d8cc8b21dbc8dd75f3903e8?el=desc) (f35f7f0) will **increase** coverage by `0.00%`.
> The diff coverage is `100.00%`.
[](https://codecov.io/gh/apache/libcloud/pull/1531?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## trunk #1531 +/- ##
=======================================
Coverage 83.07% 83.08%
=======================================
Files 394 394
Lines 84718 84736 +18
Branches 9000 9001 +1
=======================================
+ Hits 70381 70399 +18
Misses 11273 11273
Partials 3064 3064
```
| [Impacted Files](https://codecov.io/gh/apache/libcloud/pull/1531?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [libcloud/common/base.py](https://codecov.io/gh/apache/libcloud/pull/1531/diff?src=pr&el=tree#diff-bGliY2xvdWQvY29tbW9uL2Jhc2UucHk=) | `90.57% <100.00%> (+0.08%)` | :arrow_up: |
| [libcloud/test/test\_connection.py](https://codecov.io/gh/apache/libcloud/pull/1531/diff?src=pr&el=tree#diff-bGliY2xvdWQvdGVzdC90ZXN0X2Nvbm5lY3Rpb24ucHk=) | `98.25% <100.00%> (+0.09%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/libcloud/pull/1531?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/libcloud/pull/1531?src=pr&el=footer). Last update [f35f7f0...e6efd8b](https://codecov.io/gh/apache/libcloud/pull/1531?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org