Merge branch 'bugfix/ARSN-384-redirect-error-body' into q/7.10

lib/errors/arsenalErrors.ts

@@ -690,6 +690,11 @@ export const ReportNotPresent: ErrorFormat = {
         'The request was rejected because the credential report does not exist. To generate a credential report, use GenerateCredentialReport.',
 };
 
+export const Found: ErrorFormat = {
+    code: 302,
+    description: 'Resource Found'
+};
+
 // ------------- Special non-AWS S3 errors -------------
 
 export const MPUinProgress: ErrorFormat = {

lib/s3routes/routes/routeWebsite.ts

@@ -27,6 +27,11 @@ export default function routerWebsite(
                 routesUtils.statsReport500(err, statsClient);
                 // request being redirected
                 if (redirectInfo) {
+                    if (err && redirectInfo.withError) {
+                        return routesUtils.redirectRequestOnError(err,
+                            'GET', redirectInfo, dataGetInfo, dataRetrievalFn,
+                            response, resMetaHeaders, log)
+                    }
                     // note that key might have been modified in websiteGet
                     // api to add index document
                     return routesUtils.redirectRequest(redirectInfo,
@@ -57,6 +62,11 @@ export default function routerWebsite(
             (err, resMetaHeaders, redirectInfo, key) => {
                 routesUtils.statsReport500(err, statsClient);
                 if (redirectInfo) {
+                    if (err && redirectInfo.withError) {
+                        return routesUtils.redirectRequestOnError(err,
+                            'HEAD', redirectInfo, null, dataRetrievalFn,
+                            response, resMetaHeaders, log)
+                    }
                     return routesUtils.redirectRequest(redirectInfo,
                         // TODO ARSN-217 encrypted does not exists in request.connection
                         // @ts-ignore

lib/s3routes/routesUtils.ts

@@ -664,6 +664,8 @@ export function streamUserErrorPage(
     log: RequestLogger,
 ) {
     setCommonResponseHeaders(corsHeaders, response, log);
+    response.setHeader('x-amz-error-code', err.message);
+    response.setHeader('x-amz-error-message', err.description);
     response.writeHead(err.code, { 'Content-type': 'text/html' });
     response.on('finish', () => {
         // TODO ARSN-216 Fix logger
@@ -863,6 +865,52 @@ export function redirectRequest(
     return undefined;
 }
 
+/**
+ * redirectRequestOnError - redirect with an error body
+ * @param err - arsenal error object
+ * @param method - HTTP method
+ * @param routingInfo - info for routing
+ * @param [routingInfo.withError] - flag to differentiate from routing rules
+ * @param [routingInfo.location] - location header
+ * @param dataLocations --
+ *   - array of locations to get streams from backend
+ * @param retrieveDataParams - params to create instance of
+ * data retrieval function
+ * @param response - response object
+ * @param corsHeaders - CORS-related response headers
+ * @param log - Werelogs instance
+ */
+export function redirectRequestOnError(
+    err: ArsenalError,
+    method: 'HEAD' | 'GET',
+    routingInfo: {
+        withError: true;
+        location: string;
+    },
+    dataLocations: { size: string | number }[] | null,
+    retrieveDataParams: any,
+    response: http.ServerResponse,
+    corsHeaders: { [key: string]: string },
+    log: RequestLogger,
+) {
+    response.setHeader('Location', routingInfo.location);
+
+    if (!dataLocations && err.is.Found) {
+        if (method === 'HEAD') {
+            return errorHeaderResponse(err, response, corsHeaders, log);
+        }
+        response.setHeader('x-amz-error-code', err.message);
+        response.setHeader('x-amz-error-message', err.description);
+        return errorHtmlResponse(err, false, '', response, corsHeaders, log);
+    }
+
+    // This is reached only for website error document (GET only)
+    const overrideErrorCode = err.flatten();
+    overrideErrorCode.code = 301;
+    return streamUserErrorPage(ArsenalError.unflatten(overrideErrorCode)!,
+        dataLocations || [], retrieveDataParams, response, corsHeaders, log);
+}
+
 /**
  * Get bucket name and object name from the request
  * @param request - http request object

tests/unit/s3routes/routesUtils/redirectRequestOnError.spec.js

@@ -0,0 +1,105 @@
+const assert = require('assert');
+const sinon = require('sinon');
+
+const DummyRequestLogger = require('../../storage/metadata/mongoclient/utils/DummyRequestLogger');
+const HttpResponseMock = require('../../../utils/HttpResponseMock');
+const routesUtils = require('../../../../lib/s3routes/routesUtils');
+const { errors } = require('../../../../index');
+const DataWrapper = require('../../../../lib/storage/data/DataWrapper');
+
+const corsHeaders = {
+    'access-control-allow-origin': '*',
+    'access-control-allow-methods': 'GET',
+};
+
+function assertHeaders(responseMock, expectedHeaders) {
+    for (const [key, val] of Object.entries(expectedHeaders)) {
+        assert.strictEqual(responseMock._headers[key], val);
+    }
+}
+
+describe('routesUtils.redirectRequestOnError', () => {
+    describe('from request on folder containing ' +
+    'index without trailing /', () => {
+        const errorHeaders = {
+            'x-amz-error-code': errors.Found.type,
+            'x-amz-error-message': errors.Found.description,
+        };
+
+        it('should redirect 302 with body on GET', () => {
+            const responseMock = new HttpResponseMock();
+            const routing = { withError: true, location: '/photos/' };
+            routesUtils.redirectRequestOnError(
+                errors.Found, 'GET',
+                routing, null, null, responseMock,
+                corsHeaders, new DummyRequestLogger(),
+            );
+
+            assert.strictEqual(responseMock.statusCode, 302);
+            assertHeaders(responseMock, corsHeaders);
+            assertHeaders(responseMock, errorHeaders);
+            assert.strictEqual(responseMock._headers.Location, routing.location);
+            assert.match(responseMock._body, /<h1>302 Found<\/h1>/);
+            assert.match(responseMock._body, /<li>Code: Found<\/li>/);
+            assert.match(responseMock._body, /<li>Message: Resource Found<\/li>/);
+        });
+
+        it('should redirect 302 without body on HEAD', () => {
+            const responseMock = new HttpResponseMock();
+            const routing = { withError: true, location: '/photos/' };
+            routesUtils.redirectRequestOnError(
+                errors.Found, 'HEAD',
+                routing, null, null, responseMock,
+                corsHeaders, new DummyRequestLogger(),
+            );
+
+            assert.strictEqual(responseMock.statusCode, 302);
+            assertHeaders(responseMock, corsHeaders);
+            assertHeaders(responseMock, errorHeaders);
+            assert.strictEqual(responseMock._headers.Location, routing.location);
+            assert.strictEqual(responseMock._body, null);
+        });
+    });
+
+    describe('from error document redirect location header', () => {
+        let dataWrapperGetStub;
+
+        afterAll(() => {
+            if (dataWrapperGetStub) {
+                dataWrapperGetStub.restore();
+            }
+        });
+
+        it('should redirect 301 with body on GET', () => {
+            const responseMock = new HttpResponseMock();
+            const routing = { withError: true,
+                location: 'http://scality.com/test' };
+            const errorHeaders = {
+                'x-amz-error-code': errors.AccessDenied.type,
+                'x-amz-error-message': errors.AccessDenied.description,
+            };
+            dataWrapperGetStub = sinon.stub(DataWrapper.prototype, 'get');
+
+            const mockedDataLocations = [{ mock: true }];
+            const mockedRetrieveDataParams = {
+                mockRetrieveDataParams: true,
+            };
+
+            routesUtils.redirectRequestOnError(
+                errors.AccessDenied, 'GET',
+                routing, mockedDataLocations, mockedRetrieveDataParams,
+                responseMock, corsHeaders, new DummyRequestLogger(),
+            );
+
+            assert.strictEqual(responseMock.statusCode, 301);
+            assertHeaders(responseMock, corsHeaders);
+            assertHeaders(responseMock, errorHeaders);
+            assert.strictEqual(responseMock._headers.Location, routing.location);
+            assert.strictEqual(dataWrapperGetStub.callCount, 1);
+            assert.strictEqual(dataWrapperGetStub.getCall(0).args[0],
+                mockedDataLocations[0]);
+            assert.strictEqual(dataWrapperGetStub.getCall(0).args[1],
+                responseMock);
+        });
+    });
+});

tests/utils/HttpResponseMock.js

@@ -1,3 +1,4 @@
+const http = require('http');
 
 /**
  * Basic response mock to catch response values.
@@ -9,6 +10,7 @@
 class HttpResponseMock {
     constructor() {
         this.statusCode = null;
+        this.statusMessage = null;
         this._headers = {};
         this._body = null;
     }
@@ -18,7 +20,10 @@ class HttpResponseMock {
     }
 
     end(data, encoding, callback) {
-        this.write(data, encoding, callback);
+        if (!callback && typeof data === 'function') {
+            return data();
+        }
+        return this.write(data, encoding, callback);
     }
 
     write(chunk, encoding, callback) {
@@ -43,6 +48,7 @@ class HttpResponseMock {
 
     writeHead(statusCode, statusMessage, headers) {
         this.statusCode = statusCode;
+        this.statusMessage = http.STATUS_CODES[statusCode];
         let headersObj = headers;
 
         if (!headersObj && typeof statusMessage === 'object') {
@@ -61,6 +67,9 @@ class HttpResponseMock {
             Object.assign(this._headers, headersObj);
         }
     }
+
+    on() {}
+    once() {}
 }
 
 module.exports = HttpResponseMock;