Limited functionality of custom middleware (Infinite loop when trying to handle HTTP request bodies)

BFU-HCM commented 3 years ago

When using a custom middleware in a SAP UI5 application as described here, the req and res objects handed over to the middleware are of different shape. Express-specific methods like res.status() oder res.header() do not exist (which can be worked around with native Node methods tough). However, I could not find a solution for the following problem:

POST Request bodies can't be handled at all; the middleware falls into an infinite loop when trying to read them via res.on('data', ...) or other stream reading methods (or encapsulating other middlewares/parsers like http-proxy, express.json(), bodyParser.json()). When using the Chromium browser, the request is aborted after exactly 20 oder 40 seconds, a postman request seems to have no timout and runs forever (until the execution of Karma is stopped).

When serving the application directly with the same UI5 configuration and middleware files, tests can be executed without problems.

Versions: "karma": "^6.2.0", "karma-ui5": "^2.3.3", "express": "^4.17.1", "@sap/ux-ui5-tooling": "^1.2.1", "@ui5/cli": "^2.6.4", "@ui5/fs": "2.0.3", "@sap/grunt-sapui5-bestpractice-build": "^1.4.13", "@sap/ux-specification": "^1.78.8", "@ui5/logger": "2.0.0"

ui5-testing.yaml:

specVersion: '2.1'
metadata:
  name: grunt-build
type: application
framework:
  name: SAPUI5
  version: "1.84.0"
  libraries:
    - name: sap.m
    - name: sap.ui.core
    - name: sap.ui.layout
    - name: sap.ui.support
      development: true
    - name: sap.ui.table
    - name: sap.ui.unified
    #- name: sap.ui.model
    - name: sap.ushell
      development: true
    - name: themelib_sap_fiori_3
      optional: true
    - name: themelib_sap_belize
      optional: true
    #- name: themelib_sap_bluecrystal
    #  optional: true
    - name: sap.f
    - name: sap.tnt
resources:
  configuration:
    paths:
      webapp: /webapp
server:
  customMiddleware:
    - name: proxy
      beforeMiddleware: serveResources
      configuration:
        testing: true
---
specVersion: '2.1'
kind: extension
type: server-middleware
metadata:
  name: proxy
middleware:
  path: lib/middleware/proxy.js

opatests.Qunit.html:

<!DOCTYPE html>
<html>
<head>
    <meta charset="utf-8">
    <title>Integration tests</title>

    <script id="sap-ui-bootstrap"
            src="https://sapui5.hana.ondemand.com/resources/sap-ui-core.js"
            data-sap-ui-theme='sap_fiori_3'
            data-sap-ui-resourceroots='{
                "com.myapp.test": "../../"
            }'
            data-sap-ui-animation="false"
            data-sap-ui-compatVersion="edge"
            data-sap-ui-async="true">
    </script>

    <!--<link rel="stylesheet" type="text/css" href="../../../../../../../../../../resources/sap/ui/thirdparty/qunit-2.css">-->
    <link rel="stylesheet" type="text/css" href="https://code.jquery.com/qunit/qunit-2.11.0.css">
    <link href="https://fonts.googleapis.com/css2?family=Material+Icons+Round" rel="stylesheet">

    <!--<script src="../../../../../../../../../../resources/sap/ui/thirdparty/qunit-2.js"></script>-->
    <script src="https://code.jquery.com/qunit/qunit-2.11.0.js"></script>

    <script src="../../../../../../../../../../resources/sap/ui/qunit/qunit-junit.js"></script>

    <script src="opaTests.qunit.js"></script>
</head>
<body>
    <div id="qunit"></div>
    <div id="qunit-fixture"></div>
</body>
</html>

karma.conf.js:

module.exports = function (config) {
  "use strict";
  config.set({

    frameworks: ['ui5'], //, 'qunit'],
    reporters: ["progress"],

    browsers: ["Chrome_without_security"],

    customLaunchers: {
      Chrome_without_security: {
        base: 'Chrome',
        flags: [
          '--disable-web-security',
          '--no-sandbox',
          '--disable-ipc-flooding-protection',
          '--no-default-browser-check'
        ]
      }
    },

    client: {
      runInParent: true
    },

    ui5: {
      mode: "html",
      testpage: "webapp/test/integration/opaTests.qunit.html",
      configPath: "ui5-testing.yaml",
    },
  });
};

I assume this is a bug, otherwise I would appreciate an explanation of what is exactly going on here under the hood and wheter there is a known workaroung. Thanks!

matz3 commented 3 years ago

I could reproduce the described issue. From what I could see (using body-parser) is the following error: BadRequestError: request aborted at IncomingMessage.onAborted (node_modules/raw-body/index.js:231:10)

Next step would be to check whether this is also an issue when just using a middleware within karma (without the karma-ui5 plugin).

