The Wayback Machine - https://web.archive.org/web/20240723191956/https://github.com/apache/skywalking/issues/7505
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

[NodeJS Agent] When oap-server had stoped, java invokes node, the node app will raise "TypeError" and exit. #7505

Closed
3 tasks
chlook opened this issue Aug 20, 2021 · 13 comments · Fixed by apache/skywalking-nodejs#90
Assignees
Labels
bug Something isn't working and you are sure it's a bug! core feature Core and important feature. Sometimes, break backwards compatibility. high priority High priority issue, blocking next release. nodejs NodeJS backend agent related
Milestone

Comments

@chlook
Copy link

chlook commented Aug 20, 2021

Please answer these questions before submitting your issue.

  • Why do you submit this issue?
    • Question or discussion
    • [*] Bug
    • Requirement
    • Feature or performance improvement

Question

  • What do you want to know?

    When the node agent cannot connect to the server after the server is stopped, the default value is null。Want to confirm whether it can be solved。


Bug

  • Which version of SkyWalking, OS, and JRE?

    • env: k8s for aliyun cloud
    • docker-engine: 1.18.8-aliyun.1
    • oap-server: skywalking-oap-server:8.6.0-es7
    • skywalking-backend-js: master
  • Which company or project?

    • no
  • What happened?
    If possible, provide a way to reproduce the error. e.g. demo application, component version.

    • env: k8s for aliyun cloud

    • docker-engine: 1.18.8-aliyun.1

    • oap-server: skywalking-oap-server:8.6.0-es7

    • java: 8.6.0

    • node: skywalking-backend-js: master

    • ERROR

      TypeError [ERR_INVALID_ARG_TYPE]: The first argument must be of type string or an instance of Buffer, ArrayBuffer, or Array or an Array-like Object. Received undefined
          at new NodeError (node:internal/errors:278:15)
          at Function.from (node:buffer:317:9)
          at ContextCarrier._this.decode (/Users/anyone/Desktop/tmp/code/node-app-handler/node_modules/skywalking-backend-js/lib/trace/context/ContextCarrier.js:42:27)
          at ContextCarrier.set (/Users/anyone/Desktop/tmp/code/node-app-handler/node_modules/skywalking-backend-js/lib/trace/context/ContextCarrier.js:63:50)
          at /Users/anyone/Desktop/tmp/code/node-app-handler/node_modules/skywalking-backend-js/lib/trace/context/ContextCarrier.js:87:133
          at Array.forEach (<anonymous>)
          at Function.ContextCarrier.from (/Users/anyone/Desktop/tmp/code/node-app-handler/node_modules/skywalking-backend-js/lib/trace/context/ContextCarrier.js:87:88)
          at Server._sw_request (/Users/anyone/Desktop/tmp/code/node-app-handler/node_modules/skywalking-backend-js/lib/plugins/HttpPlugin.js:131:63)
          at Server.emit (node:events:376:20)
          at parserOnIncoming (node:_http_server:919:12) {
      code: 'ERR_INVALID_ARG_TYPE'
      }
      

Requirement or improvement

  • Please describe your requirements or improvement suggestions.
    Because there is no exception when Java calls Java after the server stops, but when node is called, node throws an exception and exits, affecting the operation of the application. Hope to improve.
@wu-sheng wu-sheng added the nodejs NodeJS backend agent related label Aug 20, 2021
@kezhenxu94 kezhenxu94 added the good first issue Good first issue for beginners label Aug 30, 2021
@820465323
Copy link

I have the same problem

@xinnj
Copy link

xinnj commented Jan 27, 2022

Same problem, hope it can be fixed quickly.

@wu-sheng
Copy link
Member

@xinnj @820465323 If no one intends to fix it on their own, everyone has to wait. This is how an open-source project works.

@pg-yang
Copy link
Member

pg-yang commented Jul 16, 2022

I reproduce this issue use :

  • org.apache.httpcomponents.httpclient:4.5.13
  • apache-skywalking-java-agent-8.11.0

And I not start OAP.

The request message that Httpclient send to node server:

GET / HTTP/1.1
sw8:
sw8-correlation:
sw8-x: 0-
Host: 127.0.0.1:8081
Connection: Keep-Alive
User-Agent: Apache-HttpClient/4.5.13 (Java/1.8.0_322)
Accept-Encoding: gzip,deflate

Because if OAP is not working, Java agent will create a IgnoredTracerContext
Then ContextCarrier will put incorrect data , method serialize will generate some error headers

@wu-sheng
Copy link
Member

Why is this header wrong? I am not sure of your conclusion.

@pg-yang
Copy link
Member

pg-yang commented Jul 16, 2022

