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 body limit #12

Open
fiznool opened this issue Oct 1, 2015 · 4 comments
Open

Support body limit #12

fiznool opened this issue Oct 1, 2015 · 4 comments

Comments

@fiznool
Copy link
Contributor

fiznool commented Oct 1, 2015

This is something supported by the regular express body parser. It would be nice to limit the body size, e.g.

app.use(xmlBodyParser({ limit: '1MB' }));

I'd be keen to help out with a PR if you could point me in the right direction. The README mentions using raw-body - is this the way forward?

@macedigital
Copy link
Owner

Yes, raw-body integration is planned (soon-ish).

The only thing I'm not 100% sure of is how the option passing should work:

  • use a flat-map, pass raw-body and xml2js options in the same config object
  • use a separate key for passing options for raw-body
  • use a second argument exclusively for raw-body options

Actually, option 3 looks like the cleanest solution to me, as it preserves the established api and will never cause issues if either dependency would introduce an option with the same name.

@fiznool let me know what you think. 👍 if you would provide a PR.

@fiznool
Copy link
Contributor Author

fiznool commented Oct 9, 2015

@macedigital I think mirroring the original express-body-parser options, with an extra xmlOptions property in the object would be best.

{
  limit: '1MB',
  xmlOptions: {
    async: true,
    // etc
  }
}

Regarding a PR, I've decided not to use this module after all, it turns out that XML parsing with body-parser is actually pretty easy. I've included a code snippet below in case this is useful for anybody.

Regardless, thanks @macedigital for this module!

var express = require('express'),
    typeis = require('type-is');

var app = express();

var xmlParseOptions = {
  async: false,   // Setting to `true` causes issues with exceptions
  explicitArray: false,
  trim: true,
  normalize: true,
  normalizeTags: true,
  mergeAttrs: true,
  charkey: 'value',
  attrNameProcessors: [function(attr) {
    return '@' + attr;
  }]
};

app.use(bodyParser.text({
  type: '*/xml',
  limit: '1MB'
}));

app.use(function(req, res, next) {
  if(!typeis.hasBody(req) || !typeis(req, '*.xml')) {
    return next();
  }
   // Parse as XML
  var parser = new xml2js.Parser(xmlParseOptions);
  parser.parseString(req.body, function(err, xml) {
    if(err) {
      err.status = 400;
      return next(err);
    }
    req.body = xml || req.body;
    next();
  });
});

@macedigital
Copy link
Owner

I created a gist https://gist.github.com/macedigital/3799eec3c25a29692a25ee021457b171, so this alternative solution doesn't get lost. If time permits, support for limiting body size will arrive soon.

@fiznool
Copy link
Contributor Author

fiznool commented Jan 14, 2022

Just to mention that I published body-parser-xml a while ago, which fully supports the payload size limiting option, along with every other option that the mainbody-parser package supports.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants