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

fix: allow reading body from non-2xx responses in net.request #21055

Merged
merged 5 commits into from Nov 25, 2019
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions shell/browser/api/atom_api_url_request.cc
Expand Up @@ -286,6 +286,8 @@ bool URLRequest::Write(v8::Local<v8::Value> data, bool is_last) {
network::ResourceRequest* request_ref = request_.get();
loader_ = network::SimpleURLLoader::Create(std::move(request_),
kTrafficAnnotation);

loader_->SetAllowHttpErrorResults(true);
loader_->SetOnResponseStartedCallback(
base::Bind(&URLRequest::OnResponseStarted, weak_factory_.GetWeakPtr()));
loader_->SetOnRedirectCallback(
Expand Down
66 changes: 59 additions & 7 deletions spec-main/api-net-spec.ts
Expand Up @@ -219,26 +219,31 @@ describe('net module', () => {
expect(loginAuthInfo.scheme).to.equal('basic')
})

it('should produce an error on the response object when cancelling authentication', async () => {
it('should response when cancelling authentication', async () => {
const serverUrl = await respondOnce.toSingleURL((request, response) => {
if (!request.headers.authorization) {
return response.writeHead(401, { 'WWW-Authenticate': 'Basic realm="Foo"' }).end()
response.writeHead(401, { 'WWW-Authenticate': 'Basic realm="Foo"' })
response.end('unauthenticated')
} else {
response.writeHead(200).end('ok')
}
response.writeHead(200).end('ok')
})
await expect(new Promise((resolve, reject) => {
expect(await new Promise((resolve, reject) => {
const request = net.request({ method: 'GET', url: serverUrl })
request.on('response', (response) => {
let data = ''
response.on('error', reject)
response.on('data', () => {})
response.on('end', () => resolve())
response.on('data', (chunk) => {
data += chunk
})
response.on('end', () => resolve(data))
})
request.on('login', (authInfo, cb) => {
cb()
})
request.on('error', reject)
request.end()
})).to.eventually.be.rejectedWith('net::ERR_HTTP_RESPONSE_CODE_FAILURE')
})).to.equal('unauthenticated')
})

it('should share credentials with WebContents', async () => {
Expand Down Expand Up @@ -717,6 +722,53 @@ describe('net module', () => {
expect(abortsEmitted).to.equal(1, 'request should emit exactly 1 "abort" event')
})

it('should allow to read response body from non-2xx response', async () => {
const bodyData = randomString(kOneKiloByte)
const serverUrl = await respondOnce.toSingleURL((request, response) => {
response.statusCode = 404
response.end(bodyData)
})

let requestResponseEventEmitted = false
let responseDataEventEmitted = false

const urlRequest = net.request(serverUrl)
const eventHandlers = Promise.all([
emittedOnce(urlRequest, 'response')
.then(async (params: any[]) => {
const response: Electron.IncomingMessage = params[0]
requestResponseEventEmitted = true
const statusCode = response.statusCode
expect(statusCode).to.equal(404)
const buffers: Buffer[] = []
response.on('data', (chunk) => {
buffers.push(chunk)
responseDataEventEmitted = true
})
await new Promise((resolve, reject) => {
response.on('error', () => {
reject(new Error('error emitted'))
})
emittedOnce(response, 'end')
.then(() => {
const receivedBodyData = Buffer.concat(buffers)
expect(receivedBodyData.toString()).to.equal(bodyData)
})
.then(resolve)
.catch(reject)
})
}),
emittedOnce(urlRequest, 'close')
])

urlRequest.end()

await eventHandlers

expect(requestResponseEventEmitted).to.equal(true)
expect(responseDataEventEmitted).to.equal(true)
})

describe('webRequest', () => {
afterEach(() => {
session.defaultSession.webRequest.onBeforeRequest(null)
Expand Down
11 changes: 9 additions & 2 deletions spec-main/api-session-spec.ts
Expand Up @@ -511,12 +511,19 @@ describe('session module', () => {
const fetch = (url: string) => new Promise((resolve, reject) => {
const request = net.request({ url, session: ses })
request.on('response', (response) => {
let data = ''
let data: string | null = null
response.on('data', (chunk) => {
if (!data) {
data = ''
}
data += chunk
})
response.on('end', () => {
resolve(data)
if (!data) {
reject(new Error('Empty response'))
} else {
resolve(data)
}
})
response.on('error', (error: any) => { reject(new Error(error)) })
})
Expand Down