Azure / azure-relay-node

☁️Node.js library for Azure Relay Hybrid Connections
https://docs.microsoft.com/en-us/azure/service-bus-relay/relay-what-is-it
MIT License
12 stars 14 forks source link

hyco-https + express.js = TypeError: requestChannel.pendingRequest.handleBody is not a function #30

Open rubinovk opened 5 years ago

rubinovk commented 5 years ago

Actual Behavior

  1. Create listener with express:
    
    const http = require('http')
    const https = require('hyco-https')
    http.ServerResponse = https.ServerResponse
    const express = require('express')
    var app = express()

var uri = https.createRelayListenUri(ns, path) var server = https.createRelayedServer( { server: uri, token: () => https.createRelayToken(uri, keyrule, key) }, app )

app.post('/my-hc', (req, res) => { console.log('received') res.status(200).send('hey') })

server.listen()

2. Sender connects and sends data
3. Listener receives the request on `app.post('/my-hc')` and crashes with 

hyco-https\lib\HybridConnectionHttpsServer.js:653 requestChannel.pendingRequest.handleBody(event.data); ^ TypeError: requestChannel.pendingRequest.handleBody is not a function


4. If the sent data was roughly < 50 KB then __no response is sent__ and sender also crashes with `Error: read ECONNRESET     at TLSWrap.onread (net.js:622:25)`
5. If the sent data was roughly > 50KB then __response is sent__ and sender gets status 200 and does not crash.

> Note: same scenario works OK without express.js integration.

## Expected Behavior
No crashing

## Versions  
- OS platform and version: Win 10
- Node Version: 10.15.3
- NPM package version or commit ID: "express": "^4.16.4", "hyco-https": "^1.3.0",
anxious-coder-lhs commented 4 years ago

Same issue.

curtiskeisler commented 4 years ago

I'm having the same issue when I use Express and body-parser to process a post similar to the above.

I triaged the issue a little and found that on this line:

https://github.com/Azure/azure-relay-node/blob/7c6034b1e465ad0be6be841fcaba29a0e30396cf/hyco-https/lib/HybridConnectionHttpsServer.js#L408

. . . the pendingRequest doesn't have a handleBody method. It appears in my debugger in VS Code that it is an IncomingMessage object.

However, when I inspect the object, the handleBody method is not there (which is the error reported above).

IncomingMessage is defined in the HybridConnectionHttpsServer.js file here:

https://github.com/Azure/azure-relay-node/blob/7c6034b1e465ad0be6be841fcaba29a0e30396cf/hyco-https/lib/HybridConnectionHttpsServer.js#L14

and here's the function that we are looking for:

https://github.com/Azure/azure-relay-node/blob/7c6034b1e465ad0be6be841fcaba29a0e30396cf/hyco-https/lib/_hyco_incoming.js#L77

It's as if there are multiple IncomingMessage classes derived from different sources.

Perhaps this messages needs to be recast or something?

Also, something odd I saw is that this code directly below the IncomingMessage definition above

https://github.com/Azure/azure-relay-node/blob/7c6034b1e465ad0be6be841fcaba29a0e30396cf/hyco-https/lib/HybridConnectionHttpsServer.js#L17

is commented out but later, there is code that uses those constants as shown here:

https://github.com/Azure/azure-relay-node/blob/7c6034b1e465ad0be6be841fcaba29a0e30396cf/hyco-https/lib/HybridConnectionHttpsServer.js#L559

My guess is that if that code is ever hit, it will fail. Being in proximity to this and other comments, I wonder if that commenting out or a related check may have introduced an issue here?

Purely speculation!

jfggdl commented 3 years ago

Thanks for reporting this issue.

diegohexi commented 3 years ago

Pull request created for this fix #56