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

Compilation of the FileRequestHandler.resource may yield bad values for "v1/<tenant>/files/export" (no slash at the end) URIs #256

Open
amn opened this issue Oct 10, 2024 · 0 comments · May be fixed by #259

Comments

@amn
Copy link
Contributor

amn commented Oct 10, 2024

The API may abort a request for /v1/<tenant>/files/export because utils.any_path_islink called at https://github.com/unioslo/tsd-file-api/blob/v2.17.0/tsdfileapi/api.py#L1774 calling path.is_symlink in at least the first iteration of its while path != path.parent loop, may raise a "permission denied" error.

This doesn't happen for every case of backing storage directory, for some reason -- for TSD the argument to the aforementioned utils.any_path_islink call with the value of '/ess/projects02/p1337/data/durable/file-export/v1/p1337/files/export' succeeds (returns False), with request succeeding, while for the value of '/ess/projects03/p694/data/durable/file-export/v1/p694/files/export' raises as described above.

(cases of the raised error are logged with https://github.com/unioslo/tsd-file-api/blob/v2.17.0/tsdfileapi/api.py#L1869, but the stack trace is ordinarily not recoverable from the logs, so for the sake of troubleshooting I had to temporarily change the implicated logger.error call to logger.exception, so perhaps we can do another round of replacing logger.error with logger.exception which exposed the original cause)

If we disregard the curiosity of the backing storage effectively causing path.is_symlink to raise in some cases of otherwise equivalent paths, the issue, I think, is caused by the compilation of the resource attribute for the FileRequestHandler object, in the prepare method, starting at https://github.com/unioslo/tsd-file-api/blob/v2.17.0/tsdfileapi/api.py#L1081.

For URIs like .../export without the trailing slash, resource ends up with the value that equals the URI, in fact the pass statement is executed at https://github.com/unioslo/tsd-file-api/blob/v2.17.0/tsdfileapi/api.py#L1090. This is the case in TSD for both p694 and p1337.

If resource is meant to be the [possibly optional] part following /v1/<tenant>/files/export, then URIs either must feature a trailing slash or I suppose the compilation of resource must be amended to support /v1/<tenant>/files/export with no trailing slash.

The documentation seems to allow ../export for listing exportables, without the trailing slash in the URI: https://github.com/unioslo/tsd-api-docs/blob/master/integration/file-api.md#simple-file-download

In summary, I think solving this should be about amending how resource is compiled because that's where the trouble starts, regardless of the "weird" backing storage behaviour.

Sorry for the copious description, didn't have the time to boil it down to the essentials, at least there should be enough detail (I hope!).

@amn amn changed the title Compilation of the FileRequestHandler.resource may yield bad values for some "export" URIs Compilation of the FileRequestHandler.resource may yield bad values for "v1/<tenant>/files/export" (no slash at the end) URIs Oct 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant