Skip to content

Commit

Permalink
fix(Network): explicitly throw on content request for redirect respon…
Browse files Browse the repository at this point in the history
…se (puppeteer#2352)

DevTools protocol doesn't support returning body of redirect responses.
We should explicitly throw in this case.

References puppeteer#1896.
  • Loading branch information
aslushnikov authored Apr 11, 2018
1 parent 18f2ecd commit f8cba45
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 6 deletions.
18 changes: 12 additions & 6 deletions lib/NetworkManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,7 @@ class NetworkManager extends EventEmitter {
const response = new Response(this._client, request, redirectStatus, redirectHeaders, fromDiskCache, fromServiceWorker, securityDetails);
request._response = response;
request._redirectChain.push(request);
response._bodyLoadedPromiseFulfill.call(null, new Error('Response body is unavailable for redirect responses'));
this._requestIdToRequest.delete(request._requestId);
this._interceptionIdToRequest.delete(request._interceptionId);
this._attemptedAuthentications.delete(request._interceptionId);
Expand Down Expand Up @@ -274,7 +275,7 @@ class NetworkManager extends EventEmitter {
// @see https://crbug.com/750469
if (!request)
return;
request._completePromiseFulfill.call(null);
request.response()._bodyLoadedPromiseFulfill.call(null);
this._requestIdToRequest.delete(request._requestId);
this._interceptionIdToRequest.delete(request._interceptionId);
this._attemptedAuthentications.delete(request._interceptionId);
Expand All @@ -291,7 +292,9 @@ class NetworkManager extends EventEmitter {
if (!request)
return;
request._failureText = event.errorText;
request._completePromiseFulfill.call(null);
const response = request.response();
if (response)
response._bodyLoadedPromiseFulfill.call(null);
this._requestIdToRequest.delete(request._requestId);
this._interceptionIdToRequest.delete(request._interceptionId);
this._attemptedAuthentications.delete(request._interceptionId);
Expand Down Expand Up @@ -319,9 +322,6 @@ class Request {
this._interceptionHandled = false;
this._response = null;
this._failureText = null;
this._completePromise = new Promise(fulfill => {
this._completePromiseFulfill = fulfill;
});

this._url = url;
this._resourceType = resourceType.toLowerCase();
Expand Down Expand Up @@ -523,6 +523,10 @@ class Response {
this._request = request;
this._contentPromise = null;

this._bodyLoadedPromise = new Promise(fulfill => {
this._bodyLoadedPromiseFulfill = fulfill;
});

this._status = status;
this._url = request.url();
this._fromDiskCache = fromDiskCache;
Expand Down Expand Up @@ -581,7 +585,9 @@ class Response {
*/
buffer() {
if (!this._contentPromise) {
this._contentPromise = this._request._completePromise.then(async() => {
this._contentPromise = this._bodyLoadedPromise.then(async error => {
if (error)
throw error;
const response = await this._client.send('Network.getResponseBody', {
requestId: this._request._requestId
});
Expand Down
11 changes: 11 additions & 0 deletions test/network.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,17 @@ module.exports.addTests = function({testRunner, expect}) {
expect(await response.text()).toBe('{"foo": "bar"}\n');
expect(await response.json()).toEqual({foo: 'bar'});
});
it('Page.Events.Response should throw when requesting body of redirected response', async({page, server}) => {
server.setRedirect('/foo.html', '/empty.html');
const response = await page.goto(server.PREFIX + '/foo.html');
const redirectChain = response.request().redirectChain();
expect(redirectChain.length).toBe(1);
const redirected = redirectChain[0].response();
expect(redirected.status()).toBe(302);
let error = null;
await redirected.text().catch(e => error = e);
expect(error.message).toContain('Response body is unavailable for redirect responses');
});
it('Page.Events.Response should not report body unless request is finished', async({page, server}) => {
await page.goto(server.EMPTY_PAGE);
// Setup server to trap request.
Expand Down

0 comments on commit f8cba45

Please sign in to comment.