nlunets commented 3 years ago

Hi @matz3 i came accross the same problem now with a different middleware and can confirm a few things :

if the middleware runs within karma -> no problem
If the middleware runs before your csp middleware -> no problem

It seems that there is a delay after the csp middleware which ends up with the router getting out of the middleware stack and completing the request. It would be great if you could have a look :)

matz3 commented 3 years ago

@nlunets thanks for your input! I think this will really help further investigate into this issue.

Fjaoos commented 3 years ago

Hi @matz3 I am able to provide a SAPUI5 application that uses the @sap/ux-ui5-fe-mockserver-middleware for testing. Please contact me if you want me to provide the full (not working) example to you.

Regards fjaoos

matz3 commented 3 years ago

I was able to isolate the issue by using connect, body-parser and router. It seems to be a combination of calling req.pause / req.resume() (which happens internally in karma) and using the router package. I'll share the code soon and try to check which module causes the issue.

matz3 commented 3 years ago

Example to reproduce: https://github.com/matz3/connect-router-body-parser-issue

matz3 commented 3 years ago

I could now fully understand the scenario and will open an issue for the karma project to understand why those methods are called and to check how it could be solved.

As mentioned above, karma calls req.pause() and req.resume(). After calling resume, the request stream can still be read, but only within the same tick. This is usually the case, but the router module defers the call to next, which means that nested routers are causing this issue. One router is created within karma-ui5 to apply the tooling middleware and one (nested) router is created for the csp middleware. So every middleware that comes after can't consume the request stream anymore.

tiosteel commented 3 years ago

Hi @matz3,

I also have the same case in my current project: My run karma.conf.js is also using configPath option

function karmaConfig(config) {
    config.set({
        basePath: '.',
        frameworks: ['ui5'],
        ui5: {
            type: "application",
            configPath: "app/ui5-fiori.yaml",
            paths: {
                webapp: 'app'
            }
        },
        singleRun: true,
        browsers: ['Chrome_without_security'],
        browserNoActivityTimeout: 1800000,
        failOnFailingTestSuite: false,
        customLaunchers: {
            Chrome_without_security: {
                base: 'Chrome',
                flags: ['--disable-web-security'],
            },
        },
    });
}

and the app is started with the mockserver:

specVersion: '2.4'
metadata:
    name: 'fioriLaunchpad'
type: application

resources:
    configuration:
        paths:
            webapp: './app'

server:
    customMiddleware:
        - name: sap-fe-mockserver
          mountPath: /
          afterMiddleware: compression
          configuration:
              annotations:
                  localPath: './customer-order/annotations.cds'
              service:
                  urlBasePath: '/api/v1'
                  name: 'ui'
                  metadataXmlPath: './test/ui/mockserver/localService/metadata.xml'
                  mockdataRootPath: './test/ui/mockserver/localService/data'
        - name: fiori-tools-proxy
          afterMiddleware: compression
          configuration:
              ignoreCertError: false # If set to true, certificate errors will be ignored. E.g. self-signed certificates will be accepted
              ui5:
                  path:
                      - /resources
                      - /test-resources
                  url: https://ui5.sap.com
                  version:

so karma successfully loaded my testrunner, but when the metadata of the mockserver is loaded there's an issue at node_modules\@sap\ux-ui5-fe-mockserver-middleware\dist\middleware.js line 71 res.header is undefined

currently I'm trying it with

karma-ui5 2.3.4
@sap/ux-ui5-fe-mockserver-middleware 1.3.1 (but I also tried 1.3.5 and higher loaded from sap package repo - errors are similar, e.g. req.header is undefined)

matz3 commented 3 years ago

@tiosteel as already stated above (https://github.com/SAP/karma-ui5/issues/344#issuecomment-914245560) using beforeMiddleware: csp might be sufficient as a workaround, depending on the use-case of the middleware.

AdamWang93 commented 3 years ago

Using beforeMiddleware: csp in ui5.yaml can be a workaround. `

name: ui5-middleware-cfdestination beforeMiddleware: csp configuration: authenticationMethod: none debug: true port: 1091 xsappJson: webapp/xs-app.json destinations:
- name: XXX url: https://XXXXX `

lgonzalezsap commented 2 years ago

Hi all, Had a similar issue with a Fiori Elements app using mockserver @sap/ux-ui5-fe-mockserver-middleware 1.5.1 and adding beforeMiddleware: csp did the trick

name: sap-fe-mockserver beforeMiddleware: csp configuration: service: urlBasePath: /backend/odata name: '' metadataXmlPath: ./webapp/localService/metadata.xml mockdataRootPath: ./webapp/localService/mockdata generateMockData: true

Many thanks!

SAP / karma-ui5

Limited functionality of custom middleware (Infinite loop when trying to handle HTTP request bodies) #344