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

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

Open
BFU-HCM opened this issue Aug 10, 2021 · 11 comments
Assignees
Labels
bug Something isn't working

Comments

@BFU-HCM
Copy link

BFU-HCM commented Aug 10, 2021

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
Copy link
Member

matz3 commented Aug 27, 2021

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).

@matz3 matz3 added the bug Something isn't working label Aug 27, 2021
@nlunets
Copy link
Member

nlunets commented Sep 7, 2021

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
Copy link
Member

matz3 commented Sep 8, 2021

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

@Fjaoos
Copy link

Fjaoos commented Sep 9, 2021

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 matz3 self-assigned this Oct 1, 2021
@matz3
Copy link
Member

matz3 commented Oct 1, 2021

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
Copy link
Member

matz3 commented Oct 1, 2021

@matz3
Copy link
Member

matz3 commented Oct 7, 2021

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
Copy link

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
Copy link
Member

matz3 commented Oct 13, 2021

@tiosteel as already stated above (#344 (comment)) using beforeMiddleware: csp might be sufficient as a workaround, depending on the use-case of the middleware.

@AdamWang93
Copy link

AdamWang93 commented Oct 27, 2021

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
Copy link

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

7 participants