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

Support parsing of application/xml content-type header. #92

Closed
SK-FComputer opened this issue Oct 29, 2021 · 5 comments
Closed

Support parsing of application/xml content-type header. #92

SK-FComputer opened this issue Oct 29, 2021 · 5 comments
Labels
feature New functionality or improvement

Comments

@SK-FComputer
Copy link

Support plan

  • is this issue currently blocking your project? (yes/no): yes
  • is this issue affecting a production system? (yes/no): yes

Context

  • node version: v14.16.0
  • module version:
  • └─ @hapi/[email protected]
    └─ @hapi/[email protected]
  • environment (e.g. node, browser, native): node
  • used with (e.g. hapi application, another framework, standalone, ...): hapi application
  • any other relevant information:

What problem are you trying to solve?

Support parsing of application/xml content-type header.

Currently it is almost impossible to support parsing XML post requests to json in the pre handler.
As payload allow, doesn't accept this header and throws error to failaction.
Even if you check the request.header and try to request.continue the request, payload turns null.

Do you have a new or modified API suggestion to solve the problem?

// application/xml
    if(mime.match(/application\/xml/)){
        return payload.toString()
    }

Adding this to:


Solves the issue.

@devinivy
Copy link
Member

devinivy commented Oct 29, 2021

Unfortunately this change would allow the payload through without parsing it or checking that it's a valid XML payload. Due to the complexity of XML parsing, it's most likely that this will be left to users for the time being, and hapi does enable users to do this themselves. To do so, configure request.options.payload.parse to false then handle the parsing within your application. Here's an example: https://runkit.com/60c18db503676e001ce17d66/617c1083c4a4b3000876cd4f

@Nargonath
Copy link
Member

Also please don't ping Eran (hueniverse) in your comment @SK-FComputer since he's not maintaining hapi anymore.

@SK-FComputer
Copy link
Author

@Nargonath sorry, just looked for the one dev with most commits.

Unfortunately this change would allow the payload through without parsing it or checking that it's a valid XML
@devinivy That would require adding a dependency on an XML parser/validator or creating one our self, I don't know if @Hapi allows this ?
The validity should be checked after the parsing to string anyways, this would just allow it to go through, so IE:
Allows application/xml, which is basically the same as text/xml which is allowed and not validated.

But #90 is interesting, would you consider implementing a fix, if i created a pull request for that ?

@SK-FComputer
Copy link
Author

@Nargonath & @devinivy
I've used patch-package to patch @hapi/[email protected] for the project I'm working on, to create what is needed to support custom parsers.

Here is the diff that solved my problem:

subtext

diff --git a/node_modules/@hapi/subtext/lib/index.js b/node_modules/@hapi/subtext/lib/index.js
index 28b91de..888a432 100644
--- a/node_modules/@hapi/subtext/lib/index.js
+++ b/node_modules/@hapi/subtext/lib/index.js
@@ -107,7 +107,7 @@ internals.parse = async function (req, tap, options, contentType) {
     // Output: 'data'
 
     const payload = await Wreck.read(source, { timeout: options.timeout, maxBytes: options.maxBytes });
-    return internals.object(options, payload, contentType.mime);
+    return await internals.object(options, payload, contentType.mime);
 };
 
 
@@ -172,42 +172,73 @@ internals.raw = async function (req, tap, options) {
 };
 
 
