Merge pull request #112 from grafana/fix/various-fixes-related-to-wei…

…rd-input-data

[fix] various fixes related to weird input data

src/parse/postData.js

@@ -1,7 +1,32 @@
+const { empty } = require('../aid')
 const params = require('./params')
 
+function inferMimeType(node) {
+  if (!empty(node.mimeType)) {
+    return node.mimeType
+  }
+
+  if (Array.isArray(node.params) && node.params.length > 0) {
+    return 'application/x-www-form-urlencoded'
+  }
+
+  try {
+    const body = JSON.parse(node.text)
+
+    // We only assume it to be JSON if it's an object or array, because
+    // any other value might just as well be arbitrary text.
+    if (typeof body === 'object' && body !== null) {
+      return 'application/json'
+    }
+  } catch {
+    // Must not be JSON then.
+  }
+
+  return 'text/plain'
+}
+
 function postData(node, spec) {
-  spec.type = node.mimeType
+  spec.type = inferMimeType(node)
 
   if (node.params && node.params.length) {
     spec.params = new Map()

src/parse/queryItem.js

@@ -1,11 +1,14 @@
 function queryItem(node, spec) {
   const item = {}
+
   if (node.value) {
     item.value = node.value
   }
+
   if (node.comment) {
     item.comment = node.comment
   }
+
   if (node.name) {
     if (!spec.has(node.name)) {
       spec.set(node.name, new Set())

src/parse/request.js

@@ -4,6 +4,8 @@ const queryString = require('./queryString')
 const state = require('./state/request')
 const { emptyObject, getContentTypeValue } = require('../aid')
 
+const EMPTY_BODY_METHODS = ['HEAD', 'GET', 'OPTIONS']
+
 function request(node, spec) {
   spec.method = node.method.toUpperCase()
   spec.address = node.url
@@ -20,7 +22,11 @@ function request(node, spec) {
     headers(node.headers, spec.headers)
   }
 
-  if (node.postData && !emptyObject(node.postData)) {
+  if (
+    !EMPTY_BODY_METHODS.includes(node.method) &&
+    node.postData &&
+    !emptyObject(node.postData)
+  ) {
     postData(node.postData, spec.post)
     contentType(node.postData.mimeType, spec.headers)
   }
@@ -47,7 +53,7 @@ function addBoundary(boundary, headers) {
 
   if (contentType) {
     const items = [...contentType.values()]
-    const newItems = items.map((item) => {
+    const newItems = items.map(item => {
       const value = getContentTypeValue(item.value)
       if (value === 'multipart/form-data') {
         return { value: `${value}; boundary=${boundary}` }

src/validate/param.js

@@ -17,7 +17,10 @@ function param(node, i, j) {
 function validate(node, i, j) {
   if (empty(node.name)) {
     console.warn(`[WARN] Discarding param with missing name (${i}:${j})`)
+
+    return
   }
+
   if (typeof node.name !== 'string') {
     throw new InvalidArchiveError(
       createErrorParams({
@@ -28,6 +31,7 @@ function validate(node, i, j) {
       `Param name must be a string`
     )
   }
+
   if (node.value && typeof node.value !== 'string') {
     throw new InvalidArchiveError(
       createErrorParams({
@@ -38,6 +42,7 @@ function validate(node, i, j) {
       `Param value must be a string`
     )
   }
+
   if (node.fileName && typeof node.fileName !== 'string') {
     throw new InvalidArchiveError(
       createErrorParams({
@@ -48,6 +53,7 @@ function validate(node, i, j) {
       `Param file name must be a string`
     )
   }
+
   if (node.contentType && typeof node.contentType !== 'string') {
     throw new InvalidArchiveError(
       createErrorParams({
@@ -58,6 +64,7 @@ function validate(node, i, j) {
       `Param content type must be a string`
     )
   }
+
   if (node.comment && typeof node.comment !== 'string') {
     throw new InvalidArchiveError(
       createErrorParams({

src/validate/postData.js

@@ -28,18 +28,15 @@ function postData(node, i, assay) {
 }
 
 function validate(node, i) {
-  if (empty(node.mimeType)) {
-    throw new InvalidArchiveError(
-      createErrorParams({
-        name: 'MissingPostDataType',
-        index: i,
-        path: 'mimeType',
-      }),
-      `Post data MIME type is required`
+  const mimeType = empty(node.mimeType) ? '' : node.mimeType
+
+  if (mimeType === '') {
+    console.warn(
+      `[WARN] Post data MIME type is missing and will be inferred from the content (${i})`
     )
   }
 
-  if (typeof node.mimeType !== 'string') {
+  if (typeof mimeType !== 'string') {
     throw new InvalidArchiveError(
       createErrorParams({
         name: 'InvalidPostDataType',

src/validate/queryItem.js

@@ -1,3 +1,4 @@
+const { empty } = require('../aid')
 const { InvalidArchiveError } = require('../error')
 const { createQueryStringPath } = require('./utils/path')
 const { createQueryStringIndexes } = require('./utils/indexes')
@@ -12,6 +13,12 @@ function queryItem(node, i, j) {
 }
 
 function validate(node, i, j) {
+  if (empty(node.name)) {
+    console.warn(`[WARN] Discarding query item with missing name (${i}:${j})`)
+
+    return
+  }
+
   if (typeof node.name !== 'string') {
     throw new InvalidArchiveError(
       createErrorParams({

src/validate/queryString.js

@@ -7,11 +7,11 @@ const { createQueryStringIndexes } = require('./utils/indexes')
 /*
  * [j]: object
  */
-function queryString(node, i, assay) {
+function queryString(node, i) {
   validate(node, i)
   for (let j = 0; j < node.length; j++) {
     const item = node[j]
-    queryItem(item, i, j, assay)
+    queryItem(item, i, j)
   }
 }
 

src/validate/request.js

@@ -49,6 +49,7 @@ function validate(node, i) {
       `Request method is required`
     )
   }
+
   if (typeof node.method !== 'string') {
     throw new InvalidArchiveError(
       createErrorParams({
@@ -59,24 +60,28 @@ function validate(node, i) {
       `Request method must be one of: 'GET', 'POST', 'PUT', 'PATCH', 'OPTIONS', 'DELETE', 'HEAD'`
     )
   }
+
   if (empty(node.url)) {
     throw new InvalidArchiveError(
       createErrorParams({ name: 'MissingRequestUrl', index: i, path: 'url' }),
       `Request URL is required`
     )
   }
+
   if (typeof node.url !== 'string') {
     throw new InvalidArchiveError(
       createErrorParams({ name: 'InvalidRequestUrl', index: i, path: 'url' }),
       `Request URL must be a string`
     )
   }
+
   if (!(absoluteUrl.test(node.url) || variableStart.test(node.url))) {
     throw new InvalidArchiveError(
       createErrorParams({ name: 'InvalidRequestUrl', index: i, path: 'url' }),
       `Request URL must be absolute or start with variable`
     )
   }
+
   if (node.queryString && !Array.isArray(node.queryString)) {
     throw new InvalidArchiveError(
       createErrorParams({
@@ -87,6 +92,7 @@ function validate(node, i) {
       `Request queryString must be an array`
     )
   }
+
   if (node.headers && !Array.isArray(node.headers)) {
     throw new InvalidArchiveError(
       createErrorParams({
@@ -97,6 +103,7 @@ function validate(node, i) {
       `Request headers are invalid, must be an array`
     )
   }
+
   if (node.cookies && !Array.isArray(node.cookies)) {
     throw new InvalidArchiveError(
       createErrorParams({
@@ -107,6 +114,7 @@ function validate(node, i) {
       `Request cookies are invalid, must be an array`
     )
   }
+
   if (node.postData && !isPlainObject(node.postData)) {
     throw new InvalidArchiveError(
       createErrorParams({
@@ -117,6 +125,7 @@ function validate(node, i) {
       `Request postData must be a plain object`
     )
   }
+
   if (node.comment && typeof node.comment !== 'string') {
     throw new InvalidArchiveError(
       createErrorParams({
@@ -135,14 +144,7 @@ function relation(node, i) {
     node.postData &&
     !emptyObject(node.postData)
   ) {
-    throw new InvalidArchiveError(
-      createErrorParams({
-        name: 'InvalidRequestData',
-        index: i,
-        path: 'postData',
-      }),
-      `Request postData usage with GET requests is prohibited`
-    )
+    console.warn(`[WARN] GET request has postData object (${i})`)
   }
   if (
     node.headers &&

test/unit/parse/param.test.js

@@ -5,37 +5,43 @@ function makeSpec() {
   return new Map()
 }
 
-test('minimal', (t) => {
+test('it should ignore the param if the name is empty', t => {
+  const spec = makeSpec()
+  param({ name: null }, spec)
+  t.deepEqual(spec, new Map())
+})
+
+test('it should use an empty object when only the name is given', t => {
   const spec = makeSpec()
   param({ name: 'search' }, spec)
   t.deepEqual(spec, new Map().set('search', new Set([{}])))
 })
 
-test('value', (t) => {
+test('it should set the value of params when a value is given', t => {
   const spec = makeSpec()
   param({ name: 'search', value: 'kittens' }, spec)
   t.deepEqual(spec, new Map().set('search', new Set([{ value: 'kittens' }])))
 })
 
-test('empty value', (t) => {
+test('it should set the value of params when it is empty', t => {
   const spec = makeSpec()
   param({ name: 'search', value: '' }, spec)
   t.deepEqual(spec, new Map().set('search', new Set([{ value: '' }])))
 })
 
-test('fileName', (t) => {
+test('it should set the fileName of params when a fileName is given', t => {
   const spec = makeSpec()
   param({ name: 'data', fileName: 'data.csv' }, spec)
   t.deepEqual(spec, new Map().set('data', new Set([{ fileName: 'data.csv' }])))
 })
 
-test('contentType', (t) => {
+test('it should set the contentType of params when a contentType is given', t => {
   const spec = makeSpec()
   param({ name: 'data', contentType: 'text/csv' }, spec)
   t.deepEqual(spec, new Map().set('data', new Set([{ type: 'text/csv' }])))
 })
 
-test('comment', (t) => {
+test('it should set the comment of params when a comment is given', t => {
   const spec = makeSpec()
   param({ name: 'data', comment: 'Test importing data' }, spec)
   t.deepEqual(

test/unit/parse/postData.test.js

@@ -8,21 +8,21 @@ function makeSpec() {
   return {}
 }
 
-test.serial('basic', (t) => {
+test.serial('basic', t => {
   const spec = makeSpec()
   postData({ mimeType: 'text/plain' }, spec)
   t.deepEqual(spec, { type: 'text/plain' })
   t.true(params.notCalled)
 })
 
-test.serial('text', (t) => {
+test.serial('text', t => {
   const spec = makeSpec()
   postData({ mimeType: 'text/plain', text: 'Great post' }, spec)
   t.deepEqual(spec, { type: 'text/plain', value: 'Great post' })
   t.true(params.notCalled)
 })
 
-test.serial('params', (t) => {
+test.serial('params', t => {
   const spec = makeSpec()
   postData(
     {
@@ -38,14 +38,44 @@ test.serial('params', (t) => {
   t.true(params.calledOnce)
 })
 
-test.serial('allows chartset in mimeType', (t) => {
+test.serial('allows charset in mimeType', t => {
   const spec = makeSpec()
   postData({ mimeType: 'text/plain;charset=UTF-8', text: 'Great post' }, spec)
   t.deepEqual(spec, { type: 'text/plain;charset=UTF-8', value: 'Great post' })
 })
 
-test.serial('comment', (t) => {
+test.serial('comment', t => {
   const spec = makeSpec()
   postData({ mimeType: 'text/plain', comment: 'Test post body' }, spec)
   t.deepEqual(spec, { type: 'text/plain', comment: 'Test post body' })
 })
+
+test.serial(
+  'it should assume mime-type is text/plain when mime-type is empty',
+  t => {
+    const spec = makeSpec()
+    postData({ mimeType: '', comment: 'Test post body' }, spec)
+    t.deepEqual(spec, { type: 'text/plain', comment: 'Test post body' })
+  }
+)
+
+test.serial(
+  'it should assume mime-type is application/x-www-form-urlencoded when mime-type is empty when a params array is present',
+  t => {
+    const spec = makeSpec()
+    postData({ mimeType: '', params: [{ name: 'param' }] }, spec)
+    t.deepEqual(spec, {
+      type: 'application/x-www-form-urlencoded',
+      params: new Map(),
+    })
+  }
+)
+
+test.serial(
+  'it should assume mime-type is application/json when text can be deserialized to a JSON-object',
+  t => {
+    const spec = makeSpec()
+    postData({ mimeType: '', text: '{ "prop": 1 }' }, spec)
+    t.deepEqual(spec, { type: 'application/json', value: '{ "prop": 1 }' })
+  }
+)