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

Add an Accept header for manifests #3770

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jcoyne
Copy link
Collaborator

@jcoyne jcoyne commented Jun 8, 2023

Our IIIF server provides v2 APIs by default, but if the client provides an Accept header with the v3 profile, it will provide a v3 response. This triggers that behavior.

Our IIIF server provides v2 APIs by default, but if the client provides an Accept header with the v3 profile, it will provide a v3 response.  This triggers that behavior.
@jbaiter
Copy link
Collaborator

jbaiter commented Jun 8, 2023

Warning: I just tried to implement this in my own application, and it seems to break compatibility with some IIIF endpoints, most notably that of the Vatican.

The reason for this is, that by setting the Accept header on the fetch Request, a pre-flight OPTIONS request is now sent to the Manifest endpoint. If that endpoint does not respond correctly, the browser is unable to make the request and users won't be able to view the Manifest.

Example: https://digi.vatlib.it/iiif/MSS_Arch.Cap.S.Pietro.C.132/manifest.json

Fetches fine without the Accept header, but breaks with a 403 (without a CORS header on the response) on the pre-flight request that is now sent due to the presence of the custom 'Accept' header.

@jcoyne
Copy link
Collaborator Author

jcoyne commented Jun 8, 2023

@jbaiter can you give me more details about this? I'm not seeing this behavior in a simple test:

<!DOCTYPE html>
<html>
<head>
</head>
<body>
<script>

const run = async function () {
  const url = 'https://digi.vatlib.it/iiif/MSS_Arch.Cap.S.Pietro.C.132/manifest.json'
  console.log('fetch with no headers')
  const result = await fetch(url)
  console.log("done")
  console.log(result)

  console.log('fetch with headers')
  const result2 = await fetch(url, { Accept: 'application/ld+json, application/json' })
  console.log("done")
  console.log(result2.json())

  console.log('fetch with headers')
  const result3 = await fetch(url, { Accept: 'application/ld+json;q=0.9;profile="http://iiif.io/api/presentation/3/context.json", application/ld+json;q=0.7;profile="http://iiif.io/api/presentation/2/context.json"'})
  console.log("done")
  console.log(result3.json())
}
run()
</script>
<p>Look in the console</p>
</body>
</html>

@jbaiter
Copy link
Collaborator

jbaiter commented Jun 8, 2023

Gotcha, I found out why my code was triggering the preflight request: I was using a fetch ponyfill (cross-fetch) that somehow didn't use native fetch in the browser but its own XHR-based polyfill. Since the rules for a "simple request" are stricter for XMLHttpRequests, the fetch was triggering a preflight request in my case.

@jbaiter
Copy link
Collaborator

jbaiter commented Jun 8, 2023

Sorry, but turns out that wasn't it, I'm seeing the same result with native fetch now (latest Firefox).

I think your sample code is not actually setting the Accept header, you have to set the headers value in the fetch options dict, and not pass in the headers as the second argument:

https://kk7jk2.csb.app/
https://codesandbox.io/s/divine-hooks-kk7jk2?file=/index.html

@jbaiter
Copy link
Collaborator

jbaiter commented Jun 9, 2023

I think we're running into this:

If value’s length is greater than 128, then return false.

https://fetch.spec.whatwg.org/#cors-safelisted-request-header

Accept headers are usually safe and don't trigger a preflight request, except if the above condition is met, which is the case, unfortunately :-/
Maybe we can strip down the value so it stays below 128 characters?

-- edit1:

Nope, that's not it either (see updated sandbox).
At least for Chrome/WebKit, the cause seems to be that it considers ; and " to be delimiters according to RFC7230 and considers all Accept header values that contain such a delimiter to be illegal (?!), and consequently to be CORS-unsafe.

If you ask me, this looks like a bug in the browser. the ;profile="..." extension is legal syntax according to RFC7230: https://datatracker.ietf.org/doc/html/rfc7231#section-3.1.1.1.

The main problem here is ofc that the Vatican IIIF implementation is broken since it does not implement the CORS protocol as mandated by the spec, but still, M3 should probably strife for maximum compatibility.
Maybe we can try these requests twice, once with the Accept header, and if that fails, without?

-- edit2:

Scratch all of the above, I wasn't reading the fetch spec closely enough, here's what defines a "CORS-unsafe request header byte":

The quotes around the profile value are optional, but there's no way around the colon, so I guess we'll have to try again to support IIIF endpoints with broken CORS implementations? (thanks @irv ). I'll try stripping the quotes 👍

@irv
Copy link

irv commented Oct 18, 2023

I don't have anything really useful to contribute but @jbaiter isn't is a semicolon ; and not a colon : in the accept header?

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 this pull request may close these issues.

3 participants