Transport: When 'Accept' header is stripped - the migration process silently stops.
Opened this issue · 3 comments
When 'Accept' header is stripped - the migration process silently stops.
On our server we stripped the 'Accept' header from requests which caused the migration via the transport plugin to silently stop.
The server.class.php (in the transport-extention) determines it can't stream, because of the missing header, but the front-end still expects a stream-response. The backend sends a json response which the front-end can't handle which causes an error in the console: 'EventSource's response has a MIME type ("application/json") that is not "text/event-stream". Aborting the connection.'
Possible things to 'fix' this:
- Check before migration starts if server support streaming: if not show a clear error on this.
- Add better debugging so the front-end can get more information on why migration failed.
Why are Accept headers stripped? They are a standard way for browsers and servers to communicate intent and support: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Accept. With it omitted, applications can behave unexpectedly, sometimes not as obvious as what you've experienced with Transport.
The server can handle any response; but, if the browser supports the newer one, yet the server is unaware, the server will try the old model while the browser expects the new model.
Two things:
- We may not want to add fallbacks if there is a standard method that's broken without good reason.
- If the return type is the legacy version, we may want to try unpack that and send the message to the Logger. Still, I'm not sure if browsers allow unpacking of unexpected MIME type responses in streams.
That said, I did want to test if the server supports streaming beforehand, but then to determine the chunk buffer size -- however, that returned false numbers too often, so I left it out (didn't even commit the code).
Fair enough
We are using a 'whitelist' approach on which headers are forwarded to the webserver, and intentional or not but the 'Accept' header wasn't whitelisted in this case ~ it is now.
This issue was mostly just a heads-up, ie. 'hey we noticed this'.
As it feels like weird behaviour when the plugin fallsback to json-communication, even though the frontend doesn't work with it.
Maybe simply throw a hard exception then? But up to you ofcourse as it is a freak-occurance
I'll probably drop the legacy handler since practically all browsers support the event streams: https://caniuse.com/eventsource.
That'll remove the negotiation between the browser and the server, so it'll always return a stream whether the Accept header is read or not.
That'd make a fix for #626 obsolete, but I'll still push that through for now. I may want to use the datastore in the future when we gracefully handle connection drops.
About the error you saw in the console: I cannot move that to the logger because it is unreachable; it is invoked by the browser, which restricts further access:
If I'd log event from https://github.com/sybrew/The-SEO-Framework-Extension-Manager/blob/2.6.1/extensions/free/transport/trunk/lib/js/sse.worker.js#L84-L88.
I'll get the following, which contains nothing useful:
The issue remains a corner case, but I'll leave this open for when I find new information (or remove legacy support altogether).