-internals.object = function (options, payload, mime) {
-
-    // Binary
-
-    if (mime === 'application/octet-stream') {
-        return payload.length ? payload : null;
-    }
-
-    // Text
+internals.object = async function (options, payload, mime) {
+
+    // Default parsers
+    let parsers = [
+        {
+            // Binary
+            mime: 'application/octet-stream',
+            parse: (unparsedPayload) =>
+                unparsedPayload.length ? unparsedPayload : null,
+        },
+        {
+            // Text
+            mime: /^text\/.+$/,
+            parse: (unparsedPayload) => unparsedPayload.toString('utf8'),
+        },
+        {
+            // JSON
+            mime: /^application\/(?:.+\+)?json$/,
+            parse: (unparsedPayload) => {
+                if (!unparsedPayload.length) {
+                    return null;
+                }
 
-    if (mime.match(/^text\/.+$/)) {
-        return payload.toString('utf8');
+                try {
+                    return Bourne.parse(unparsedPayload.toString('utf8'), {
+                        protoAction: options.protoAction,
+                    });
+                } catch (err) {
+                    const error = Boom.badRequest(
+                        'Invalid request payload JSON format',
+                        err
+                    );
+                    error.raw = payload;
+                    throw error;
+                }
+            },
+        },
+        {
+            // Form-encoded
+            mime: 'application/x-www-form-urlencoded',
+            parse: (unparsedPayload) => {
+                const parse = options.querystring || Querystring.parse;
+                return unparsedPayload.length
+                    ? parse(unparsedPayload.toString('utf8'))
+                    : {};
+            },
+        },
+    ];
+
+    if (options.customParsers && options.customParsers.length) {
+        parsers = options.customParsers.concat(parsers);
     }
 
-    // JSON
-
-    if (/^application\/(?:.+\+)?json$/.test(mime)) {
-        if (!payload.length) {
-            return null;
-        }
-
-        try {
-            return Bourne.parse(payload.toString('utf8'), { protoAction: options.protoAction });
-        }
-        catch (err) {
-            const error = Boom.badRequest('Invalid request payload JSON format', err);
-            error.raw = payload;
-            throw error;
+    const parser = parsers.find((parser) => {
+        if (parser.mime) {
+            if (parser.mime instanceof RegExp) {
+                return parser.mime.test(mime);
+            } else if (parser.mime instanceof String) {
+                return parser.mime === mime;
+            }
         }
-    }
 
-    // Form-encoded
+        return false;
+    });
 
-    if (mime === 'application/x-www-form-urlencoded') {
-        const parse = options.querystring || Querystring.parse;
-        return payload.length ? parse(payload.toString('utf8')) : {};
+    if (parser) {
+        return await parser.parse(payload);
     }
 
     const error = Boom.unsupportedMediaType();

hapi

diff --git a/node_modules/@hapi/hapi/lib/config.js b/node_modules/@hapi/hapi/lib/config.js
index 5279a18..9cd765e 100644
--- a/node_modules/@hapi/hapi/lib/config.js
+++ b/node_modules/@hapi/hapi/lib/config.js
@@ -142,6 +142,7 @@ internals.routeBase = Validate.object({
     payload: Validate.object({
         output: Validate.valid('data', 'stream', 'file').default('data'),
         parse: Validate.boolean().allow('gunzip').default(true),
+        customParsers: Validate.array().items(Validate.object({mime: Validate.alternatives(Validate.object(), Validate.string()), parse: Validate.function()})),
         multipart: Validate.object({
             output: Validate.valid('data', 'stream', 'file', 'annotated').required()
         })

@types/hapi

diff --git a/node_modules/@types/hapi__hapi/index.d.ts b/node_modules/@types/hapi__hapi/index.d.ts
index 564ead8..acc7273 100644
--- a/node_modules/@types/hapi__hapi/index.d.ts
+++ b/node_modules/@types/hapi__hapi/index.d.ts
@@ -1306,6 +1306,18 @@ export type PayloadOutput = 'data' | 'stream' | 'file';
  */
 export type PayloadCompressionDecoderSettings = object;
 
+
+export interface RouteOptionsPayloadCustomParser {
+     /**
+     * A string or a RegExp instance to test with content-type mime of request.
+     */
+    mime: string | RegExp;
+    /**
+     * A function that takes the request payload as a Buffer, and returns the desired output.
+     */
+    parse: (unparsedPayload: Buffer) => Promise<any>;
+}
+
 /**
  * Determines how the request payload is processed.
  * [See docs](https://github.com/hapijs/hapi/blob/master/API.md#-routeoptionspayload)
@@ -1320,7 +1332,7 @@ export interface RouteOptionsPayload {
      * * multipart/form-data
      * * text/*
      * A string or an array of strings with the allowed mime types for the endpoint. Use this settings to limit the set of allowed mime types. Note that allowing additional mime types not listed
-     * above will not enable them to be parsed, and if parse is true, the request will result in an error response.
+     * above will not enable them to be parsed, and if parse is true, the request will result in an error response if a custom parser is not defined.
      * [See docs](https://github.com/hapijs/hapi/blob/master/API.md#-routeoptionspayloadallow)
      */
     allow?: string | string[] | undefined;
@@ -1403,6 +1415,12 @@ export interface RouteOptionsPayload {
      */
     parse?: boolean | 'gunzip' | undefined;
 
+    /**
+     * @default none.
+     * Add custom parsers for unsupported content-types.
+     */
+    customParsers?: RouteOptionsPayloadCustomParser[] | undefined;
+
     /**
      * @default to 10000 (10 seconds).
      * Payload reception timeout in milliseconds. Sets the maximum time allowed for the client to transmit the request payload (body) before giving up and responding with a Request Timeout (408)

@devinivy
Copy link
Member

devinivy commented Nov 1, 2021

Closing this in favor of the broader feature #90 and hapijs/hapi#4223.

@devinivy devinivy closed this as completed Nov 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New functionality or improvement
Projects
None yet
Development

No branches or pull requests

3 participants