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

error code for expired file access #379

Closed
soxofaan opened this issue Apr 14, 2021 · 7 comments · Fixed by #380
Closed

error code for expired file access #379

soxofaan opened this issue Apr 14, 2021 · 7 comments · Fixed by #380

Comments

@soxofaan
Copy link
Member

@laxiwuka is working on implementing signed result download urls (Open-EO/openeo-python-driver#50)

he is also incorporating expiring of download urls, but as far as we know there is no dedicated error code to return when requesting a file that is expired.

Can we add some kind of FileExpired error code?

@m-mohr
Copy link
Member

m-mohr commented Apr 14, 2021

This is intentionally not in the API spec, because these signed URLs could be hosted anywhere and the endpoint where the files reside is part of the API, too. Like VITO you could store the results on a S3 bucket and there you can't provide a specific error code and the files will likely just disappear (HTTP 404). So we could only recommend an error code, but clients can't rely on it so the purpose of a pre-defined error code may be limited (i.e. back-ends can just make a custom one). If you still think it's useful, we could add a recommended error code, I'm not advocating against having one, but I'm also not seeing a real advantage of having one.

@soxofaan
Copy link
Member Author

soxofaan commented Apr 14, 2021

In practice all error codes are just recommendations anyway, right? A sloppy backend can choose to raise "500 Internal" everywhere where a better choice is available, and still be "compliant":

* The `code` is either one of the [standardized textual openEO error codes](errors.json) or a proprietary error code.

The standardized error codes are mainly about making the development/debug/maintenance experience better, even if they are un-handled by a client. In that sense it is useful to define this error code, I think.

Which alternative would you recommend?
A custom error with status 403 (forbidden) or 410 (Gone), code FileExpired and message like File access has expired for {path}?

@m-mohr
Copy link
Member

m-mohr commented Apr 14, 2021

Yes, correct.

I think I'd define it as follows:

	"LinkExpired": {
		"description": "The signed URLs for batch job results have expired. Please send a request to `GET /jobs/{job_id}/results` to refresh the links.",
		"message": "The links to the batch job results have expired. Please request the results again.",
		"http": 410,
		"tags": [
			"Batch Jobs"
		]
	},

and mention it as recommendation in the GET /jobs/{job_id}/results endpoint.

I realized just now, that the API doesn't say what to do after expiration. I guess it's implicitly clear that GET /jobs/{job_id}/results always returns valid links so that re-requesting that endpoint returns new links if required, but it's not clearly stated.

@soxofaan
Copy link
Member Author

Interesting, I didn't consider that you can just "refresh" the urls by calling .../results again. That would actually be a reason to use a relatively small expiry timespan (couple of minutes) to improve security. If so, that would be interesting to explicitly add as hint or recommendation in the spec as you mention.

@m-mohr
Copy link
Member

m-mohr commented Apr 14, 2021

see PR #380

I wouldn't do a too short timespan. Usecase is that you can read it with STAC clients, e.g. in QGIS. If that expires too fast, it will get annoying. So I'd do at least an hour...

@soxofaan
Copy link
Member Author

sure, order of magnitude of hours is fine too.
I originally had days or weeks in mind for those urls.

@m-mohr
Copy link
Member

m-mohr commented Apr 14, 2021

Days or weeks should also be fine, it works for Google Docs, too.
But restricting it to some hours (maybe 8 hours for a working day ;-) ) is probably also fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants