From ff20fb09de235ff53554e5bc8e6b8c7ea8034f06 Mon Sep 17 00:00:00 2001 From: Marcin Rataj Date: Fri, 31 Aug 2018 16:01:42 +0200 Subject: [PATCH] fix: 'invalid Read on closed Body' in files.add under Chrome Details: https://github.com/ipfs-shipyard/ipfs-companion/issues/480#issuecomment-417657758 --- add-on/src/lib/ipfs-request.js | 30 ++++++++- .../lib/ipfs-request-workarounds.test.js | 64 +++++++++++++++++++ 2 files changed, 91 insertions(+), 3 deletions(-) create mode 100644 test/functional/lib/ipfs-request-workarounds.test.js diff --git a/add-on/src/lib/ipfs-request.js b/add-on/src/lib/ipfs-request.js index 8f25f4a08..a5c38da97 100644 --- a/add-on/src/lib/ipfs-request.js +++ b/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')) { diff --git a/test/functional/lib/ipfs-request-workarounds.test.js b/test/functional/lib/ipfs-request-workarounds.test.js new file mode 100644 index 000000000..774e04944 --- /dev/null +++ b/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 /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 /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() + }) +})