Merge pull request #562 from ipfs-shipyard/fix/files.add-in-chrome

Workaround for 'invalid Read on closed Body' in files.add (webext in Chrome)

> Chrome does allow setting `Expect` via [`onBeforeSendHeaders`](https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/webRequest/onBeforeSendHeaders) WebExtension API

This means we now have a workaround for #480 that works in both Firefox and Chromium.

Details: #480 (comment) & ipfs/kubo#5168 (comment)

Closes #480

Author: lidel

add-on/src/lib/ipfs-request.js

@@ -82,19 +82,43 @@ function createRequestModifier (getState, dnslinkResolver, ipfsPathValidator, ru
         // There is a bug in go-ipfs related to keep-alive connections
         // that results in partial response for ipfs.files.add
         // mangled by error "http: invalid Read on closed Body"
-        // More info: https://github.com/ipfs/go-ipfs/issues/5168
-        if (request.url.includes('/api/v0/add')) {
+        // More info (ipfs-companion): https://github.com/ipfs-shipyard/ipfs-companion/issues/480
+        // More info (go-ipfs): https://github.com/ipfs/go-ipfs/issues/5168
+        if (request.url.includes('/api/v0/add') && request.url.includes('stream-channels=true')) {
+          let addExpectHeader = true
+          const expectHeader = { name: 'Expect', value: '100-continue' }
+          const warningMsg = '[ipfs-companion] Executing "Expect: 100-continue" workaround for ipfs.files.add due to https://github.com/ipfs/go-ipfs/issues/5168'
           for (let header of request.requestHeaders) {
-            if (header.name === 'Connection') {
+            // Workaround A: https://github.com/ipfs/go-ipfs/issues/5168#issuecomment-401417420
+            // (works in Firefox, but Chromium does not expose Connection header)
+            /* (disabled so we use the workaround B in all browsers)
+            if (header.name === 'Connection' && header.value !== 'close') {
               console.warn('[ipfs-companion] Executing "Connection: close" workaround for ipfs.files.add due to https://github.com/ipfs/go-ipfs/issues/5168')
               header.value = 'close'
+              addExpectHeader = false
               break
             }
+            */
+            // Workaround B: https://github.com/ipfs-shipyard/ipfs-companion/issues/480#issuecomment-417657758
+            // (works in Firefox 63 AND Chromium 67)
+            if (header.name === expectHeader.name) {
+              addExpectHeader = false
+              if (header.value !== expectHeader.value) {
+                console.log(warningMsg)
+                header.value = expectHeader.value
+              }
+              break
+            }
+          }
+          if (addExpectHeader) {
+            console.log(warningMsg)
+            request.requestHeaders.push(expectHeader)
           }
         }
         // For some reason js-ipfs-api sent requests with "Origin: null" under Chrome
         // which produced '403 - Forbidden' error.
         // This workaround removes bogus header from API requests
+        // TODO: check if still necessary
         for (let i = 0; i < request.requestHeaders.length; i++) {
           let header = request.requestHeaders[i]
           if (header.name === 'Origin' && (header.value == null || header.value === 'null')) {

test/functional/lib/ipfs-request-workarounds.test.js

@@ -0,0 +1,64 @@
+'use strict'
+const { describe, it, before, beforeEach, after } = require('mocha')
+const { expect } = require('chai')
+const { URL } = require('url')
+const browser = require('sinon-chrome')
+const { initState } = require('../../../add-on/src/lib/state')
+const createRuntimeChecks = require('../../../add-on/src/lib/runtime-checks')
+const { createRequestModifier } = require('../../../add-on/src/lib/ipfs-request')
+const createDnslinkResolver = require('../../../add-on/src/lib/dnslink')
+const { createIpfsPathValidator } = require('../../../add-on/src/lib/ipfs-path')
+const { optionDefaults } = require('../../../add-on/src/lib/options')
+
+// const nodeTypes = ['external', 'embedded']
+
+describe('modifyRequest processing', function () {
+  let state, dnslinkResolver, ipfsPathValidator, modifyRequest, runtime
+
+  before(function () {
+    global.URL = URL
+    global.browser = browser
+  })
+
+  beforeEach(async function () {
+    state = initState(optionDefaults)
+    const getState = () => state
+    dnslinkResolver = createDnslinkResolver(getState)
+    runtime = Object.assign({}, await createRuntimeChecks(browser)) // make it mutable for tests
+    ipfsPathValidator = createIpfsPathValidator(getState, dnslinkResolver)
+    modifyRequest = createRequestModifier(getState, dnslinkResolver, ipfsPathValidator, runtime)
+  })
+
+  describe('a request to <apiURL>/api/v0/add with stream-channels=true', function () {
+    const expectHeader = { name: 'Expect', value: '100-continue' }
+    it('should apply the "Expect: 100-continue" fix for https://github.com/ipfs/go-ipfs/issues/5168 ', function () {
+      const request = {
+        method: 'POST',
+        requestHeaders: [
+          { name: 'Content-Type', value: 'multipart/form-data; boundary=--------------------------015695779813832455524979' }
+        ],
+        type: 'xmlhttprequest',
+        url: `${state.apiURLString}api/v0/add?progress=true&wrapWithDirectory=true&pin=true&wrap-with-directory=true&stream-channels=true`
+      }
+      expect(modifyRequest.onBeforeSendHeaders(request).requestHeaders).to.deep.include(expectHeader)
+    })
+  })
+
+  describe('a request to <apiURL>/api/v0/ with Origin=null', function () {
+    it('should remove Origin header ', function () {
+      const bogusOriginHeader = { name: 'Origin', value: 'null' }
+      const request = {
+        requestHeaders: [ bogusOriginHeader ],
+        type: 'xmlhttprequest',
+        url: `${state.apiURLString}api/v0/id`
+      }
+      expect(modifyRequest.onBeforeSendHeaders(request).requestHeaders).to.not.include(bogusOriginHeader)
+    })
+  })
+
+  after(function () {
+    delete global.URL
+    delete global.browser
+    browser.flush()
+  })
+})