Skip to content

Commit 9c5c336

Browse files
committed
Fix calculation of the resource part in "files" URIs not featuring trailing slash
Without the `/` at the end of e.g. `/v1/<tenant>/files/export`, the value of the `resource` attribute in preparation for handling the request with `FileRequestHandler`, becomes identical to the URI. While not critical with current code, the condition does then cascade into additional computation in e.g. `any_path_islink` (and potentially other places that depend on the value of the `resource` attribute), and there's been cases of `any_path_islink` raising errors which would abort the request. Again, it's a matter of interpretation whether such requests where `any_path_islink` aborts, should be aborted since no listing would succeed for these anyway, but in any case I thought the `resource` attribute should be computed "correctly". The latter in quotes because I can't be sure there's not intention for the attribute to ever have valid values like `v1/<tenant>/files/export`, but what this change has going for it is that `resource` value will be the same between URIs that only differ by presence of the terminating slash. And if `resource` should bear empty string value for `/v1/<tenant/files/export` URIs _without_ the trailing slash, then one could argue it shouldn't in the very least be `/v1/<tenant>/files/export` for URIs _with_ the trailing slash, an assertion which holds for this change. This correction should also prove beneficial longer term -- with the rest of the code that may potentially depend on "sane" values of `resource`. Of course all this hinges on allowing URIs like `.../export` _without_ the trailing slash, but [current documentation lists these explicitly](http://github.com/unioslo/tsd-api-docs/blob/master/integration/file-api.md#simple-file-download), so I assume they are indeed allowed. An alternative is adding the trailing slash with e.g. a reverse proxy ahead of the API, so the API can simply assume the trailing slash. But since the API is more of an authority on the semantics and category of the URIs that, after all, originate with it, I thought it's more fitting to be handling this in the API and not in a reverse proxy.
1 parent a3efc8e commit 9c5c336

File tree

1 file changed

+1
-1
lines changed

1 file changed

+1
-1
lines changed

tsdfileapi/api.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1078,7 +1078,7 @@ def prepare(self) -> Optional[Awaitable[None]]:
10781078

10791079
# ensure resource is not reserved
10801080
delimiter = self.endpoint or self.namespace
1081-
resource_parts = uri.split(f"/{delimiter}/")
1081+
resource_parts = re.split(rf"/{delimiter}(?:/|$)", uri)
10821082
resource = resource_parts[-1] if resource_parts else None
10831083
if self.request.method in ("GET", "HEAD", "DELETE"):
10841084
work_dir = self.export_dir

0 commit comments

Comments
 (0)