Got: Disable method overwriting

[unixfox]

Describe the bug

• Node.js version: v13.1.0
• OS & version: ArchLinux
• Got library version: commit https://github.com/sindresorhus/got/commit/518f0f5906e1fcd38691555b17fb91db643f3d47

When doing a POST request with followAllRedirects: true as an option to an endpoint that redirect with a status code of 302 the library got send a POST request to the new endpoint instead of doing a simple GET request like the request library would do it.
This bug report is related to this issue: https://github.com/sindresorhus/got/issues/568

Actual behavior

Send a POST request on the second request after following the redirect.

HTTPError: Response code 405 (Method Not Allowed)
    at EventEmitter.<anonymous> (/tmp/node_modules/got/dist/source/as-promise.js:110:31)
    at processTicksAndRejections (internal/process/task_queues.js:93:5) {
  name: 'HTTPError'
}
Wrong method on redirect.

Expected behavior

Send a simple GET request on the second request after following the redirect.

200
Good method on redirect.

Code to reproduce

Code with the got library:

const got = require('got');

(async () => {
    try {
        const response = await got("http://localhost:3000", {
            method: "POST",
            form: {
                username: "username",
                password: "password"
            },
            followRedirect: true
        });
        console.log(response.statusCode);
        console.log(response.body);
    } catch (error) {
        console.log(error);
        console.log(error.response.body);
    }
})();

Code with the request library:

const request = require('request');

const options = {
    method: 'POST',
    uri: 'http://localhost:3000',
    form: {
        username: "username",
        password: "password"
    },
    followAllRedirects: true
};

request(options, function(err, res, body) {
    console.log(res.statusCode);
    console.log(body);
});

Express code to test the behavior:

const express = require('express');
const app = express();

app.get('/redirect', (req, res) => {
    res.send('Good method on redirect.');
    console.log('Good method on redirect.');
});

app.post('/redirect', (req, res) => {
    res.status(405).send('Wrong method on redirect.');
    console.log('Wrong method on redirect.');
});

app.post('/', (req, res) => {
    res.redirect(302, "/redirect");
    console.log('Received POST request, doing a redirect...');
});

app.listen(3000);

Checklist

• [x] I have read the documentation.
• [x] I have tried my code with the latest version of Node.js and Got.

[szmarczak]

Actually this is not a bug, as the spec allows method overwriting.

[szmarczak]

@sindresorhus Should we make an option for this?

[szmarczak]

Another solution would be to retry with GET if method overwriting fails.

In other words, it would automatically fallback to GET (I'd go with this).

[unixfox]

I don't really understand why it shouldn't follow the same behavior as a 303.
I just tested by changing res.redirect(302, "/redirect"); to res.redirect(303, "/redirect"); in the Express code and it works like I expected:

200
Good method on redirect.

I mean isn't the issue #568 about to clone the behavior of a 303 to a 302/301?

Why got shouldn't follow the same behavior as request, other web browsers (Firefox, Chrome and so on), axios, node-fetch and probably other alternative libraries to request that I didn't test.

[szmarczak]

Why got shouldn't follow the same behavior as request, other web browsers

These both are spec-compliant. There is nothing wrong with method rewriting as of RFC 7231:

the spec allows method overwriting.

Read more here: https://github.com/sindresorhus/got/issues/568#issuecomment-503451992

[sindresorhus]

@szmarczak Yeah, we could implement an option for this. I think the current default should stay though.