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

Ensure GraphQLUpload scalar errors when parsing invalid values #181

Merged
merged 4 commits into from Jan 19, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.md
Expand Up @@ -9,6 +9,7 @@
- Replaced the [`tap`](https://npm.im/tap) dev dependency with [`test-director`](https://npm.im/test-director), [`coverage-node`](https://npm.im/coverage-node), and [`hard-rejection`](https://npm.im/hard-rejection) to improve the dev experience and reduce the dev install size by ~75.7 MB. These new dev dependencies require Node.js v10+.
- Reorganized files. This is only a breaking change for projects using undocumented deep imports.
- Removed now redundant Node.js version compatibility logic in the `processRequest` function.
- The `processRequest` function now places references to instances of the now exported and documented `Upload` class in the GraphQL operation for the `GraphQLUpload` scalar to derive its value, and the `GraphQLUpload` scalar now throws a `GraphQLError` when it parses an invalid value, fixing [#175](https://github.com/jaydenseric/graphql-upload/issues/175) via [#181](https://github.com/jaydenseric/graphql-upload/pull/181).
- The `GraphQLUpload` scalar `parseLiteral` and `serialize` methods now throw `GraphQLError` (instead of `Error`) instances, with tweaked messages.

### Patch
Expand Down
6 changes: 5 additions & 1 deletion lib/GraphQLUpload.js
@@ -1,6 +1,7 @@
'use strict'

const { GraphQLScalarType, GraphQLError } = require('graphql')
const Upload = require('./Upload')

/**
* A GraphQL `Upload` scalar that can be used in a
Expand Down Expand Up @@ -60,7 +61,10 @@ const { GraphQLScalarType, GraphQLError } = require('graphql')
module.exports = new GraphQLScalarType({
name: 'Upload',
description: 'The `Upload` scalar type represents a file upload.',
parseValue: value => value,
parseValue(value) {
if (value instanceof Upload) return value.promise
throw new GraphQLError('Upload value invalid.')
},
parseLiteral(ast) {
throw new GraphQLError('Upload literal unsupported.', ast)
},
Expand Down
25 changes: 15 additions & 10 deletions lib/Upload.js
@@ -1,48 +1,53 @@
'use strict'

/**
* An expected file upload.
* A file expected to be uploaded as it has been declared in the `map` field of
* a [GraphQL multipart request](https://github.com/jaydenseric/graphql-multipart-request-spec).
* The [`processRequest`]{@link processRequest} function places references to an
* instance of this class wherever the file is expected in the
* [GraphQL operation]{@link GraphQLOperation}. The
* [`Upload` scalar]{@link GraphQLUpload} derives it’s value from the
* [`promise`]{@link Upload#promise} property.
* @kind class
* @name Upload
* @ignore
*/
module.exports = class Upload {
constructor() {
/**
* Promise that resolves file upload details.
* Promise that resolves file upload details. This should only be utilized
* by [`GraphQLUpload`]{@link GraphQLUpload}.
* @kind member
* @name Upload#promise
* @type {Promise<FileUpload>}
* @ignore
*/
this.promise = new Promise((resolve, reject) => {
/**
* Resolves the upload promise with the file upload details.
* Resolves the upload promise with the file upload details. This should
* only be utilized by [`processRequest`]{@link processRequest}.
* @kind function
* @name Upload#resolve
* @param {FileUpload} file File upload details.
* @ignore
*/
this.resolve = file => {
/**
* The file upload details, available when the
* [upload promise]{@link Upload#promise} resolves.
* [upload promise]{@link Upload#promise} resolves. This should only be
* utilized by [`processRequest`]{@link processRequest}.
* @kind member
* @name Upload#file
* @type {undefined|FileUpload}
* @ignore
*/
this.file = file

resolve(file)
}

/**
* Rejects the upload promise with an error.
* Rejects the upload promise with an error. This method should only be
* utilized by [`processRequest`]{@link processRequest}.
* @kind function
* @name Upload#reject
* @param {object} error Error instance.
* @ignore
*/
this.reject = reject
})
Expand Down
2 changes: 2 additions & 0 deletions lib/index.js
Expand Up @@ -4,6 +4,7 @@ exports.GraphQLUpload = require('./GraphQLUpload')
exports.processRequest = require('./processRequest')
exports.graphqlUploadKoa = require('./graphqlUploadKoa')
exports.graphqlUploadExpress = require('./graphqlUploadExpress')
exports.Upload = require('./Upload')

/**
* File upload details that are only available after the file’s field in the
Expand Down Expand Up @@ -40,6 +41,7 @@ exports.graphqlUploadExpress = require('./graphqlUploadExpress')
* @param {ServerResponse} response [Node.js HTTP server response instance](https://nodejs.org/api/http.html#http_class_http_serverresponse).
* @param {ProcessRequestOptions} [options] Options for processing the request.
* @returns {Promise<GraphQLOperation | Array<GraphQLOperation>>} GraphQL operation or batch of operations for a GraphQL server to consume (usually as the request body).
* @see [`processRequest`]{@link processRequest}.
*/

/**
Expand Down
15 changes: 10 additions & 5 deletions lib/processRequest.js
Expand Up @@ -11,11 +11,16 @@ const ignoreStream = require('./ignoreStream')

/**
* Processes a [GraphQL multipart request](https://github.com/jaydenseric/graphql-multipart-request-spec).
* Errors are created with [`http-errors`](https://npm.im/http-errors) to assist
* in sending responses with appropriate HTTP status codes. Used in
* It parses the `operations` and `map` fields to create an
* [`Upload`]{@link Upload} instance for each expected file upload, placing
* references wherever the file is expected in the
* [GraphQL operation]{@link GraphQLOperation} for the
* [`Upload` scalar]{@link GraphQLUpload} to derive it’s value. Errors are
* created with [`http-errors`](https://npm.im/http-errors) to assist in
* sending responses with appropriate HTTP status codes. Used in
* [`graphqlUploadExpress`]{@link graphqlUploadExpress} and
* [`graphqlUploadKoa`]{@link graphqlUploadKoa} and can be used to
* create custom middleware.
* [`graphqlUploadKoa`]{@link graphqlUploadKoa} and can be used to create
* custom middleware.
* @kind function
* @name processRequest
* @type {ProcessRequestFunction}
Expand Down Expand Up @@ -211,7 +216,7 @@ module.exports = function processRequest(
)

try {
operationsPath.set(path, map.get(fieldName).promise)
operationsPath.set(path, map.get(fieldName))
} catch (error) {
return exit(
createError(
Expand Down
45 changes: 44 additions & 1 deletion readme.md
Expand Up @@ -66,6 +66,11 @@ The [GraphQL multipart request spec](https://github.com/jaydenseric/graphql-mult
### Table of contents

- [class GraphQLUpload](#class-graphqlupload)
- [class Upload](#class-upload)
- [Upload instance method reject](#upload-instance-method-reject)
- [Upload instance method resolve](#upload-instance-method-resolve)
- [Upload instance property file](#upload-instance-property-file)
- [Upload instance property promise](#upload-instance-property-promise)
- [function graphqlUploadExpress](#function-graphqluploadexpress)
- [function graphqlUploadKoa](#function-graphqluploadkoa)
- [function processRequest](#function-processrequest)
Expand Down Expand Up @@ -133,6 +138,40 @@ _A manually constructed schema with an image upload mutation._

---

### class Upload

A file expected to be uploaded as it has been declared in the `map` field of a [GraphQL multipart request](https://github.com/jaydenseric/graphql-multipart-request-spec). The [`processRequest`](#function-processrequest) function places references to an instance of this class wherever the file is expected in the [GraphQL operation](#type-graphqloperation). The [`Upload` scalar](#class-graphqlupload) derives it’s value from the [`promise`](#upload-instance-property-promise) property.

#### Upload instance method reject

Rejects the upload promise with an error. This method should only be utilized by [`processRequest`](#function-processrequest).

| Parameter | Type | Description |
| :-------- | :----- | :-------------- |
| `error` | object | Error instance. |

#### Upload instance method resolve

Resolves the upload promise with the file upload details. This should only be utilized by [`processRequest`](#function-processrequest).

| Parameter | Type | Description |
| :-------- | :----------------------------- | :------------------- |
| `file` | [FileUpload](#type-fileupload) | File upload details. |

#### Upload instance property file

The file upload details, available when the [upload promise](#upload-instance-property-promise) resolves. This should only be utilized by [`processRequest`](#function-processrequest).

**Type:** `undefined` | [FileUpload](#type-fileupload)

#### Upload instance property promise

Promise that resolves file upload details. This should only be utilized by [`GraphQLUpload`](#class-graphqlupload).

**Type:** Promise&lt;[FileUpload](#type-fileupload)>

---

### function graphqlUploadExpress

Creates [Express](https://expressjs.com) middleware that processes [GraphQL multipart requests](https://github.com/jaydenseric/graphql-multipart-request-spec) using [`processRequest`](#function-processrequest), ignoring non-multipart requests. It sets the request body to be [similar to a conventional GraphQL POST request](#type-graphqloperation) for following GraphQL middleware to consume.
Expand Down Expand Up @@ -199,7 +238,7 @@ _Basic [`graphql-api-koa`](https://npm.im/graphql-api-koa) setup._

### function processRequest

Processes a [GraphQL multipart request](https://github.com/jaydenseric/graphql-multipart-request-spec). Errors are created with [`http-errors`](https://npm.im/http-errors) to assist in sending responses with appropriate HTTP status codes. Used in [`graphqlUploadExpress`](#function-graphqluploadexpress) and [`graphqlUploadKoa`](#function-graphqluploadkoa) and can be used to create custom middleware.
Processes a [GraphQL multipart request](https://github.com/jaydenseric/graphql-multipart-request-spec). It parses the `operations` and `map` fields to create an [`Upload`](#class-upload) instance for each expected file upload, placing references wherever the file is expected in the [GraphQL operation](#type-graphqloperation) for the [`Upload` scalar](#class-graphqlupload) to derive it’s value. Errors are created with [`http-errors`](https://npm.im/http-errors) to assist in sending responses with appropriate HTTP status codes. Used in [`graphqlUploadExpress`](#function-graphqluploadexpress) and [`graphqlUploadKoa`](#function-graphqluploadkoa) and can be used to create custom middleware.

**Type:** [ProcessRequestFunction](#type-processrequestfunction)

Expand Down Expand Up @@ -261,6 +300,10 @@ Processes a [GraphQL multipart request](https://github.com/jaydenseric/graphql-m

**Returns:** Promise&lt;[GraphQLOperation](#type-graphqloperation) | Array&lt;[GraphQLOperation](#type-graphqloperation)>> — GraphQL operation or batch of operations for a GraphQL server to consume (usually as the request body).

#### See

- [`processRequest`](#function-processrequest).

---

### type ProcessRequestOptions
Expand Down
24 changes: 23 additions & 1 deletion test/lib/GraphQLUpload.test.js
@@ -1,10 +1,32 @@
'use strict'

const { throws } = require('assert')
const { doesNotThrow, throws } = require('assert')
const { parseValue } = require('graphql')
const GraphQLUpload = require('../../lib/GraphQLUpload')
const Upload = require('../../lib/Upload')

module.exports = tests => {
tests.add('`GraphQLUpload` scalar `parseValue` with a valid value.', () => {
doesNotThrow(() => {
GraphQLUpload.parseValue(new Upload())
})
})

tests.add(
'`GraphQLUpload` scalar `parseValue` with an invalid value.',
() => {
throws(
() => {
GraphQLUpload.parseValue(true)
},
{
name: 'GraphQLError',
message: 'Upload value invalid.'
}
)
}
)

tests.add('`GraphQLUpload` scalar `parseLiteral`.', () => {
throws(
() => {
Expand Down