You are right , I'm not sure header is wrong 。
But I found :

  1. When the request message include header named sw8(but the value is null) , the node server will cause exception ;
  2. Sometimes the request message node include header named sw8 (the value is correct) , then node server is working correctly

I will read the java agent code in depth

@wu-sheng
Copy link
Member

No one could guarantee sw8 is not null. The agent should tolerant this case.
sw8 could be null by sampling, no backend connection, etc.

@pg-yang
Copy link
Member

pg-yang commented Jul 16, 2022

I see , sometimes GRPCChannelStatus (ContextManagerExtendService#status)is not reliable.
So some request header can carry sw8 when the GRPCChannelStatus is not DISCONNECT
and other time , request maybe carry a blank header named sw8
Node agent should detect sw8' value . not just detect hasOwnProperty :

    ContextCarrier.from = function (map) {
        if (!map.hasOwnProperty('sw8'))
            return;
        var carrier = new ContextCarrier();
        carrier.items.filter(function (item) { return map.hasOwnProperty(item.key); }).forEach(function (item) { return (item.value = map[item.key]); });
        return carrier;
    };
@wu-sheng
Copy link
Member

Yes, if it requires sw8, then this become a security attach point if this service is exposed.
People could attach the service through an illegal sw8 header. @kezhenxu94

@kezhenxu94
Copy link
Member

Yes, if it requires sw8, then this become a security attach point if this service is exposed.
People could attach the service through an illegal sw8 header. @kezhenxu94

Let me investigate this issue and try to fix it as soon as possible.

@pg-yang
Copy link
Member

pg-yang commented Jul 16, 2022

Case 1 , sw8 header is right , working correctly:


^@^@^@^@^@20:08:47.172655 IP localhost.57335 > localhost.sunproxyadmin: Flags [P.], seq 2399126858:2399127312, ack 2719539646, win 6379, options [nop,nop,TS val 712954341 ecr 525747447], length 454
E.....@.@..................J...............
*~...VD.GET / HTTP/1.1
sw8: 1-M2FjZTcwYzk0YTQwNDRmYjgxYjVmNmJmNDUyYWViMDIuMS4xNjU3OTczMzI3MDY1MDAwMQ==-M2FjZTcwYzk0YTQwNDRmYjgxYjVmNmJmNDUyYWViMDIuMS4xNjU3OTczMzI3MDY1MDAwMA==-0-WW91cl9BcHBsaWNhdGlvbk5hbWU=-Mzk0YmM5ZmVlMDA2NDEyNzk2YjJjMjFjZGZjZDA5YjJAMTkyLjE2OC4wLjEwOA==-Lw==-MTI3LjAuMC4xOjgwODE=
sw8-correlation:
sw8-x: 0-
Host: 127.0.0.1:8081
Connection: Keep-Alive
User-Agent: Apache-HttpClient/4.5.13 (Java/18.0.1)
Accept-Encoding: gzip,deflate


^@^@^@^@^@^@^@^@^@^@^@20:19:37.094507 IP localhost.sunproxyadmin > localhost.57335: Flags [P.], seq 1:208, ack 454, win 6372, options [nop,nop,TS val 526397394 ecr 712954341], length 207
E.....@.@..................................
.`/.*~..HTTP/1.1 200 OK
Content-Type: text/html
Date: Sat, 16 Jul 2022 12:19:37 GMT
Connection: keep-alive
Keep-Alive: timeout=5
Transfer-Encoding: chunked

28
Congrats you have a created a web server
0

Case 2 , sw8 header exist , but value is null , working error :

20:19:47.074184 IP localhost.57772 > localhost.sunproxyadmin: Flags [P.], seq 2615598964:2615599149, ack 3217890930, win 6379, options [nop,nop,TS val 501835136 ecr 4012315039], length 185
E.....@.@..................t...r...........
..e..'..GET / HTTP/1.1
sw8:
sw8-correlation:
sw8-x: 0-
Host: 127.0.0.1:8081
Connection: Keep-Alive
User-Agent: Apache-HttpClient/4.5.13 (Java/18.0.1)
Accept-Encoding: gzip,deflate
@wu-sheng
Copy link
Member

@ypg521 If you could submit a fix quickly referring Java's validation implementations, we could review that.

@wu-sheng wu-sheng added high priority High priority issue, blocking next release. bug Something isn't working and you are sure it's a bug! core feature Core and important feature. Sometimes, break backwards compatibility. and removed good first issue Good first issue for beginners labels Jul 16, 2022
@wu-sheng wu-sheng added this to the NodeJS 0.5.1 milestone Jul 16, 2022
@wu-sheng
Copy link
Member

Announced CVE-2022-36127 and 0.5.1 fixed this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working and you are sure it's a bug! core feature Core and important feature. Sometimes, break backwards compatibility. high priority High priority issue, blocking next release. nodejs NodeJS backend agent related
6 participants