Some HTTP request back http 500 code after update to 4.0.0.

nbjsnb commented 5 days ago

Current Behavior

The ver3.1.3's http-request flow back "statusCode: 500" after upgrading to 4.0,[hassos node-red addons] Copy the flow to node-red 3.13 can back the right msg； I checked the request information and found that the “content-type” was not in the returned msg, but I configured it in the function.

After reading the 4.0 release, I found the following updates. Not sure if these updates are the cause of this issue. Other updates The following configuration options have been added to the default settings file: httpAdminCookieOptions can be used to customise the options set on cookies as part of the authentication system. httpStaticCors can be used to set custom cross-origin resource sharing rules for the content served up via the httpStatic option node-red --version now reports the node-red, node.js and os information withou

Expected Behavior

Wrong Msg[ver:4.0.0] Right Msg [ver:3.1.3]

Steps To Reproduce

No response

Example flow

No response

Environment

Node-RED version:4.0.0
Node.js version:20.0/HASSOS DOCKER
npm version:10.3.0
Platform/OS:Windows10/hassos
Browser:Chroms126.0.6478.63

hardillb commented 5 days ago

Hello,

The change notes you mention are all to do with the HTTP server built into Node-RED, and how it responds to incoming HTTP requests, not to do with HTTP requests made to other sites using the HTTP-Request node. These are not related to this problem.

A 500 error normally means a problem on the server side, is there a msg.payload returned along with the 500 error that might include a description of what the server side problem might be?

I assume you do not have access to the server side logs?

Node-RED 3.1.3 is a relatively old v3.1.x release (latest is 3.1.11) would it be possible to test with a newer 3.1.x release to help narrow down when the change was made? I don't think there were any specific changes in 4.0.0 apart from updating the http client library version we use.

nbjsnb commented 5 days ago

Hello,

The change notes you mention are all to do with the HTTP server built into Node-RED, and how it responds to incoming HTTP requests, not to do with HTTP requests made to other sites using the HTTP-Request node. These are not related to this problem.

A 500 error normally means a problem on the server side, is there a msg.payload returned along with the 500 error that might include a description of what the server side problem might be?

I assume you do not have access to the server side logs?

Node-RED 3.1.3 is a relatively old v3.1.x release (latest is 3.1.11) would it be possible to test with a newer 3.1.x release to help narrow down when the change was made? I don't think there were any specific changes in 4.0.0 apart from updating the http client library version we use.

I use finddle to capture the http request.I finde the request have some diffent. In 3.1.x the http-request User-agent can be send right.

the User-agent will be replace the other message in v4.0.

we can see that if an API does not need to verify the user-agent, it can return normal data. If this API needs to verify the user-agent, it will report an error. if I delete the user-agent in 3.1.x flow。It will be returen the same wrong message.

nbjsnb commented 5 days ago

Hello,

The change notes you mention are all to do with the HTTP server built into Node-RED, and how it responds to incoming HTTP requests, not to do with HTTP requests made to other sites using the HTTP-Request node. These are not related to this problem.

A 500 error normally means a problem on the server side, is there a msg.payload returned along with the 500 error that might include a description of what the server side problem might be?

I assume you do not have access to the server side logs?

Node-RED 3.1.3 is a relatively old v3.1.x release (latest is 3.1.11) would it be possible to test with a newer 3.1.x release to help narrow down when the change was made? I don't think there were any specific changes in 4.0.0 apart from updating the http client library version we use.

3.1.11 is right

nbjsnb commented 5 days ago

Hello,

The change notes you mention are all to do with the HTTP server built into Node-RED, and how it responds to incoming HTTP requests, not to do with HTTP requests made to other sites using the HTTP-Request node. These are not related to this problem.

A 500 error normally means a problem on the server side, is there a msg.payload returned along with the 500 error that might include a description of what the server side problem might be?

I assume you do not have access to the server side logs?

Node-RED 3.1.3 is a relatively old v3.1.x release (latest is 3.1.11) would it be possible to test with a newer 3.1.x release to help narrow down when the change was made? I don't think there were any specific changes in 4.0.0 apart from updating the http client library version we use.

hello： I used a temporary workaround to resolve this issue. However, this modification is completely contrary to the logic of the code. Nevertheless, it is currently functioning correctly. I am looking forward to the official solution being provided."

modify the code https://github.com/node-red/node-red/blob/master/packages/node_modules/%40node-red/nodes/core/network/21-httprequest.js 537line.

if (!opts.headers.hasOwnProperty('user-agent')) { opts.headers['user-agent'] = 'Mozilla/5.0 (Node-RED)'; } delete the character'!'.

GogoVega commented 5 days ago

Hi, It's not the problem:

The user-agent property of the message should be copied here:

https://github.com/node-red/node-red/blob/cb0c48457952bc6209c4e1729038b12b9100630e/packages/node_modules/%40node-red/nodes/core/network/21-httprequest.js#L319-L325

The condition you show serves as a default which is why it is negated.

Steve-Mcl commented 5 days ago

@GogoVega By removing the ! the user is effectively NOT sending a user-agent string.

pseudo

if options has user-agent
   overwrite user agent with default
end_if

In essence, dont add a user agent.

Perhaps the server in question occasionally doesn't like this UA string?

NOTE: A default UA was added because it SHOULD be present according to the RFC:

