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

Write after end when piping #50

Closed
rexxars opened this issue Dec 19, 2016 · 11 comments
Closed

Write after end when piping #50

rexxars opened this issue Dec 19, 2016 · 11 comments
Assignees
Labels

Comments

@rexxars
Copy link
Contributor

rexxars commented Dec 19, 2016

I have a fairly simple case where I'm building a tarball-stream on the fly and piping it to a request that's going through follow-redirects.

It almost immediately crashes with this error:

Error: write after end
    at ClientRequest.OutgoingMessage.write (_http_outgoing.js:439:15)
    at Writable.RedirectableRequest._write (/home/espenh/webdev/sanity/cli/node_modules/follow-redirects/index.js:170:23)
    at doWrite (_stream_writable.js:333:12)
    at clearBuffer (_stream_writable.js:440:7)
    at onwrite (_stream_writable.js:372:7)
    at WritableState.onwrite (_stream_writable.js:89:5)
    at CorkedRequest.finish (_stream_writable.js:547:7)
    at afterWrite (_stream_writable.js:387:3)
    at onwrite (_stream_writable.js:378:7)
    at TLSSocket.WritableState.onwrite (_stream_writable.js:89:5)

I'll see if I can find time to write a reduced test case at some point, but don't have time right at this moment. Switching to regular HTTP/HTTPS for this request fixes the problem.

@RubenVerborgh
Copy link
Collaborator

Thanks for reporting. If you don't find the time for a reduced test case, you can also send me your current code in private.

@RubenVerborgh
Copy link
Collaborator

Closing this because of inactivity.

jcready added a commit to jcready/axios that referenced this issue Apr 8, 2017
Using the follow-redirects 1.0.0 causes this reported write after end issue: follow-redirects/follow-redirects#50. It looks like the problem with follow-redirects was fixed in 1.1.0 follow-redirects/follow-redirects@9eec6f0 but if axios is going to update the dependency it might as well update to the latest version now.
@fogine
Copy link

fogine commented May 30, 2017

Hello @RubenVerborgh ,

we have reproduced the issue with follow-redirects@1.2.3, HERE is the code.
Run node index.js
Substantial information is in the index.js

nodejs 6.8

Thanks for your contributions to OS!

@RubenVerborgh RubenVerborgh reopened this May 30, 2017
@RubenVerborgh
Copy link
Collaborator

Thanks, will have a look.

@rexxars
Copy link
Contributor Author

rexxars commented May 30, 2017

Thanks for making this reproducible @fogine - I'm sorry I never got around to doing so.

@olalonde
Copy link
Contributor

olalonde commented Jun 20, 2017

UPDATE: I created a test that reproduces this issue here: https://travis-ci.org/binded/follow-redirects-test

Hey @RubenVerborgh , I also stumbled upon this bug through axios: axios/axios#964

Full test case (indirectly, using axios :/):

const test = require('blue-tape')
const axios = require('axios')
const express = require('express')
const { promisify } = require('util')
const concat = require('concat-stream')
const FormData = require('form-data')
const axiosVersion = require('axios/package.json').version

const listen = (app) => new Promise((resolve, reject) => {
  const server = app.listen((err) => {
    if (err) return reject(err)
    resolve(server)
  })
})

let closeServer
let baseURL
let port
let client
let server

const startServer = async () => {
  const app = express()

  app.post('/upload', (req, res) => {
    // console.error(req.headers)
    req.pipe(concat(() => {
      res.json({ headers: req.headers })
    }))
  })

  server = await listen(app)

  closeServer = promisify(server.close.bind(server))
  port = server.address().port
  baseURL = `http://localhost:${port}`
  client = axios.create({
    baseURL,
    /*
    headers: {
      'x-identity-token': 'test123',
    },
    */
  })
  return server
}

test('start server', startServer)

test('upload file', async (t) => {
  const buf = new Buffer('deadbeef', 'hex')
  const form = new FormData()
  form.append('some-image', buf, {
    filename: 'some-image.jpg',
    contentType: 'image/jpeg',
  })

  const headers = form.getHeaders()
  const boundaryId = headers['content-type'].substr(-24)
  t.deepEqual(headers, {
    /* eslint-disable max-len */
    'content-type': `multipart/form-data; boundary=--------------------------${boundaryId}`,
  })

  const response = await client.post('/upload', form, { headers })

  t.deepEqual(response.data, {
    headers: {
      accept: 'application/json, text/plain, */*',
      connection: 'close',
      'content-type': `multipart/form-data; boundary=--------------------------${boundaryId}`,
      host: `localhost:${port}`,
      'transfer-encoding': 'chunked',
      'user-agent': `axios/${axiosVersion}`,
    },
  })
})

test('stop server', async () => {
  await closeServer()
})

The bug was introduced by follow-redirects@0.3.0 v0.2.0...v0.3.0

@RubenVerborgh
Copy link
Collaborator

Thanks. I will create a fix soon.

@RubenVerborgh
Copy link
Collaborator

This fixes @olalonde's test case. Please verify.

@olalonde
Copy link
Contributor

@RubenVerborgh thanks!

@RubenVerborgh
Copy link
Collaborator

@rwpino Thanks, replied there.

lily-white added a commit to lily-white/xiaoshentong that referenced this issue Jun 5, 2021
Using the follow-redirects 1.0.0 causes this reported write after end issue: follow-redirects/follow-redirects#50. It looks like the problem with follow-redirects was fixed in 1.1.0 follow-redirects/follow-redirects@9eec6f0 but if axios is going to update the dependency it might as well update to the latest version now.
redsky030511 added a commit to redsky030511/axios-server that referenced this issue May 13, 2024
Using the follow-redirects 1.0.0 causes this reported write after end issue: follow-redirects/follow-redirects#50. It looks like the problem with follow-redirects was fixed in 1.1.0 follow-redirects/follow-redirects@9eec6f0 but if axios is going to update the dependency it might as well update to the latest version now.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants