2018年11月17日 星期六

Submit a rails ActiveStorage issue

https://github.com/rails/rails/issues/34470



### Steps to reproduce

When passing invalid file object to initialize DirectUpload instance, it will fail silently while creating because FileChecksum fail to acknowledge it's a invalid object.

### Expected behavior

I expect when I passing a invalid file object to initialize DirectUpload instance to create upload, it should show error message.

```js
// This result is what I got from UppyJS, it doesn't contains real file data but it still provide meta data like file mimeType, name...etc

const result = {
  "source": "GoogleDrive",
  "id": "uppy-kittyjpg-image/jpeg",
  "name": "kitty.jpg",
  "extension": "jpg",
  "meta": {
    "name": "kitty.jpg",
    "type": "image/jpeg"
  },
  "type": "image/jpeg",
  "data": {
    "isFolder": false,
    "icon": "https://drive-thirdparty.googleusercontent.com/16/type/image/jpeg",
    "name": "kitty.jpg",
    "mimeType": "image/jpeg",
    "id": "1Q61JlL7ojCoV3zqZCWFOTaJe054LJ_GU",
    "thumbnail": "http://localhost:3020/drive/thumbnail/1Q61JlL7ojCoV3zqZCWFOTaJe054LJ_GU",
    "requestPath": "1Q61JlL7ojCoV3zqZCWFOTaJe054LJ_GU",
    "modifiedDate": "2018-11-07T15:06:35.239Z",
    "custom": {
      "isTeamDrive": false
    }
  },
  "progress": {
    "percentage": 0,
    "bytesUploaded": 0,
    "bytesTotal": 0,
    "uploadComplete": false,
    "uploadStarted": false
  },
  "size": 0,
  "isRemote": true,
  "remote": {
    "serverUrl": "http://localhost:3020",
    "url": "http://localhost:3020/drive/get/1Q61JlL7ojCoV3zqZCWFOTaJe054LJ_GU",
    "body": {
      "fileId": "1Q61JlL7ojCoV3zqZCWFOTaJe054LJ_GU"
    },
    "providerOptions": {
      "serverUrl": "http://localhost:3020",
      "provider": "drive",
      "authProvider": "google"
    }
  },
  "preview": "http://localhost:3020/drive/thumbnail/1Q61JlL7ojCoV3zqZCWFOTaJe054LJ_GU"
}

const uploadFile = (file) {
  const url = input.dataset.directUploadUrl
  const upload = new DirectUpload(file, url)

  upload.create((error, blob) => {
      if (error) {
          console.log("Error!!")
      } else {
          console.log("No Error")
      }
  })
}

uploadFile(result.data) // => Should print something
```

### Actual behavior

```js
const uploadFile = (file) {
  const url = input.dataset.directUploadUrl
  const upload = new DirectUpload(file, url)

  upload.create((error, blob) => {
      if (error) {
          console.log("Error!!")
      } else {
          console.log("No Error")
      }
  })
}

uploadFile(result.data) // => Nothing printed
```

### Digging into it

The reason there are no error is because `rails/activestorage/app/javascript/activestorage/file_checksum.js`, in `create` function it only add error listener for fileReader, however, when passing invalid file object into FileChecksum, in this case, in line 14, `this.file.size` is `undefined` so `this.chunkCount` will be `NaN` at the first place, it never had chance to raise error inside fileReader.

According to https://github.com/rails/rails/blob/master/activestorage/app/javascript/activestorage/file_checksum.js#L42

It will just return false and invoke no callback.

I don't know if it's designed by purpose or maybe we can add some check to make sure file object is valid? It might not be an issue if you know what you're doing very well, however, it did take me a while to investigate it while I was trying to integrate Uppy with ActiveStorage, so I think it's worth to mention or add checking mechanism into codebase. Any thought? I'm happy to send a PR for this if you think it's worth to check.


### System configuration
**Rails version**:
5.2.1

**Ruby version**:
2.5.1

**ActiveStorage npm package version**
5.2.1

沒有留言:

張貼留言