Undici does not like lowercase method names

[dglozic]

Bug Description

When using request with lowercase method (say, put) undici accepts the method name but nginx complains about it and throws error 400.

Reproducible By

Make a request call by explicitly specifying method property as lowercase (say, put).

Expected Behavior

Method names should be case-insensitive.

Logs & Screenshots

Error from nginx proxy when put is used:

2023/04/17 14:48:42 [info] 36173#0: *119 client sent invalid method while reading client request line, client: 127.0.0.1, server: 127.0.0.1:9442, request: "put /v2/user_profiles/johndoe@acme.com/reload HTTP/1.1"

Environment

Using Node.js v18.14.2, and the latest version of undici.

[ronag]

I don't think the spec disallows lower case methods. This looks more like a user error than a bug.

[dglozic]

I am going to try to distill a use case that proves the problem that happens when method is put but goes away when I set it to PUT.

[ronag]

I believe you. But I am not convinced this is a undici bug.

[dglozic]

Let me dig a bit deeper - if it is me I will happily close :-).

[dglozic]

Here is a snippet where I can reproduce it without fail:

const {
	request,
	Agent
} = require("undici");

function testUndici() {
	const token = process.env.TOKEN;
	if (!token) {
		exit("Missing token - exiting");
		return;
	}

	// Making a put
	const options = {
		headers: {
			"Authorization": "Bearer " + token,
			"Accept": "application/json"
		},
		method: "GET",
		dispatcher: new Agent({ connect: { rejectUnauthorized: false } })
	};
	request("https://localhost:9442/v2/user_profiles/dejan@ca.ibm.com", options)
		.then((response) => {
			if (response.statusCode !== 200) {
				throw new Error("Unexpected status code: " + response.statusCode);
			}
			return response.statusCode < 400 ? response.body.json() : response.body.text();
		})
		.then((result) => {
			console.log("Response from reload profile: " + JSON.stringify(result));
			exit();
		})
		.catch((err) => {
			exit("Error detected: " + err.message);
		})
	;
}

function exit(message) {
	if (message) {
		console.log(message);
	}
	// eslint-disable-next-line no-process-exit
	process.exit(message ? 1 : 0);
}

testUndici();

If I use GET it works. If I rename it to get, it fails in nginx proxy with error 400.

[dglozic]

The only quirk above is this:

		dispatcher: new Agent({ connect: { rejectUnauthorized: false } })

as I am running this in localhost using https and my server is using self-signed certificate. We use default global dispatcher when deployed in the cluster.

[ronag]

Don't use lowercase if your server doesn't support it...

[dglozic]

Sure, so your position is that you are passing the method 'as is' and it is the caller's responsibility to use the method that the target server can support?

I think request from which we are porting was more forgiving (probably converting methods to upper case before sending). We can do the same, was just confused as to what the contract was.

Note that this happens in both localhost nginx (for testing) and k8s ingress based on nginx - same behaviour. Many of your users will use this against a service running in k8s. I am surprised it wasn't flagged before.

[mcollina]

From a quick scavenge in the RFCs:

https://datatracker.ietf.org/doc/html/rfc7231#section-4.1

This specification defines a number of standardized methods that are
commonly used in HTTP, as outlined by the following table. By
convention, standardized methods are defined in all-uppercase
US-ASCII letters.

Apparently method names are all in uppercase.

[dglozic]

I understand that, but if that is a spec, undici should reject 'options' object with a method that is not in uppercase (as 'invalid method value' error or something).

[dglozic]

I understand this goes into the same grey area where RFC for bearer tokens states it should be "Authorization: Bearer {token}" and some services can handle both "Bearer" and "bearer" and some throw a hissy fit on "bearer" because RFC.

[ronag]

The standard methods are uppercase but servers are not forbidden from implementing whatever method name and casing they wish.

The upper casing is more of a convention than a rule.

[dglozic]

Correct, and it looks like nginx (and nginx-based k8s ingress) does not like lowercase. OK, I can close this and we will make sure we don't use methods not supported by the APIs we are calling.