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

Incorrect headers being set on DELETE request when using fetchUtils from ra-data-simple-rest #5452

Closed
rahulbhanushali opened this issue Oct 28, 2020 · 1 comment · Fixed by #5568
Labels

Comments

@rahulbhanushali
Copy link

What you were expecting:

DELETE request should be not fired with Content-Type header set to application/json.

What happened instead:

DELETE request is being fired with Content-Type header set to application/json.
DELETE requests are usually fired with empty body and that is happening correctly with ra-data-simple-rest as well but the only problem is Content-Type request header is being set which shouldn't be case.
This fails with frameworks like fastify. Related discussion here: fastify/fastify#297 (comment).

Steps to reproduce:

Delete an entity from the UI and observe the HTTP request headers.

Other information:

This is happening mainly because of the assumption in the code that all requests should be fired with Content-Type set to application/json in the fetch.ts file.

export const createHeadersFromOptions = (options: Options): Headers => {
    const requestHeaders = (options.headers ||
        new Headers({
            Accept: 'application/json',
        })) as Headers;
    if (
        !requestHeaders.has('Content-Type') &&
        !(options && (!options.method || options.method === 'GET')) &&
        !(options && options.body && options.body instanceof FormData)
    ) {
        requestHeaders.set('Content-Type', 'application/json');
    }
    if (options.user && options.user.authenticated && options.user.token) {
        requestHeaders.set('Authorization', options.user.token);
    }

    return requestHeaders;
};

Environment

  • React-admin version: 3.9.4
  • Last version that did not exhibit the issue (if applicable):
  • React version: 16.14
  • Browser: Chrome
  • Stack trace (in case of a JS error):
@fzaninotto
Copy link
Member

I'd say that it's not really incorrect - the HTTP standard doesn't force any content type for responses with an empty body. I'd say that a strict interpretation of the HTTP standard by some server libraries have them decode the body as soon as the content-type says the content is JSON, but that's their problem.

That being said, it doesn't do any harm to change the content-type header to text/plain for delete in ra-data-simple-rest.

I'm marking this as a bug, feel free to submit a PR to fix it!

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

Successfully merging a pull request may close this issue.

2 participants