The "User-Agent" header field contains information about the user agent originating the request, which is often used by servers to help identify the scope of reported interoperability problems, to work around or tailor responses to avoid particular user agent limitations, and for analytics regarding browser or operating system use. A user agent SHOULD send a User-Agent field in each request unless specifically configured not to do so

and SHOULD is defined as

SHOULD This word, or the adjective "RECOMMENDED", mean that there may exist valid reasons in particular circumstances to ignore a particular item, but the full implications must be understood and carefully weighed before choosing a different course.

Some API/WEB servers consider the lack of a UA to be malicious and others actively require a UA for that very reason. In saying that, there are some that reject requests based on the content of UA. It is pretty much a win-win-lose-win situation.

It would be helpful to know if instead of modifying the source, if adding a header of user-agent: "" would have the same effect?

nbjsnb commented 5 days ago

@GogoVega By removing the ! the user is effectively NOT sending a user-agent string.

pseudo
if options has user-agent
   overwrite user agent with default
end_if
In essence, dont add a user agent.

Perhaps the server in question occasionally doesn't like this UA string?

NOTE: A default UA was added because it SHOULD be present according to the RFC:

The "User-Agent" header field contains information about the user agent originating the request, which is often used by servers to help identify the scope of reported interoperability problems, to work around or tailor responses to avoid particular user agent limitations, and for analytics regarding browser or operating system use. A user agent SHOULD send a User-Agent field in each request unless specifically configured not to do so

and SHOULD is defined as

SHOULD This word, or the adjective "RECOMMENDED", mean that there may exist valid reasons in particular circumstances to ignore a particular item, but the full implications must be understood and carefully weighed before choosing a different course.

Some API/WEB servers consider the lack of a UA to be malicious and others actively require a UA for that very reason. In saying that, there are some that reject requests based on the content of UA. It is pretty much a win-win-lose-win situation.

It would be helpful to know if instead of modifying the source, if adding a header of user-agent: "" would have the same effect?

I used Fiddle to capture packets. When I configured the [user-agent] in the function, the user-agent in the headers was replaced with 'Mozilla/5.0 (Node-RED)'. This behavior is inconsistent with the expected result of the code at line 537. However, when I removed the '!' from line 537, the user-agent from the function was correctly passed and sent to the HTTP server. I also received the correct response, which is quite perplexing.

Steve-Mcl commented 5 days ago

When I configured the [user-agent] in the function, the user-agent in the headers was replaced with 'Mozilla/5.0 (Node-RED)'.

I have tested this also and the user-agent IS correctly set. Could you share a flow or function code please.

What I have discovered is an issue whereby users cannot set an empty header (issue) but that should not stop you setting your own header value.

nbjsnb commented 5 days ago

When I configured the [user-agent] in the function, the user-agent in the headers was replaced with 'Mozilla/5.0 (Node-RED)'.

I have tested this also and the user-agent IS correctly set. Could you share a flow or function code please.

What I have discovered is an issue whereby users cannot set an empty header (will be raising an issue and PR very shortly) but that should not stop you setting your own header value.

A simply flow to setting msg.headers by function, I also tested setting the User-Agent (UA) by injection, with the same wrong results

msg.headers={ "accept": "*/*", "User-Agent": "manager/4.6.46 (android; 2106118C 11);lang=zh-CN;clientIdentifier=Domestic", "content-type": "application/json", "token": "xxxxx", "Connection": "close", "accept-encoding": "gzip", }

GogoVega commented 5 days ago

Should User-Agent be lowercase?

Steve-Mcl commented 5 days ago

@GogoVega no, but you have stumbled upon a bug.

FYI, The header case is unimportant to the RFC (HTTP header names are case-insensitive, according to RFC 2616: 4.2) however, this check is done before the normalisation so user-agent is not found at line 537 and therefore User-Agent is overwritten.

In short, NR should not be looking for the absence of user-agent in a case sensitive way

Steve-Mcl commented 5 days ago

At this point, all things considered (see comments in #4789) I believe the best option is to remove setting a default user-agent (against the RFC recommendations).

This will revert to http-request to its operational state as in 3.x stream.

I do believe GOT adds a user-agent if left empty (need to verify) and that may change between GOT versions (also need to verify)

I will raise a PR for review based on this premise.

Steve-Mcl commented 4 days ago

@nbjsnb and ayone else on NR 4.0.0 finding this thread, to get past this until the fix from #4791 is released, a simple workaround as was confirmed here: https://discourse.nodered.org/t/strange-issue-with-http-request-after-upgrade-to-4-0/88892/10?u=steve-mcl

In short, do NOT use the built in User-Agent option on the nodes UI, instead, do one of the following:

set user-agent (lower case) to a valid user agent string in the msg.headers object
add a manual header on the nodes UI and enter the lower-case version user-agent and set a non empty string value

nbjsnb commented 4 days ago

@nbjsnb and ayone else on NR 4.0.0 finding this thread, to get past this until the fix from #4791 is released, a simple workaround as was confirmed here: https://discourse.nodered.org/t/strange-issue-with-http-request-after-upgrade-to-4-0/88892/10?u=steve-mcl

In short, do NOT use the built in User-Agent option on the nodes UI, instead, do one of the following:

set user-agent (lower case) to a valid user agent string in the msg.headers object

add a manual header on the nodes UI and enter the lower-case version user-agent and set a non empty string value

Many thanks for your kind information. Good job

node-red / node-red