Repository navigation
Disallow reading from external datasets - #129
Conversation
|
I'll investigate the CI failure. It is linked to Sphinx but the tests pass. |
t20100
left a comment
t20100
left a comment
There was a problem hiding this comment.
I would use 403 error code, otherwise LGTM!
| except NotSupportedError as e: | ||
| raise create_error(501, str(e)) |
There was a problem hiding this comment.
Why not using 403 Forbidden?
https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Status/403:
The HTTP 403 Forbidden client error response status code indicates that the server understood the request but refused to process it. This status is similar to 401, except that for 403 Forbidden responses, authenticating or re-authenticating makes no difference. The request failure is tied to application logic, such as insufficient permissions to a resource or action.
https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Status/501:
If you have visited a web page and you are seeing 501 errors, these issues require investigation and fixing by server owners or administrators.
| - `safe`: Convert to a type supported by JS typedarray (https://developer.mozilla.org/fr/docs/Web/JavaScript/Reference/Global_Objects/TypedArray) | ||
| """ | ||
| if self._h5py_entity.external: | ||
| raise NotSupportedError("Cannot resolve external datasets") |
axelboc
left a comment
axelboc
left a comment
There was a problem hiding this comment.
I agree with using 403 👍
fe8043e to
c7285fc
Compare
c7285fc to
d575f13
Compare
39d7218 to
9d9142d
Compare
External datasets are a scary HDF5 feature in terms of security. Also, they don't seem used in all the usecases we encountered.
I therefore propose that we completely disallow reading from external datasets in h5grove. The metadata can be served but serving the data (and therefore resolving the external dataset) via
/datawill now raise a specific error./statswill also raise an error since it needs to get the data to compute stats.🤖 Claude used to generate the tests and update the
api.yaml