search

bluzi / jsonstore

:rocket: jsonstore offers a free and secured JSON-based cloud datastore for small projects | Inactive
https://www.jsonstore.io/
MIT License
2.03k stars 74 forks source link

Internal Server Error if Referer is missing in request to /get-link #10

Closed peterhellberg closed 6 years ago

peterhellberg commented 6 years ago

I think that the use of the Referer header in the backend code should be removed completely.

curl www.jsonstore.io/get-link
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="utf-8">
<title>Error</title>
</head>
<body>
<pre>Internal Server Error</pre>
</body>
</html>

The backend should not depend on the client to send a valid Referer header:

curl -H 'Referer: http://bogus.url' www.jsonstore.io/get-link
{"link":"http://bogus.url/fbbee72d1ac49a589480c578963aa13f71d53416510bdfd87974f300aa886282/"}
bluzi commented 6 years ago

Thanks for reporting, will have a look soon.

medakk commented 6 years ago

It is better to use Host to construct the link, instead of Referer. Host is mandatory in HTTP/1.1.

Also, it think it would be useful to have an optional prefix to all the URLs, so someone can host jsonstore at a different endpoint other than the base, like http://www.example.com/jsonstore/get-link

peterhellberg commented 6 years ago

I don't think that any header should be used. Instead the base URL should be loaded from an environment variable. https://12factor.net/config

(With the default value being https://www.jsonstore.io/)

medakk commented 6 years ago

On second thoughts, I don't see the point in returning an entire URL here. Just the key would suffice, like

$ curl www.jsonstore.io/get-key
{"key":"fbbee72d1ac49a589480c578963aa13f71d53416510bdfd87974f300aa886282"}

Returning a complete URL would make sense in something like pagination.

peterhellberg commented 6 years ago

The point of this endpoint is to construct the full URL for copying from the form on https://www.jsonstore.io/ so there has to be logic to construct it somewhere at least.

medakk commented 6 years ago

That logic can be moved to the client, which is aware of the endpoint at which the API exists.

akshendra commented 6 years ago

Using an environment variables seems like a good idea. We can bootstrap that variable (base url) into the client while building for different environments.

bluzi commented 6 years ago

I agree that if this endpoint will be used by external services and not just by the client of the front page, it probably shouldn't return a URL based on the referrer.

I'm voting for the idea of having an environment variable, if anyone wants to make a PR, feel free.

akshendra commented 6 years ago

@bluzi thinking about using a .env, moving firebase config into it as well.

bluzi commented 6 years ago

@akshendra solved this by changing /get-link to /get-token. It's already deployed.