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

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

Merged
merged 2 commits into from
Sep 3, 2018
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 27 additions & 3 deletions add-on/src/lib/ipfs-request.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')) {
Expand Down
64 changes: 64 additions & 0 deletions test/functional/lib/ipfs-request-workarounds.test.js
Original file line number Diff line number Diff line change
@@ -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()
})
})