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

Datasette should serve Access-Control-Max-Age #2079

Closed
simonw opened this issue May 25, 2023 · 8 comments
Closed

Datasette should serve Access-Control-Max-Age #2079

simonw opened this issue May 25, 2023 · 8 comments

Comments

@simonw
Copy link
Owner

simonw commented May 25, 2023

Currently the CORS headers served are:

def add_cors_headers(headers):
headers["Access-Control-Allow-Origin"] = "*"
headers["Access-Control-Allow-Headers"] = "Authorization, Content-Type"
headers["Access-Control-Expose-Headers"] = "Link"
headers["Access-Control-Allow-Methods"] = "GET, POST, HEAD, OPTIONS"

Serving Access-Control-Max-Age: 600 would allow browsers to cache that for 10 minutes, avoiding additional CORS pre-flight OPTIONS requests during that time.

@simonw
Copy link
Owner Author

simonw commented May 25, 2023

Also need to update this documentation:

datasette/docs/json_api.rst

Lines 453 to 465 in 9584879

Enabling CORS
-------------
If you start Datasette with the ``--cors`` option, each JSON endpoint will be
served with the following additional HTTP headers::
Access-Control-Allow-Origin: *
Access-Control-Allow-Headers: Authorization, Content-Type
Access-Control-Expose-Headers: Link
Access-Control-Allow-Methods: GET, POST, HEAD, OPTIONS
This allows JavaScript running on any domain to make cross-origin
requests to interact with the Datasette API.

Or maybe make that automated via cog.

@simonw
Copy link
Owner Author

simonw commented May 25, 2023

I'm going with 3600 for 1 hour instead of 600 for 10 minutes.

@simonw
Copy link
Owner Author

simonw commented May 25, 2023

I ran this on https://www.example.com/ twice using the console:

fetch(
  `https://latest.datasette.io/ephemeral/foo/1/-/update`,
  {
    method: "POST",
    mode: "cors",
    headers: {
      Authorization: `Bearer tok`,
      "Content-Type": "application/json",
    },
    body: JSON.stringify({update: {blah: 1}}),
  }
)
  .then((r) => r.json())
  .then((data) => {
    console.log(data);
  });

And got this in the network pane:

image

@simonw simonw closed this as completed in b49fa44 May 25, 2023
@simonw
Copy link
Owner Author

simonw commented May 25, 2023

@simonw
Copy link
Owner Author

simonw commented May 25, 2023

Weird... after the deploy went out:

image

But the request did indeed get the new header:

image

So I'm not sure why it's making multiple POST requests like that.

Maybe it's because the attempted POST failed with a 404?

@simonw
Copy link
Owner Author

simonw commented May 25, 2023

https://fetch.spec.whatwg.org/#http-access-control-max-age says:

Indicates the number of seconds (5 by default) the information provided by the Access-Control-Allow-Methods and Access-Control-Allow-Headers headers can be cached.

So there was already a 5s cache anyway.

@simonw
Copy link
Owner Author

simonw commented May 25, 2023

Mystery solved as to why I wasn't seeing this work:

CleanShot 2023-05-25 at 15 38 32@2x

I had "Disable Cache" checked!

I ran this experiment after un-checking that box:

fetch('https://latest.datasette.io/ephemeral/foo/1/-/update', {
    method: 'POST',
    headers: {
        'Content-Type': 'application/json'
    },
    body: JSON.stringify({ test: 'test' })
});
// And run it again
fetch('https://latest.datasette.io/ephemeral/foo/1/-/update', {
    method: 'POST',
    headers: {
        'Content-Type': 'application/json'
    },
    body: JSON.stringify({ test: 'test' })
});
// Now try a thing that doesn't serve that max-age header yet:
fetch('https://latest-with-plugins.datasette.io/ephemeral/foo/1/-/update', {
    method: 'POST',
    headers: {
        'Content-Type': 'application/json'
    },
    body: JSON.stringify({ test: 'test' })
});
// And a second time but within 5s
fetch('https://latest-with-plugins.datasette.io/ephemeral/foo/1/-/update', {
    method: 'POST',
    headers: {
        'Content-Type': 'application/json'
    },
    body: JSON.stringify({ test: 'test' })
});
// Third time after waiting longer than 5s
fetch('https://latest-with-plugins.datasette.io/ephemeral/foo/1/-/update', {
    method: 'POST',
    headers: {
        'Content-Type': 'application/json'
    },
    body: JSON.stringify({ test: 'test' })
});
// Try that original one again - still within the 1hr cache time
fetch('https://latest.datasette.io/ephemeral/foo/1/-/update', {
    method: 'POST',
    headers: {
        'Content-Type': 'application/json'
    },
    body: JSON.stringify({ test: 'test' })
});

The results show that the cache of 1hr was being obeyed for latest.datasette.io while the latest-with-plugins.datasette.io default cache of 5s was being obeyed too.

image

@simonw
Copy link
Owner Author

simonw commented May 25, 2023

Wrote this up as a TIL: https://til.simonwillison.net/http/testing-cors-max-age

simonw added a commit that referenced this issue Aug 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant